fix: retry transport errors wrapped in ReqLLM.Error.API.Request#797
Merged
mikehostetler merged 2 commits intoJul 6, 2026
Merged
Conversation
Transient transport failures (:closed, :timeout, :econnrefused) often
reach the retry step wrapped in a ReqLLM.Error.API.Request rather than as
a bare Req.TransportError -- e.g. a stale pooled connection closed on
reuse surfaces as
%ReqLLM.Error.API.Request{cause: %Finch.TransportError{reason: :closed}}.
should_retry?/2 only matched the bare shape, so the wrapped form fell
through to the catch-all and was never retried, defeating the step's own
stated intent for these reasons despite the max_retries: 3 default. Add a
clause that unwraps the transport cause (mirroring the existing
wrapped-429 handling just above it) and retries the same transient
reasons. The reason list is extracted into @transient_reasons so the
bare and wrapped clauses stay in sync.
mikehostetler
added
ready_to_merge
and removed
needs_work
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
2 participants
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.
Summary
ReqLLM.Step.Retry.should_retry?/2retries transient transport errors (:closed,:timeout,:econnrefused) only when they arrive as a bare%Req.TransportError{}. In practice these failures frequently reach the retry step wrapped in a%ReqLLM.Error.API.Request{}, so they fall through to the catch-all clause and are never retried — defeating the step's own stated intent (and themax_retries: 3default) for exactly the errors it's meant to handle.The most common case is a stale pooled connection that the peer closed while idle: on first reuse it surfaces as
A single such stale connection currently becomes a hard, user-facing failure even though an immediate retry on a fresh connection would succeed.
Root cause
should_retry?/2already unwraps%ReqLLM.Error.API.Request{}for the 429 case (to honorRetry-After), but there's no equivalent unwrapping for transport errors — the transport clause only matches%Req.TransportError{}, while the wrapped cause is a%Finch.TransportError{}/%Mint.TransportError{}.Fix
Add a
should_retry?/2clause that unwraps the transportcausefrom%ReqLLM.Error.API.Request{}and retries the same transient reasons — mirroring the existing wrapped-429 handling right above it. The reason list is extracted into an@transient_reasonsmodule attribute so the bare and wrapped clauses can't drift apart.Non-transient wrapped causes (e.g.
:nxdomain) and wrapped HTTP errors (429 handled above, others) are unaffected.Tests
test/req_llm/step/retry_test.exscovering the wrapped transient reasons (all{:delay, 0}) and a wrapped non-transient reason (false).test/req_llm/step/retry_test.exspasses (15 tests, 0 failures).Context
Found while hardening a downstream app against production
:closedincidents: real transport failures through the OpenRouter provider path consistently arrived wrapped, soshould_retry?/2never fired and the retry budget went unused. Verified againstreq_llm1.17.0.