Add RateLimitError for HTTP 429 responses from Google recognizer - #904
Add RateLimitError for HTTP 429 responses from Google recognizer#904harshitayadavv wants to merge 3 commits into
Conversation
obtain_transcription() previously wrapped every HTTPError -- including rate limiting (429) -- into a generic RequestError using only e.reason, discarding the actual status code. This made it impossible to catch rate limiting separately from other request failures, or to implement backoff using the Retry-After header. Adds a RateLimitError(RequestError) subclass with a retry_after attribute parsed from the Retry-After header when present. 429 responses now raise RateLimitError specifically; all other HTTPErrors continue to raise RequestError as before, so this is backward compatible for anyone already catching RequestError. Includes a test covering the 429 case and confirming retry_after is parsed correctly. Ran full test suite, flake8, and mypy locally -- all clean except pre-existing, unrelated failures from optional recognizer backends not installed in this environment (cohere, vosk, groq, openai, faster_whisper, soundfile, pocketsphinx, google.cloud.speech).
|
@harshitayadavv Thanks for the proposal. This looks like a useful improvement. |
Per @ftnext's review: Retry-After can be a non-negative integer number of seconds OR an HTTP-date, per RFC 9110 10.2.3. The previous implementation only handled the numeric form via float(), silently discarding a date-formatted header instead of parsing it. Note: I could not find official Google documentation for this specific endpoint's Retry-After format, since it's an undocumented legacy API (the same one Chromium uses internally, accessed via a hardcoded key). Rather than assert an unverifiable claim about the exact format Google returns, this makes the parsing correct for both valid forms per the HTTP spec, so the behavior is correct regardless of which one is actually used.
|
Good catch, and you're right to ask, I can't point to official documentation for this specific endpoint's Retry-After format, since it's an undocumented legacy API (the same one Chromium uses internally). Rather than assume, I've updated the parsing to correctly handle both valid forms defined by RFC 9110 §10.2.3 (seconds or an HTTP-date), so it's correct regardless of which one this endpoint actually sends. Added a test covering the HTTP-date case too. |
|
@harshitayadavv Thanks for the update and for making the parser RFC-compliant. However, my main question is whether this Google endpoint actually returns a Could you provide evidence that an actual 429 response includes this header? Otherwise, I would prefer to keep this change scoped to raising RateLimitError for HTTP 429 and omit Retry-After handling until it is documented or observed. |
Per @ftnext's follow-up review: drop the retry_after attribute and Retry-After parsing entirely, since we have no documentation or observed evidence that this endpoint actually sends that header at all. Keeping the change scoped to just raising RateLimitError for HTTP 429, which is directly observable and doesn't rely on any unverified assumption about response headers.
|
That's a fair point, I don't have evidence this endpoint sends Retry-After at all, so asserting a retry_after attribute on RateLimitError would be speculative. I've removed it entirely and scoped this back down to just raising RateLimitError for HTTP 429, which is the part I can actually observe and verify. |
|
@harshitayadavv Thanks for simplifying the change. However, now that |
Problem
obtain_transcription()inspeech_recognition/recognizers/google.pycatches
HTTPErrorand wraps every case -- including HTTP 429 (ratelimited) -- into a generic
RequestError, using onlye.reason(astring like "Too Many Requests") and discarding
e.codeentirely.This makes it impossible to:
key, network issues)
Retry-Afterheader is neversurfaced
Reproduction
Fix
Adds
RateLimitError(RequestError)inspeech_recognition/exceptions.pywith a
retry_after: float | Noneattribute, parsed from theRetry-Afterheader when the service returns HTTP 429. Since itsubclasses
RequestError, this is backward compatible -- existing codecatching
RequestErrorcontinues to work unchanged, while callers whowant to specifically detect and back off on rate limiting can now catch
RateLimitError.Includes a test in
tests/recognizers/test_google.pycovering the 429case and confirming
retry_afterparses correctly.Ran the full test suite,
flake8, andmypylocally -- all cleanexcept pre-existing failures from optional recognizer backends not
installed in this environment (unrelated to this change).