-
Notifications
You must be signed in to change notification settings - Fork 736
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
Implement an abuse handler #289
Conversation
// Check to see whether we hit a 403, and the message indicates the | ||
// abuse handler (occurs on too many concurrent requests) | ||
if (responseCode == HttpURLConnection.HTTP_FORBIDDEN && | ||
errorString.contains("You have triggered an abuse detection mechanism.")) { |
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.
@kohsuke This is the only bit I'm not totally sure about. The best bit would be to just be able to check for 403, but I'm not sure whether 403 happens in more circumstances than just API abuse. There wasn't anything else in the header indicating abuse (though maybe I could just check for Retry-After)
Additionally, I imagine that there isn't a way to get a localized error string here, but if there was the check would be broken. Thoughts?
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.
My 2 cents:
I'd just check for status code and retry-after, those are the ones that are documented.
The human readable hint/text isn't quite documented.
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.
Okay, yeah I think you're right.
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.
Fixed
@@ -576,6 +576,15 @@ private InputStream wrapStream(InputStream in) throws IOException { | |||
InputStream es = wrapStream(uc.getErrorStream()); | |||
try { | |||
if (es!=null) { | |||
String errorString = IOUtils.toString(es, "UTF-8"); |
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.
Dead store to errorString in org.kohsuke.github.Requester.handleApiError(IOException) [org.kohsuke.github.Requester] At Requester.java:[line 579]
If too many requests are made within X amount of time (not the traditional hourly rate limit), github may begin returning 403. Then we should wait for a bit to attempt to access the API again. In this case, we parse out the Retry-After field returned and sleep until that (it's usually 60 seconds)
Fixed and moved some code around. |
If too many requests are made within X amount of time (not the traditional hourly rate limit), github may begin returning 403. Then we should wait for a bit to attempt to access the API again. In this case, we parse out the Retry-After field returned and sleep until that (it's usually 60 seconds)
Fixes #285