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

VcsRepository: do not continue when receiving 429 rate limit exception #10132

Merged

Conversation

glaubinix
Copy link
Contributor

Currently Composer ignores all TransportExceptions (except 401 and 403) while fetching composer.json files to import all available versions and then skips those versions.

This pull request changes this behaviour for rate limit errors with a 429 status code. After a rate limit error response was received all future requests to that resource will most likely receive the same error status code which make it unlikely that Composer is able to finish the command.

Sample Composer output for such a scenario:

Reading composer.json of acme/package (2.0.0) 
Downloading https://api.bitbucket.org/2.0/repositories/acme/package/src/hash/composer.json
[429] https://api.bitbucket.org/2.0/repositories/acme/package/src/hash/composer.json
Skipped tag 2.0.0, no composer file was found (429 HTTP status code)
Reading composer.json of acme/package (2.0.1) 
Downloading https://api.bitbucket.org/2.0/repositories/acme/package/src/hash/composer.json
[429] https://api.bitbucket.org/2.0/repositories/acme/package/src/hash/composer.json
Skipped tag 2.0.1, no composer file was found (429 HTTP status code)
Reading composer.json of acme/package (2.0.2) 
Downloading https://api.bitbucket.org/2.0/repositories/acme/package/src/hash/composer.json
[429] https://api.bitbucket.org/2.0/repositories/acme/package/src/hash/composer.json
Skipped tag 2.0.2, no composer file was found (429 HTTP status code)

I do wonder whether the behaviour in the VcsRepository should be changed in general and abort for most errors including 5XX errors and curl errors. A VcsRepository that doesn't receive all available versions can lead to an unexpected result of a Composer command e.g. composer update might unexpectedly downgrade a package because the request to fetch the composer.json file of the latest tag returned a 500 response.

@Seldaek
Copy link
Member

Seldaek commented Oct 2, 2021

Yeah I guess the odds of github starting to return 500s in the middle of the update is low enough and it'll break enough other things that the update most likely wouldn't complete anyway, but technically maybe we could include all 5xx. Will merge this already though as the rate limit is definitely a more likely scenario and it makes sense for sure.

@Seldaek Seldaek merged commit edccad4 into composer:master Oct 2, 2021
@Seldaek Seldaek added this to the 2.1 milestone Oct 2, 2021
@Seldaek Seldaek added the Bug label Oct 2, 2021
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.

None yet

2 participants