feat(http): add an opt-in retry policy - #54
Merged
Conversation
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
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.
Closes #35. Adds ADR-0009, which amends ADR-0006 rather than quietly contradicting it.
The decision the issue asked to settle first
timeoutMsstays the budget for a single attempt. Each attempt gets the full amount, andretry.maxElapsedMsis there for callers who want a ceiling on the whole sequence.Reinterpreting it as an overall deadline was rejected for two reasons. It would silently redefine an existing option, so identical code would behave differently after an upgrade and nobody would find out until the day a request was slow. And it would shrink each attempt's budget as attempts accumulate, so the final attempt, made under the worst conditions, would get the least time to succeed. A per-attempt budget also matches what a caller means when they write
timeoutMs: a single request must not hang for more than this.What is retried
ping,query, andqueryTable. Writes only withretryWrites, which is the explicit acceptance of duplication risk that ADR-0006 required.executeis never retried, whatever the configuration.ADR-0006 rejected "retry reads, never writes" because
POST /api/v1/sqlcarries bothSELECTandINSERTand the client cannot tell them apart without parsing SQL. That is still true and this does not solve it by parsing. It uses the method the caller chose as the declaration:queryandqueryTablereturn rows and are treated as reads,executeexists for statements whose point is their effect. A caller who sends anINSERTthroughqueryhas miscategorised it, and the TSDoc on both methods and the README say so plainly. That is a documented judgement about intent rather than a guarantee, and the ADR records it as a cost rather than pretending otherwise.Which failures
Timeouts, network failures, HTTP 429, and 5xx other than 501. Rejected credentials, malformed SQL, an oversized payload, and a caller abort are final, because the server will decide them the same way next time. 501 is excluded because a server that will never implement an endpoint will not implement it in 200 ms.
Backoff and cancellation
Exponential from
initialMs, capped atmaxMs, full jitter on by default so a fleet that failed together does not return in lockstep. ARetry-Afterheader wins over the computed delay, since the server knows when it will be ready, but is still capped bymaxMsso a mistaken or hostile value cannot park the caller.Retry-Afteris carried out of band in aWeakMaprather than bolted onto the error classes, so those stay a description of what went wrong instead of a channel for transport bookkeeping.An
AbortSignalends the sequence immediately, including mid-backoff, and the abort reason propagates rather than being swallowed by a sleeping client. Backoff timers are unrefed, so a pending retry never keeps a process alive.Tests
56 unit tests across the policy itself, the transport loop, and the client wiring: default-off, per-method retryability, the write opt-in, attempt exhaustion, the elapsed ceiling, abort during backoff,
Retry-Afterhandling including nonsense values, and the fact that the last failure propagates rather than the first.Three integration tests run against a live server through a fetch that fails the first N calls the way a dropped connection does and then delegates to the real one, so a recovered request is proved to return real rows rather than a mock's idea of them.