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

Better message when exceeding rate limit #8

Closed
hemanth opened this issue Dec 10, 2015 · 6 comments
Closed

Better message when exceeding rate limit #8

hemanth opened this issue Dec 10, 2015 · 6 comments

Comments

@hemanth
Copy link
Contributor

hemanth commented Dec 10, 2015

$ dev-time SamVerschueren

SamVerschueren
HTTPError: Response code 403 (Forbidden)
    at /Users/Hemanth/.nvm/versions/node/v5.1.0/lib/node_modules/dev-time-cli/node_modules/got/index.js:102:11
    at BufferStream.<anonymous> (/Users/Hemanth/.nvm/versions/node/v5.1.0/lib/node_modules/dev-time-cli/node_modules/read-all-stream/index.js:64:3)
    at emitNone (events.js:72:20)
    at BufferStream.emit (events.js:166:7)
    at finishMaybe (/Users/Hemanth/.nvm/versions/node/v5.1.0/lib/node_modules/dev-time-cli/node_modules/readable-stream/lib/_stream_writable.js:509:14)
    at endWritable (/Users/Hemanth/.nvm/versions/node/v5.1.0/lib/node_modules/dev-time-cli/node_modules/readable-stream/lib/_stream_writable.js:519:3)
    at BufferStream.Writable.end (/Users/Hemanth/.nvm/versions/node/v5.1.0/lib/node_modules/dev-time-cli/node_modules/readable-stream/lib/_stream_writable.js:484:5)
    at Unzip.onend (_stream_readable.js:490:10)
    at Unzip.g (events.js:260:16)
    at emitNone (events.js:72:20)

This looks like I have exceeded the request limit, should we log something meaningful here?
Say like use an auth-token?

@SamVerschueren
Copy link
Owner

Good point. But the error should be thrown in dev-time.

@hemanth
Copy link
Contributor Author

hemanth commented Dec 10, 2015

Should the cli catch and throw a suitable message based on what kind of error it was, rather than dev-time doing it?

As in dev-time just throw the error and based on that the cli will log a suitable message.

@SamVerschueren
Copy link
Owner

Yes, the error Response code 403 doesn't say a lot. I think that GitHub returns the throttle limit in the headers so if the throttle limit is exceeded, it should throw new Error('throttle limit exceeded') instead of that 403. The message can then be formatted here.

@hemanth
Copy link
Contributor Author

hemanth commented Dec 10, 2015

Either way, makes sense.

@SamVerschueren SamVerschueren changed the title Better message for forbidden? Better message when exceeding rate limit Dec 10, 2015
@SamVerschueren
Copy link
Owner

1.3.1

screen shot 2015-12-10 at 20 34 00

@hemanth
Copy link
Contributor Author

hemanth commented Dec 11, 2015

👍

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

No branches or pull requests

2 participants