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

storyblok push-components aborts with "Error: Too Many Requests" #64

Open
eggnstone opened this issue Jul 25, 2023 · 20 comments
Open

storyblok push-components aborts with "Error: Too Many Requests" #64

eggnstone opened this issue Jul 25, 2023 · 20 comments
Labels
bug Something isn't working

Comments

@eggnstone
Copy link

eggnstone commented Jul 25, 2023

Current behavior:
I'm uploading my components.json (with 21 components) to an existing space:
storyblok push-components --space 12345 components.json
After a few successful updates it reliably aborts like this:

- Executing push-components task
...
- Updating component XYZ...
X An error occurred when load components file from api
X An error occurred when executing the push-components task: Error: Too Many Requests

Expected behavior:
Successful execution of the command without an error.

Steps to reproduce:
storyblok push-components --space 12345 components.json

Other information:
This is probably due to being on a limited plan where the API calls have a limited rate.
That rate should be adhered to or at least be made configurable.

Edit
storyblok-cli version: 3.25.0
It used to work with earlier version.

Edit2
It used to work with earlier versions like 3.24.1 where subtasks could fail, too, but it did not abort:

- Updating component LinkedBox...
- Checking target presets to sync...
X An error occurred when update component LinkedBox
Request failed with status code 429
- Get presets from component PageTemplate
- Updating component PageTemplate...
Hit rate limit. Retrying in 1 seconds.
- Checking target presets to sync...
✓ Component PageTemplate has been updated in Storyblok!
@eggnstone eggnstone added the bug Something isn't working label Jul 25, 2023
@ademarCardoso
Copy link
Member

Hi @eggnstone thanks to reporting the problem, could you check again ? We had an update on our API's

If you test and the problem persist, please can you send me the current plan of the source and target spaces, to check if it's a limitation by the plan.

I say this because this 429 usually only happens if you exceed the request limit within a certain period of time, but to be sure, I'll need to know the plans, but only if the problem is still happening.

@eggnstone
Copy link
Author

Yes, still happening with 3.25.1
Plan: Development
Space: 188619

But even if it's a limitation by the plan, the CLI should adjust to that and not abort with error.

@ademarCardoso
Copy link
Member

Thanks to confirm the problem, i will investigate that.

@ademarCardoso
Copy link
Member

Hello again my friend @eggnstone i added some changes and published the version 3.25.2, could you check to see if this version it's working ?

@eggnstone
Copy link
Author

Same error:

- Updating component BlogPosts...
X An error occurred when load components file from api
X An error occurred when executing the push-components task: Too Many Requests
at new Promise (<anonymous>)
  at w._statusHandler (C:\Users\Mark\AppData\Roaming\npm\node_modules\storyblok\node_modules\storyblok-js-client\dist\index.umd.js:24:5206)
  at w._methodHandler (C:\Users\Mark\AppData\Roaming\npm\node_modules\storyblok\node_modules\storyblok-js-client\dist\index.umd.js:24:5083)
  at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
  at async push (C:\Users\Mark\AppData\Roaming\npm\node_modules\storyblok\src\tasks\push-components.js:164:11)
  at async Command.<anonymous> (C:\Users\Mark\AppData\Roaming\npm\node_modules\storyblok\src\cli.js:170:7)

I always add

// sleep to avoid rate limit
await new Promise(resolve => setTimeout(resolve, 250));

in push-components.js at line 169
before

        await api.put(`components/${id}`, {
            component: components[i]
          })

@ademarCardoso
Copy link
Member

Hi @eggnstone ,this error doesn't happen to me (I tried to push 500 components), I opened a PR draft could you take a look and try to use this approach and let me know if it works or not.

If that doesn't work, I should go with your idea, I didn't go with it the first time because it can add a certain amount of time to push many components, but it's a solution.

Thanks a lot for the help.

@eggnstone
Copy link
Author

I'm no JS expert but the PR looks to me like you are collecting the push results and then wait for them at the end, but because you await each result before adding it to the array, I don't see any difference.
If you would add to the array without awaiting it, the problem would even increase, I think.

I don't see how this can address the "too many requests" which basically means "too many calls per second" because I am on a limited plan.

Is there a way to determine the plan? Then based on that plan a delay could be added before each api.put()

@marcesengel
Copy link
Contributor

@ademarCardoso would you be open to accept a PR introducing exponential back-off? We're having the same issue (strangely with very few components, I counted 7). The space currently is on the community plan, that should be the easiest way to reproduce the issue because of it having the lowest rate limits.

Screenshot

image

@marcesengel
Copy link
Contributor

After digging a bit deeper, I've found this official post concerning the management api rate limits. There it says

Customers using Storyblok’s official nodejs SDK “storyblok-js-client” are not affected by this change as the module already handles throttling the requests to the right level.

As this CLI tool is utilising the SDK, I now am left wondering if this is an upstream error?

@PhiFry
Copy link

PhiFry commented Sep 11, 2023

I think the issue lies in https://github.com/storyblok/storyblok-cli/blob/master/src/utils/api.js
image
This function creates a new instance of Storyblok js sdk everytime the CLI wants to do a request to the REST API. So if my push file has more than 3 or 6 components and my internet connection is fast then I dont think it throttles itself.

Proposed solution

make getClient() cache a Storyblok instance using lazy loading.

@PhiFry
Copy link

PhiFry commented Sep 11, 2023

Can confirm my solution worked, modified script locally and now my push components command works and it is clearly throttling now.
Screenshot 2023-09-11 at 09 26 05

@marcesengel
Copy link
Contributor

@PhiFry yeah that'd make sense - however the push function itself is passed the api as a parameter, see https://github.com/storyblok/storyblok-cli/blob/master/src/tasks/push-components.js#L79... So I'm assuming that where the function is invoked multiple clients are used?

@PhiFry
Copy link

PhiFry commented Sep 11, 2023

@marcesengel The api object is indeed passed and not created multiple times, the issue is the Storyblok object (which is the Storyblok js SDK object) is being re-created for each component when it calls the put and post commands to upload the component to Storyblok and thus the internal throttle mechanism gets reset each time.

@marcesengel
Copy link
Contributor

@PhiFry ah I see 🙌 do you think you could provide a quick PR so we can install the CLI from your fork instead of from npm? That'd be very kind 🙈

@marcesengel marcesengel mentioned this issue Sep 12, 2023
5 tasks
@marcesengel
Copy link
Contributor

@eggnstone @PhiFry I've created a fork (+ PR, see above) implementing @PhiFry's findings. Feel free to give it a try using npm i -D cashy-at/storyblok-cli#fix/request-throttling.

@mikewoudenberg
Copy link

Hi, Doesn't this fix (storyblok/storyblok-js-client#732) also fix the underlying issue here? If so, could you roll out a new version with the updated dependency?

@dojscart
Copy link

dojscart commented Mar 5, 2024

Hi @ademarCardoso, is there any chance this could be fixed? That way, Storyblok's enterprise customers that have to use SSO logins can use the official Storyblok CLI rather than a fork.

Many thanks!

@christianzoppi
Copy link
Contributor

Hi everyone! Thanks for sharing the details We recently merged #72 created by @marcesengel, which aims to mitigate the issue. Since v3.31.1 you shouldn't face this issue anymore, at least reaching the limit should be harder. If you still face this issue with that version please let us know.

@alvarosabu
Copy link
Contributor

Hi all, I'm gonna close this as completed since it was released on v3.31.1, if you still encounter this issue please feel free to re-open.

@oe-josh-martin
Copy link

oe-josh-martin commented Sep 27, 2024

@christianzoppi @alvarosabu unfortunately we're still seeing this issue with 3.32.2.

We have a script that allows people to pull from a centralised space and push out to their own spaces. They enter their space ID from a readline prompt ➜ Please enter your Storyblok Space ID (e.g. 245488): 144190.

The centralised space is 240757, and the consumer space in this instance is 144190.

const componentsPath = path.join(
    __dirname,
    '../types/generated/storyblok/components.240757.json'
);

storyblok push-components --space ${SPACE_ID} ${componentsPath} && storyblok sync --type datasources --source 240757 --target ${SPACE_ID}`

However, we're still being met with the Too Many Requests error:

➜ Please enter your Storyblok Space ID (e.g. 245488): 144190
stderr: X An error occurred when update component coralBlokChip
Too Many Requests

In our implementation we catch any stderr early and exit the task, which is why there's only one error listed before the task exits.

exec(command, (error, stdout, stderr) => {
    if (error) {
        return console.log(`Error: ${error.message}`);
    } else if (stderr) {
        return console.log(`stderr: ${stderr}`);
    } else {
        console.log(stdout);
        console.log('🎉 Done!');
    }
});

@alvarosabu alvarosabu reopened this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants