Skip to content

Cache and parse only a successful response of /rate_limit - #1310

Open
Thayorns wants to merge 1 commit into
zerocracy:masterfrom
Thayorns:1225
Open

Thayorns wants to merge 1 commit into
zerocracy:masterfrom
Thayorns:1225

Conversation

@Thayorns

Copy link
Copy Markdown
Contributor

Fbe::Middleware::RateLimit stored the response of /rate_limit before looking at its status. One 5xx was then served from the cache to every later off_quota?, the client answered "off quota" for the rest of the process, and the retries never reached GitHub. With an HTML error body the parse raised JSON::ParserError on every call, as #1225 shows.

Now a response of /rate_limit is cached and parsed only when it is a success. An error is passed on as it is, without touching the cache or the counters, so the retry and the next quota check ask GitHub again, and an error body is never parsed.

The middleware tests cover a random 5xx and a client error followed by a 200, an error with an HTML body, and a remaining count that stays unknown after an error. One more test in test_octo.rb goes through Fbe.octo with a token, a 502 with HTML and then a 200, and checks that a repository call gets through.

Closes #1225

@Thayorns

Copy link
Copy Markdown
Contributor Author

@yegor256 take a look please, happy to clarify anything about this change.

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

Successfully merging this pull request may close these issues.

Fbe::Middleware::RateLimit caches an error response of /rate_limit, so one 5xx blocks every later call

1 participant