Repository navigation
retry: keep the caller's request timeout - #152
Merged
Merged
Conversation
The retry interceptor added its attempt timeout after the caller's options, so it always replaced a timeout the caller set. Put it first, so the caller's timeout wins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
paulwe
approved these changes
Oct 6, 2026
boks1971
added a commit
that referenced
this pull request
Oct 7, 2026
) #152 put the attempt timeout before the caller's options so a caller's timeout wins. A retry interceptor nested inside another one then took the outer one's timeout for a caller's, and the outer timeout won. The outermost retry interceptor now passes the caller's own timeout down in the context. Without one, the innermost interceptor's timeout wins. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Human note: Ordering the timeout per caller's deadline makes sense, but quite a far reaching one. Should we do this?
The retry interceptor no longer replaces a timeout the caller set.
The interceptor added its own attempt timeout after the caller's options (retry.go#L42-L47). Options apply in order, so the attempt timeout always won. A caller that asked for 25 s got 3 s, 5 s, then 7 s. A caller that passed its context deadline got 3 s, and a call that was still running could then be sent again.
Production shows both. In 7 days, none of 37 million
DeleteRoomcalls took longer than 5 s, although the caller asks for 25 s.AssignJobhas a 10 s budget on the worker but times out at 3 s about 3,300 times a day, and each timeout is retried. Although there are no logs, I think a retry that arrives while the first assignment is still running can potentially start a second one?Now the attempt timeout goes first, and a caller's timeout overrides it. Callers that set no timeout keep the same behavior. Streams get the same fix.
This reaches every client that uses the retry interceptor and passes a timeout. A caller that passes its whole context deadline now gets one attempt with that deadline. A fast failure, such as
Unavailable, still retries.TestCallerTimeoutKeptsets a 1 s timeout. On main the attempts get 100, 300 and 500 ms.🤖 Generated with Claude Code