Repository navigation
Conversation
Every client that failed together retried at exactly base * 2^attempt, so a shared 5xx event produced synchronized retry waves. Draw each delay uniformly from [0, base * 2^attempt) instead.
A 503 means the server is shedding load, so blind retries add to the overload. Retry it only when the server says when via Retry-After. Any retried response that carries Retry-After waits that long instead of the jittered backoff.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| date = parsedate_to_datetime(value) | ||
| except (TypeError, ValueError): | ||
| return None | ||
| return max(0.0, date.timestamp() - time.time()) |
There was a problem hiding this comment.
🟡 Asctime retry dates use local time
For an asctime Retry-After date, parse_retry_after calculates the retry delay in local time. parsedate_to_datetime returns a naive datetime for this valid HTTP-date format, so clients outside UTC retry hours early or late.
Learn more
HTTP-date accepts the asctime form, which has no timezone suffix but still denotes GMT. parsedate_to_datetime returns a naive datetime for that form. datetime.timestamp() interprets naive values in the client's local timezone, shifting the retry delay even though the server supplied a valid date.
Example: With the client in New York at 03:25:35 UTC, Fri Oct 16 03:25:45 2026 means retry in 10 seconds. The parser instead returns 14,410 seconds, so the request waits about four hours.
Recommended fix: In parse_retry_after, assign timezone.utc to naive parsed dates before calling timestamp(), and add an asctime-format test under a non-UTC TZ.
| return max(0.0, date.timestamp() - time.time()) | |
| return max(0.0, date.replace(tzinfo=timezone.utc).timestamp() - time.time()) if date.tzinfo is None else max(0.0, date.timestamp() - time.time()) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if value.isdigit(): | ||
| return float(value) |
There was a problem hiding this comment.
🟡 Digit-shaped retry header crashes API calls
When a response contains Retry-After: ², parse_retry_after raises ValueError before the client handles its status. isdigit() accepts that character, but float() rejects it; even a terminal 400 response loses its ServerError.
Learn more
The client parses Retry-After on every non-200 response before deciding whether to retry. Python's isdigit() accepts characters like ², although float() cannot parse them. An unexpected header therefore escapes as ValueError rather than producing the response's ServerError.
Example: An API returns HTTP 400 with Retry-After: ². Instead of surfacing status 400, TwirpClient.request raises ValueError from the parser.
Recommended fix: Restrict delay-seconds to ASCII decimal digits before converting, and treat all other strings as dates or invalid headers. Test invalid Unicode digits on both terminal and retryable statuses.
| if value.isdigit(): | |
| return float(value) | |
| if value.isascii() and value.isdecimal(): | |
| return float(value) |
Was this helpful? React with 👍 or 👎 to provide feedback.
This branch has not been deployed
1 participant
Why
The failover retry loop has two gaps that matter when many clients fail at the same moment:
0.2s * 2^attempt, so every client that failed during a shared 5xx event retries in lockstep at 200ms and then 400ms, hitting the server in synchronized waves.Retry-Afteris ignored even when the server says how long to wait.What
Two commits, each with tests:
backoff(attempt, base)draws each delay uniformly from[0, base * 2^attempt)(AWS "full jitter"). Base, cap on attempts, and the timeout gate are unchanged.Retry-After, and honorRetry-After. A 503 withoutRetry-Afteris now terminal. A 503 withRetry-Afteris retried. Any retried response that carriesRetry-Afterwaits that long instead of the jittered backoff.parse_retry_afteraccepts both forms from RFC 9110: delay-seconds and an HTTP-date. Other 5xx and transport errors retry as before.Behavior change
[0, 200ms)and[0, 400ms)instead of exactly 200ms and 400ms.Retry-Afterfails after one attempt instead of up to 3, including during cross-region failover on*.livekit.cloudhosts.Retry-Afterwaits the indicated delay before the next attempt. That delay is controlled by the server and has no cap.Cross-SDK parity
Same change as livekit/node-sdks#723, from review feedback on that PR (jitter, 503/Retry-After). livekit/server-sdk-go still has the deterministic backoff and retries every 5xx.
Testing
pytest tests/api -q: 63 passed, withlivekit/test-server(built from livekit/livekitcmd/test-server) running locally, as in theTest APIworkflow.tests/api/test_failover_unit.py:test_backoff_is_full_jitter,test_503_without_retry_after_not_retried,test_503_with_retry_after_retried,test_parse_retry_after.tests/api/test_failover.pyhelper now asks it for a 502. Those tests exercise region failover, which a 502 still triggers.ruff checkandruff format --checkpass with the repo's pinned ruff (0.15.4).