-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Adding in handling for secondary rate limit exceptions #2473
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -719,7 +719,7 @@ static Exception GetExceptionForForbidden(IResponse response) | |
{ | ||
string body = response.Body as string ?? ""; | ||
|
||
if (body.Contains("rate limit exceeded")) | ||
if (body.Contains("rate limit exceeded") || body.Contains("secondary rate limit")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it might be better throw a different kind of exception for a secondary rate limit. What do you think? I'm just thinking that it would make it easier to handle this specific case as a caller, without doing string matching. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get where you are coming from, makes sense to me :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’d just define a new exception and raise that. It should be a quick change, and then we can get this approved and merged ✅ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've changed it to be it's own exception. |
||
{ | ||
return new RateLimitExceededException(response); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.