add-retry-error - #4
Merged
Merged
Conversation
A retry that gave up handed back the last error and nothing else. That error can't tell you whether you burned three retries in 700ms, spent a 30s budget, or had `.when` reject the first error so nothing was ever retried. Three different incidents, identical output. Both drivers now fail with `RetryError<E>`, carrying the last error plus `attempts()`, `elapsed()`, and a `stop_reason()`. Building it lives in one shared `give_up` so the two drivers can't drift on the arithmetic. `impl Error` is bounded on `E: Debug + Display`, not `E: Error + 'static`. The two overlap, so it's one or the other (E0119), and Debug+Display covers strictly more: `String`, `Box<dyn Error>`, and `anyhow::Error` all satisfy it and none implement `Error`. Retrying an `anyhow::Result` is about as common as application code gets, and the alternative makes it un-`?`-able. The cost is no `source()`; the inner error's text still rides in `Display`. The async driver now starts its clock on the first poll rather than at `.into_future()`, matching blocking. That gap was invisible until `elapsed()` made it public, and it also stops a future parked in a `FuturesUnordered` from billing the wait to the operation. Escape hatch is one call: `.map_err(RetryError::into_error)`. Clock reads stay O(1): two with no budget regardless of retry count, and the `MaxElapsed` path reuses the read its own budget check just did. The old exact-count test is now a slope assertion over 3 and 30 retries, and the blocking mock gained the same counter so both drivers pin it. Mutation-checked: reporting retries instead of attempts, an ungated give-up event, dropping the MaxElapsed clock reuse, a wrong stop reason in either driver, elapsed measured from zero, and sampling start at conversion time all fail the suite. ADR005 records the always-wrap call, the Error bound with its coverage table, bounded Display, no PartialEq, and the clock-start move.
The old branch was based on the pre-review jitter tip, so it was missing the six commits main gained during review, and its ADR005 collided head-on with the ADR005 that shipped in 0.3.0. Rebuilt off main, carrying only the RetryError work. Its ADR is now ADR006; main's ADR005 (testability) keeps the number it shipped under. Four conflicts, all real: both sides had added an ADR005, both had added tests at the same point in the blocking module, and both had written a new crate docs section and a new index row. The blocking tests main added during review are kept and retyped for the new `call()` signature, so the driver parity that release established survives the rebuild: both drivers now carry the same four tests. CHANGELOG entry rewritten in the style 0.3.0 settled on -- what changed and what to do about it, with the reasoning in the ADR. The old branch is preserved as archive/retry-error-pre-rebuild.
Outside-in pass on 0.4.0, written as a user following the CHANGELOG. Everything the release claims holds. `?` into anyhow::Result works for io::Error, String, Box<dyn Error> and anyhow::Error itself -- the last three do not implement std::error::Error, which is the entire reason ADR006 bounds the impl on Debug + Display. downcast_ref recovers attempts and stop_reason, which is what ADR006 offers in place of source(). Both drivers verified: when() gives not_retryable with attempts 1, max_elapsed gives an elapsed strictly under the budget. Two gaps found. The upgrade guide covered `match e` but not assert_eq! on the whole Result, which is what every existing test does and which fails with a bare E0369 that does not say what to do. And keywords still lacked `jitter` -- swapped for `async`, the most generic of the five and already implied by `tokio`.
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.
No description provided.