feat(gecko): Improve error message when Github api limit reached. #217
Conversation
MJDSys
force-pushed
the
nicer_ratelimit_error
branch
from
ce11048 to
fd18f51
Compare
|
I'm not sure why tests are failing with this change. Some of them seem to occur if the Github api limit is reached (I artificially did that from my computer for testing), and other happened without any changes from me. Some of them seem to happen because remote responses change. None of them seem related to the error message changing. |
There was a problem hiding this comment.
Ran the CircleCI test suite again and it passes. This is a great pull request because it helps describe what's going on when the request to github fails. I originally created the cached files to avoid hitting github's api not more than once per hour. Of course the user could easily get to the rate limit by cleaning out the selenium/ folder and retrying until they reach the limit.
I have a nit comment that I'd like you to change. Thanks!
lib/binaries/config_source.ts
Outdated
| @@ -157,7 +157,11 @@ export abstract class GithubApiConfigSource extends JsonConfigSource { | |||
| }); | |||
|
|
|||
| } else { | |||
| reject(new Error('response status code is not 200')); | |||
| if (response.statusCode == 403 && response.headers['x-ratelimit-remaining'] == 0) { | |||
There was a problem hiding this comment.
Please fix this conditional. This should be combined to the conditional above as:
} else if (response.statusCode == 403 && response.headers['x-ratelimit-remaining'] == 0) {
Similar nit for the else statement below.
When the Github API limit is reached, the error message now directly informs the user. Also, when any other failure occurs the status code is reported. This should hopefully give more info for issue angular#216.
MJDSys
force-pushed
the
nicer_ratelimit_error
branch
from
fd18f51 to
ab7fe25
Compare
|
I've pushed a new version with the requested change. |
When the Github API limit is reached, the error message now directly informs
the user. Also, when any other failure occurs the status code is reported.
This should hopefully give more info for issue #216.