Skip to content

fix: honor Retry-After on 5xx responses, not just 429 - #619

Merged
ffumero2003 merged 2 commits into
masterfrom
honor-retry-after-on-5xx
Oct 1, 2026
Merged

ffumero2003 merged 2 commits into
masterfrom
honor-retry-after-on-5xx

Conversation

@ffumero2003

Copy link
Copy Markdown
Contributor

Context

When the server returns a 5xx (e.g. 503 service_unavailable) carrying a Retry-After
header, the client ignores it and retries on its own default exponential backoff (~1s first
step) instead of waiting the requested interval. A 429 carrying the same Retry-After
is honored correctly. The raw responses are byte-for-byte identical apart from status/code, so
this is a client handling difference, not a server difference — and it makes the client retry
more aggressively than B2 explicitly asked for, which is exactly what Retry-After exists to
prevent. Backblaze's Integration Checklist gives the same Retry-After guidance for 5xx as for
429.

Root cause

In b2sdk/_internal/exception.py there is exactly one write site and one read site for
retry_after_seconds. The 429 branch builds TooManyRequests(retry_after_seconds=...), and
TooManyRequests.__init__ is the only constructor that stored the value. The
500 <= status < 600 branch builds ServiceError from the message string only and never reads
response_headers, and ServiceError had no retry_after_seconds slot — it inherited
B2Error.__init__, which sets retry_after_seconds = None unconditionally. b2http's
_translate_and_retry already honors a non-None retry_after_seconds on read, so only the
write side for 5xx was missing.

Fix

  • Give ServiceError an __init__(self, *args, retry_after_seconds=None) that stores the value
    when present (mirroring TooManyRequests), keeping the positional message argument.
  • Pass retry_after_seconds=response_headers.get('retry-after') from the 5xx branch.

No change to b2http's read logic — it already honors the value; only the 5xx write side was
missing.

Verification

  • New unit tests in test/unit/test_exception.py: a ServiceError built from a 503 with
    {'retry-after': 200} has retry_after_seconds == 200, and None without the header. Red
    before the change, green after.
  • Full unit suite green across all apivers (nox -s unit).
  • Independently confirmed against a B2 simulator: the api.retry_after_503 scenario goes
    FAIL → PASS (the client now waits the server-requested interval before retrying).

Fixes #606

A 5xx carrying Retry-After was ignored: the client retried on its default
backoff instead of waiting the requested time, unlike 429. Give ServiceError a
retry_after_seconds slot (mirroring TooManyRequests) and pass the header from the
5xx branch of interpret_b2_error; b2http already honors a non-None value.

@sophiecarreras sophiecarreras left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right, and it reuses the existing sleep path in _translate_and_retry. Non-blocking: (1) int(retry_after) will raise ValueError for the HTTP-date form of Retry-After, which proxies in front of B2 can emit; please fall back to None (default backoff) instead of failing inside interpret_b2_error; (2) consider whether very large values should be capped, as with 429; (3) please add a string-valued header case and a 503 test through _translate_and_retry so the actual sleep is covered. CI: unit sessions are green; the red jobs are the known test_encryption integration failures (#614).

@ffumero2003
ffumero2003 merged commit 7e14503 into master Oct 1, 2026
10 of 28 checks passed
@ffumero2003
ffumero2003 deleted the honor-retry-after-on-5xx branch October 1, 2026 17:16
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.

Retry-After is honored on 429 but structurally cannot be read on any 5xx response

2 participants