Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update the retry function to check the response status correctly #754

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

christianzoppi
Copy link
Contributor

@christianzoppi christianzoppi commented Jan 18, 2024

This PR fixes the function that retries requests when a rate limit is hit. This bug was already present in the past and somebody tried to fix it in the #732 but that didn't seem the solution.

Pull request type

Jira Link: INT-

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Other (please describe):

How to test this PR

You need to test this both in Node and client-side.

Node.js
In the playground/index-node.js put the content below and then run node playground/index-node.

const Client = require('../')

const client = new Client({
  accessToken: "TOKEN",
  rateLimit: 70,
  maxRetries: 30
});

const getLinks = async () => {
  console.time("APIrequests");
  const stories = 5000
  const perPage = 25
  const pages = Math.ceil(stories / perPage)

  try {
    const requests = []
    for (let index = 0; index < pages; index++) {
      requests.push(client.get("cdn/stories", {per_page: perPage, page: index}))
    }
    const res = (await Promise.all(requests))
    console.timeEnd("APIrequests");
} catch(err) {
  console.log(err)
}
};

getLinks();

Client-side
In the playground/main.ts add the code below, then run in the console npm run demo and in your browser refresh the page of the demo until you see the retry message in the console.

import StoryblokClient from '../'

const client = new StoryblokClient({
  accessToken: "TOKEN",
  rateLimit: 70,
  maxRetries: 30
});

const getLinks = async () => {
  console.time("APIrequests");
  const stories = 5000
  const perPage = 25
  const pages = Math.ceil(stories / perPage)

  try {
    const requests = []
    for (let index = 0; index < pages; index++) {
      requests.push(client.get("cdn/stories", {per_page: perPage, page: index}))
    }
    const res = (await Promise.all(requests))
    console.timeEnd("APIrequests");
} catch(err) {
  console.log(err)
}
};

getLinks();

What is the new behavior?

Once a rate limit is hit the retry mechanism is executed and the console will show the message "Hit rate limit. Retrying in X seconds."

Other information

Copy link
Member

@ademarCardoso ademarCardoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me 😄

src/index.ts Show resolved Hide resolved
@thiagosaife thiagosaife merged commit f9c88e9 into main Jan 18, 2024
1 check passed
@thiagosaife thiagosaife deleted the fix/retry-mechanism-bug branch January 18, 2024 14:25
Copy link

🎉 This PR is included in version 6.6.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants