fix(ai): terminal 4xx and TTFT in stream retries - #2680
Open
1688mengdie wants to merge 3 commits into
Open
Conversation
added 3 commits
August 30, 2026 11:26
The retry loop in execute_sse_request retried every non-2xx response with the same request body, including deterministic client errors such as 400. A deterministic 4xx fails identically on every attempt, so retrying it only burns the request budget/credits and adds latency without any chance of success; the bad_requests_then_success fixture even reported a "success" after a later request happened to return 200, masking the wasted retries. Add a module-private is_transient_http_status predicate that classifies server errors (5xx), rate limiting (429), and request/gateway timeouts (408/504) as transient, and treats every other status as terminal. The non-2xx branch now breaks out of the loop for a terminal status instead of continuing; only transient statuses retry, reusing the existing retry_delay_ms / exponential backoff. The successful-response path, the transport branch, and the TTFT branch are left untouched by this commit. Test: reversed bad_requests_then_success coverage to assert terminal 400 (attempts == 1) and added is_transient_http_status classification coverage; `cargo test -p bitfun-ai-adapters --jobs 4` passes. AI: generated with review; verified with the above command.
The retry loop in execute_sse_request continued on a TTFT timeout using the same request_body. A TTFT (time-to-first-token) timeout fires after the request body has already been sent to the server and the client is waiting for the first token, so re-sending the same body in a later attempt re-sends an already-billed request (double-billing for the same logical turn). Treat a TTFT timeout as a terminal error: break out of the retry loop and surface the timeout to the caller instead of re-sending the request. The existing warn!/last_error/trace reporting is preserved, and the transport and response branches are untouched by this commit. Test: added ttft_timeout_is_terminal coverage using a hanging mock (no completed response) that asserts a single attempt and a terminal error; `cargo test -p bitfun-ai-adapters --jobs 4` passes. AI: generated with review; verified with the above command.
The Response non-2xx branch computed the retry delay and slept for transient statuses (5xx/429/408), then unconditionally fell through to break. Because the sleep was inside an if that did not jump back to the top of the loop, every non-2xx response exited the retry loop -- even transient server errors. This regressed the intended "only retry transient statuses" semantics and silently removed automatic retries for 5xx/429/408 on the aggregation/compression chains. After sleeping for a transient status, continue the retry loop so the next attempt can succeed; only deterministic 4xx (and the no-retries- remaining case) fall through to the terminal break. The TTFT branch, the transport branch, and the successful-response path are untouched by this commit. Test: added transient_server_errors_are_retried coverage using a mock that returns 503 then 200 and asserts the fixture is called more than once (attempts > 1); existing terminal assertions (4xx and TTFT timeout) still pass; cargo test -p bitfun-ai-adapters --jobs 4 sse::tests passes (16 passed; 0 failed). AI: generated with review; verified with the above command.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
execute_sse_requestretries with the samerequest_bodyin two cases thatshould be terminal: a TTFT (time-to-first-token) timeout re-sends an
already-sent (already-billed) request, and a deterministic 4xx is retried even
though it fails identically on every attempt.
Root cause
The retry decision does not distinguish terminal from transient outcomes. The
TTFT branch
continues using the same request body, and the non-2xx branchretries every status, including 400. A deterministic 4xx increases the
attempt count without any chance of success, and a TTFT re-send double-bills.
Fix
timeout instead of re-sending the request.
is_transient_http_statuspredicate that classifiesserver errors (5xx), rate limiting (429), and request/gateway timeouts
(408/504) as transient, and every other status as terminal. Only transient
statuses are retried; deterministic 4xx and the no-retries-remaining case
break out of the loop. The existing
retry_delay_ms/exponential_retry_delay_ms/retry_after_delay_msbackoff is reused.are retried rather than exited.
Note: the upstream client has no request-id/idempotency mechanism (code-level
finding): the request body is a serialized value reused across attempts. This
fix removes the re-send instead of relying on provider-side idempotency. No
request-level total timeout is added (the config default
stream_ttft_timeout_secsis
Some(600), already bounding the wait).Testing
bad_requests_then_successto assert terminal 400 (attempts == 1);added
is_transient_http_statusclassification coverage,ttft_timeout_is_terminal(hanging mock, single attempt, terminal error), and
transient_server_errors_are_retried(503 -> 200, attempts > 1).cargo test -p bitfun-ai-adapters --jobs 4 sse::tests- 16 passed, 0 failed.Closes #2677
Commit list:
ee4e20046fix(ai): don't retry deterministic 4xx statuses- addsis_transient_http_status; terminal 4xx break; reverses bad_requests coverage.5319e9c54fix(ai): don't resend requests on TTFT timeout- TtftTimeoutterminal break; adds ttft_timeout_is_terminal.
47689a040fix(ai): retry transient statuses after sleep- adds the missingcontinuefor transient statuses; adds transient_server_errors_are_retried.