From a6799e79e247890266a38d77236cca3db2ba33cb Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 16 Jun 2026 10:36:35 -0500 Subject: [PATCH 1/2] fix(wsclient): retry repo-state probe on transient host backpressure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem The `Mirror to Freenet` workflow (and `freenet-git rescue`) failed with exit 128 on a single transient gateway error during the repo-state probe: error: every probe failed with transport/other error (1 probe(s)); first failure: current key 7Zazi...: recv: client error: error while executing operation in the network: contract queue full, try again later `contract queue full, try again later` is explicit backpressure the gateway invites us to retry, yet `get_state_with_legacy_fallback` ran the probe exactly once and bailed. `put_pack` already retried this same class of error on the write side, so the read-side probe was the lone gap — making the daily freenet-core mirror one transient blip away from a false-alarm Matrix page on every push (multiple failures observed 2026-06-15/16). ## Solution Retry the whole probe sequence up to 4 times with exponential backoff (2s/4s/8s, shared with `put_pack` via `retry_backoff`) when — and only when — *every* probe outcome is transient (a timeout, or a host error matching `is_transient_host_error`: "queue full" / "try again later" / timeouts). A single authoritative `NotFound`/`Empty` short-circuits the loop so genuine "no state on the network" still fails fast with the precise per-probe message and never masks data loss behind a delay. The probe body is extracted into `probe_all_keys` (returns the per-probe outcome list on failure) so the retry decision is made on structured outcomes, not by re-parsing the formatted error string. Scope note: the separate `put timed out` mirror failures go through `put_pack`, which already retries; tuning that budget is tracked in #53 and deliberately left out to keep this a single, low-risk change that does not risk the mirror job's 30-minute runtime budget. ## Testing 8 new unit tests pin the transient/permanent classifier, the `outcomes_all_transient` retry gate (single backpressure probe and all-timeout retry; authoritative-outcome and dead-connection short-circuit; empty-input guard), and the shared backoff schedule. `cargo fmt`, `cargo clippy -D warnings`, and `cargo test --workspace` all pass. Related: #36 (per-chunk fetch retry), #53 (put_pack rescue timeouts). [AI-assisted - Claude] --- crates/freenet-git/src/wsclient.rs | 244 ++++++++++++++++++++++++++++- 1 file changed, 240 insertions(+), 4 deletions(-) diff --git a/crates/freenet-git/src/wsclient.rs b/crates/freenet-git/src/wsclient.rs index 66d17d9..2651a55 100644 --- a/crates/freenet-git/src/wsclient.rs +++ b/crates/freenet-git/src/wsclient.rs @@ -257,6 +257,67 @@ impl ProbeOutcome { ProbeOutcome::OtherError(msg) } } + + /// True if this outcome describes a *transient* condition worth + /// retrying the whole probe sequence for. A [`Timeout`] (gateway + /// has the contract but routing was slow) and a transient host + /// backpressure [`OtherError`] (see [`is_transient_host_error`]) + /// both qualify; an authoritative [`NotFound`] / [`Empty`] or any + /// non-retryable transport error does not — retrying those just + /// burns time and would mask genuine data loss behind a delay. + fn is_transient(&self) -> bool { + match self { + ProbeOutcome::Timeout => true, + ProbeOutcome::OtherError(msg) => is_transient_host_error(msg), + ProbeOutcome::NotFound | ProbeOutcome::Empty => false, + } + } +} + +/// Classify a host/transport error string as a *transient* condition +/// the gateway itself invites us to retry, as opposed to a permanent +/// failure (authoritative NotFound, decode error, malformed request). +/// +/// The strings here are the ones freenet-core surfaces through the WS +/// client error and that `get_state`/`put_contract` forward verbatim +/// (e.g. via `recv: {e}`). "contract queue full, try again later" is +/// explicit backpressure; "timed out"/"timeout" covers the per-op and +/// "put/get timed out after N peer attempt(s)" network timeouts that +/// clear once routing settles. +/// +/// 2026-06-16 mirror incident: a single "contract queue full, try +/// again later" on the freenet-core repo-state probe failed the whole +/// `Mirror to Freenet` run with exit 128. `put_pack` already retried +/// this exact class on the write side; the read-side probe was the +/// lone gap, so the daily mirror was one transient blip away from a +/// false-alarm Matrix page on every push. +fn is_transient_host_error(msg: &str) -> bool { + let m = msg.to_ascii_lowercase(); + m.contains("queue full") + || m.contains("try again later") + || m.contains("timed out") + || m.contains("timeout") +} + +/// True if every probe outcome is transient (so retrying the whole +/// sequence could plausibly succeed). Empty input is not retryable — +/// there is nothing to indicate a transient cause. +/// +/// The rule is deliberately conservative: a single authoritative +/// NotFound/Empty or a non-retryable transport error among the +/// outcomes makes the whole set non-retryable, so we surface the +/// precise per-probe message immediately instead of delaying it behind +/// pointless retries. +fn outcomes_all_transient(outcomes: &[(String, ProbeOutcome)]) -> bool { + !outcomes.is_empty() && outcomes.iter().all(|(_, o)| o.is_transient()) +} + +/// Exponential backoff for transient-failure retries: 2s, 4s, 8s, … +/// for `attempt` 1, 2, 3. Shared by the read-side probe retry and +/// `put_pack`'s write-side retry so both paths back off identically +/// under gateway backpressure. +fn retry_backoff(attempt: u32) -> Duration { + Duration::from_secs(2u64.pow(attempt)) } /// GET the repo state at `current_id`; if not found, walk @@ -275,6 +336,17 @@ impl ProbeOutcome { /// to bail with "no state found at current contract key or any of N /// legacy keys", indistinguishable from genuine data loss. The new /// message says "all N+1 probes timed out", which is actionable. +/// +/// Transient failures are retried up to [`PROBE_MAX_ATTEMPTS`] times +/// with exponential backoff ([`retry_backoff`]). A retry fires only +/// when *every* probe outcome is transient ([`outcomes_all_transient`]): +/// gateway backpressure ("contract queue full, try again later") and +/// per-op timeouts clear on their own, so a single blip should not +/// fail the operation — `put_pack` already retried the same class on +/// the write side (2026-06-16 mirror exit-128 incident). An +/// authoritative NotFound/Empty short-circuits the retry loop so +/// genuine "no state on the network" still fails fast with the precise +/// message. pub async fn get_state_with_legacy_fallback( web_api: &mut WebApi, current_id: ContractInstanceId, @@ -282,8 +354,58 @@ pub async fn get_state_with_legacy_fallback( legacy_wasm_hashes: &[&[u8; 32]], timeout: Duration, ) -> Result { + let mut attempt = 1u32; + loop { + match probe_all_keys( + web_api, + current_id, + params_bytes, + legacy_wasm_hashes, + timeout, + ) + .await + { + Ok(found) => return Ok(found), + Err(outcomes) => { + // Stop on the final attempt, or as soon as any outcome + // is non-transient (authoritative NotFound/Empty or a + // dead-connection transport error) — retrying those + // only delays surfacing a real problem. + if attempt >= PROBE_MAX_ATTEMPTS || !outcomes_all_transient(&outcomes) { + return Err(format_fallback_failure(&outcomes, timeout)); + } + let backoff = retry_backoff(attempt); + tracing::warn!( + "repo-state probe attempt {attempt}/{PROBE_MAX_ATTEMPTS} failed \ + (transient host backpressure); retrying in {backoff:?}" + ); + tokio::time::sleep(backoff).await; + attempt += 1; + } + } + } +} + +/// Max number of times [`get_state_with_legacy_fallback`] runs the +/// full probe sequence when failures are transient. 4 attempts with +/// [`retry_backoff`] spends at most 2+4+8 = 14s sleeping across the +/// retries — cheap for a read-side probe, and well inside the mirror +/// job's 30-minute budget. +const PROBE_MAX_ATTEMPTS: u32 = 4; + +/// Run one full probe pass: try the current key, then each legacy key. +/// `Ok` on the first non-empty hit; `Err(outcomes)` with the per-probe +/// outcome list when every key fails (so the caller can decide whether +/// the failure is transient and worth retrying). +async fn probe_all_keys( + web_api: &mut WebApi, + current_id: ContractInstanceId, + params_bytes: &[u8], + legacy_wasm_hashes: &[&[u8; 32]], + timeout: Duration, +) -> std::result::Result> { // Probe outcomes, in (label, outcome) form. Filled as we go; - // formatted into the bail message if every probe fails. + // returned to the caller if every probe fails. let mut outcomes: Vec<(String, ProbeOutcome)> = Vec::new(); // Fast path: try the current key first. @@ -327,7 +449,7 @@ pub async fn get_state_with_legacy_fallback( } } - Err(format_fallback_failure(&outcomes, timeout)) + Err(outcomes) } /// Build the final error returned when every probe in @@ -727,8 +849,7 @@ pub async fn put_pack( ); last_err = Some(e); if attempt < MAX_ATTEMPTS { - let backoff = Duration::from_secs(2u64.pow(attempt)); - tokio::time::sleep(backoff).await; + tokio::time::sleep(retry_backoff(attempt)).await; } } } @@ -1419,4 +1540,119 @@ mod tests { assert!(msg.contains("1 NotFound"), "got: {msg}"); assert!(msg.contains("connection reset"), "got: {msg}"); } + + // ── transient-failure retry classification ──────────────────── + + /// Pin: the exact error the 2026-06-16 `Mirror to Freenet` + /// exit-128 surfaced on the repo-state probe must classify as + /// transient so the probe retries instead of failing the whole + /// mirror. Also covers the "put timed out after N peer attempt(s)" + /// network timeout (the other observed mirror failure mode). + #[test] + fn is_transient_host_error_recognizes_backpressure_and_timeouts() { + // Verbatim from the failed run's "Push to Freenet" step. + assert!(is_transient_host_error( + "recv: client error: error while executing operation in the network: \ + contract queue full, try again later" + )); + assert!(is_transient_host_error( + "recv: client error: error while executing operation in the network: \ + put timed out after 1 peer attempt(s) (0 infra-retries on same peer)" + )); + assert!(is_transient_host_error("operation timeout elapsed")); + // Case-insensitive (gateway wording may change capitalisation). + assert!(is_transient_host_error("Contract Queue Full")); + } + + /// Permanent failures must NOT be classified as transient — a + /// retry would just delay surfacing a real problem (data loss or a + /// dead connection) by the full backoff budget. + #[test] + fn is_transient_host_error_rejects_permanent_failures() { + assert!(!is_transient_host_error( + "contract 3iBuNbXTrXz... not found on the network" + )); + assert!(!is_transient_host_error("send GET: connection reset")); + assert!(!is_transient_host_error( + "decode error: invalid state bytes" + )); + } + + /// `outcomes_all_transient` gates the probe retry. The 2026-06-16 + /// incident was a single current-key probe (0 legacy keys) that + /// failed transiently — it MUST be retryable. + #[test] + fn outcomes_all_transient_retries_single_backpressure_probe() { + let outcomes = vec![( + "current key X".to_string(), + ProbeOutcome::OtherError( + "recv: client error: error while executing operation in the network: \ + contract queue full, try again later" + .to_string(), + ), + )]; + assert!(outcomes_all_transient(&outcomes)); + } + + /// All-timeout (the classic transient gateway slowdown) is + /// retryable across the current + legacy probes. + #[test] + fn outcomes_all_transient_retries_all_timeouts() { + let outcomes = vec![ + ("current key X".to_string(), ProbeOutcome::Timeout), + ("legacy key 0 (Y)".to_string(), ProbeOutcome::Timeout), + ]; + assert!(outcomes_all_transient(&outcomes)); + } + + /// A single authoritative NotFound (or Empty) anywhere in the set + /// makes the whole probe non-retryable: the contract genuinely + /// resolved with no state, so we must fail fast with the precise + /// message rather than mask data loss behind retries. + #[test] + fn outcomes_all_transient_short_circuits_on_authoritative_outcome() { + let with_not_found = vec![ + ("current key X".to_string(), ProbeOutcome::Timeout), + ("legacy key 0 (Y)".to_string(), ProbeOutcome::NotFound), + ]; + assert!(!outcomes_all_transient(&with_not_found)); + + let with_empty = vec![ + ( + "current key X".to_string(), + ProbeOutcome::OtherError("contract queue full, try again later".to_string()), + ), + ("legacy key 0 (Y)".to_string(), ProbeOutcome::Empty), + ]; + assert!(!outcomes_all_transient(&with_empty)); + } + + /// A non-retryable transport error (e.g. a dead connection) is not + /// transient, so a probe set containing one is not retried. + #[test] + fn outcomes_all_transient_rejects_non_retryable_transport_error() { + let outcomes = vec![( + "current key X".to_string(), + ProbeOutcome::OtherError("send GET: connection reset".to_string()), + )]; + assert!(!outcomes_all_transient(&outcomes)); + } + + /// Empty input is never retryable — there is nothing to suggest a + /// transient cause (defends the `!outcomes.is_empty()` guard, which + /// otherwise lets `Iterator::all` vacuously return true). + #[test] + fn outcomes_all_transient_empty_is_not_retryable() { + assert!(!outcomes_all_transient(&[])); + } + + /// `retry_backoff` is the exponential 2s/4s/8s schedule shared by + /// the probe retry and `put_pack`. Pinned so a change to either + /// retry site keeps a single, predictable backoff policy. + #[test] + fn retry_backoff_is_exponential() { + assert_eq!(retry_backoff(1), Duration::from_secs(2)); + assert_eq!(retry_backoff(2), Duration::from_secs(4)); + assert_eq!(retry_backoff(3), Duration::from_secs(8)); + } } From 1993e15af80a9e5990245f525e288b6410126a6e Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 16 Jun 2026 10:54:03 -0500 Subject: [PATCH 2/2] =?UTF-8?q?fix(wsclient):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20retry=20transient=20legacy=20probe=20after=20curren?= =?UTF-8?q?t=20NotFound?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings from PR #56 (Codex + adversarial Claude reviewers): - Codex P2 (correctness): the "all outcomes transient" gate failed to retry a transient legacy-key probe when the current key was authoritatively NotFound — the normal migration/fallback case. The current key's NotFound says nothing about whether a legacy key holds the state, so a transient blip on that legacy probe was wrongly treated as fatal. Replaced `outcomes_all_transient` with `outcomes_worth_retrying` (3-way `RetryDisposition`): retry when ANY outcome is transient and NONE is a hard transport error; NotFound/ Empty are per-key authoritative and no longer block a sibling retry. A dead-connection HardError still aborts fast. New regression test `outcomes_worth_retrying_retries_transient_legacy_after_current_not_found`. - Doc accuracy (MAJOR): the PROBE_MAX_ATTEMPTS comment claimed "at most 14s". Timeout outcomes are also retryable and each burns a full per-op timeout, so the real worst case is ~`attempts × (1+legacy) × timeout`. Comment now states the true budget (~12 min for freenet-core, L=0) and flags the legacy-key / chunked-put_pack scaling. - warn! message no longer hardcodes "backpressure" (also fires on plain timeouts): now "transient: host backpressure or timeout". - retry_backoff uses saturating_pow + a MAX_BACKOFF_SECS=60 cap so a future attempt-ceiling bump can't overflow/panic; values 2/4/8 for the current callers are unchanged. Pinned by an extended backoff test. - Documented that is_transient_host_error's substring match is intentionally broad, and corrected put_pack's stale "retries on transient host errors" doc (it retries on any error, safe because re-PUTs are idempotent). [AI-assisted - Claude] --- crates/freenet-git/src/wsclient.rs | 256 ++++++++++++++++++++--------- 1 file changed, 177 insertions(+), 79 deletions(-) diff --git a/crates/freenet-git/src/wsclient.rs b/crates/freenet-git/src/wsclient.rs index 2651a55..1526b73 100644 --- a/crates/freenet-git/src/wsclient.rs +++ b/crates/freenet-git/src/wsclient.rs @@ -258,22 +258,45 @@ impl ProbeOutcome { } } - /// True if this outcome describes a *transient* condition worth - /// retrying the whole probe sequence for. A [`Timeout`] (gateway - /// has the contract but routing was slow) and a transient host - /// backpressure [`OtherError`] (see [`is_transient_host_error`]) - /// both qualify; an authoritative [`NotFound`] / [`Empty`] or any - /// non-retryable transport error does not — retrying those just - /// burns time and would mask genuine data loss behind a delay. - fn is_transient(&self) -> bool { + /// How this single outcome bears on whether the *whole* probe + /// sequence is worth retrying. See [`RetryDisposition`]. + fn retry_disposition(&self) -> RetryDisposition { match self { - ProbeOutcome::Timeout => true, - ProbeOutcome::OtherError(msg) => is_transient_host_error(msg), - ProbeOutcome::NotFound | ProbeOutcome::Empty => false, + ProbeOutcome::Timeout => RetryDisposition::Transient, + ProbeOutcome::OtherError(msg) if is_transient_host_error(msg) => { + RetryDisposition::Transient + } + // A non-transient transport error (dead connection, send + // failure) will not clear on a retry over the same WebApi. + ProbeOutcome::OtherError(_) => RetryDisposition::HardError, + // Authoritative for THIS key only. + ProbeOutcome::NotFound | ProbeOutcome::Empty => RetryDisposition::Authoritative, } } } +/// How a single [`ProbeOutcome`] bears on whether the whole probe +/// sequence is worth retrying ([`outcomes_worth_retrying`]). +#[derive(Debug, PartialEq, Eq)] +enum RetryDisposition { + /// A timeout or host backpressure — a retry of this key could + /// plausibly succeed. At least one of these makes the sequence + /// retryable. + Transient, + /// A hard transport error (dead connection, send failure) that + /// will not clear on a retry over the same `WebApi`. Its presence + /// aborts the retry loop so we fail fast instead of burning the + /// backoff budget on a connection that is already gone. + HardError, + /// Authoritative for THIS key only (`NotFound` / `Empty`). Says + /// nothing about sibling keys, so it neither triggers nor blocks a + /// retry of a sibling that failed transiently. This is the + /// migration/legacy-fallback case: the current key is genuinely + /// absent while a legacy key transiently times out — that legacy + /// probe is exactly the one worth retrying (freenet-git#56 review). + Authoritative, +} + /// Classify a host/transport error string as a *transient* condition /// the gateway itself invites us to retry, as opposed to a permanent /// failure (authoritative NotFound, decode error, malformed request). @@ -291,6 +314,13 @@ impl ProbeOutcome { /// this exact class on the write side; the read-side probe was the /// lone gap, so the daily mirror was one transient blip away from a /// false-alarm Matrix page on every push. +/// +/// The match is deliberately broad (free-form substring, not bound to +/// a stdlib enum the way `from_get_state_err`'s `const` prefixes are): +/// none of the *permanent* error strings freenet-core surfaces today +/// contain these tokens, and a rare false-transient costs only one +/// extra backoff cycle before the same error resurfaces — cheaper than +/// silently dropping a genuinely-retryable variant whose wording drifts. fn is_transient_host_error(msg: &str) -> bool { let m = msg.to_ascii_lowercase(); m.contains("queue full") @@ -299,27 +329,47 @@ fn is_transient_host_error(msg: &str) -> bool { || m.contains("timeout") } -/// True if every probe outcome is transient (so retrying the whole -/// sequence could plausibly succeed). Empty input is not retryable — -/// there is nothing to indicate a transient cause. +/// True if the probe sequence is worth retrying: at least one outcome +/// is [`Transient`](RetryDisposition::Transient) **and** none is a +/// [`HardError`](RetryDisposition::HardError). Empty input is not +/// retryable (no transient cause to recover from). /// -/// The rule is deliberately conservative: a single authoritative -/// NotFound/Empty or a non-retryable transport error among the -/// outcomes makes the whole set non-retryable, so we surface the -/// precise per-probe message immediately instead of delaying it behind -/// pointless retries. -fn outcomes_all_transient(outcomes: &[(String, ProbeOutcome)]) -> bool { - !outcomes.is_empty() && outcomes.iter().all(|(_, o)| o.is_transient()) +/// Authoritative [`NotFound`]/[`Empty`] outcomes are per-key and do +/// **not** block a retry: when the current key is genuinely absent but +/// a legacy key times out, that legacy probe is precisely the one +/// worth retrying — the current key's NotFound says nothing about the +/// legacy key (freenet-git#56 Codex review). A `HardError` (dead +/// connection) still aborts, since retrying the same socket is futile. +fn outcomes_worth_retrying(outcomes: &[(String, ProbeOutcome)]) -> bool { + let mut any_transient = false; + for (_, outcome) in outcomes { + match outcome.retry_disposition() { + RetryDisposition::HardError => return false, + RetryDisposition::Transient => any_transient = true, + RetryDisposition::Authoritative => {} + } + } + any_transient } -/// Exponential backoff for transient-failure retries: 2s, 4s, 8s, … -/// for `attempt` 1, 2, 3. Shared by the read-side probe retry and -/// `put_pack`'s write-side retry so both paths back off identically -/// under gateway backpressure. +/// Exponential backoff for transient-failure retries: 2s, 4s, 8s for +/// `attempt` 1, 2, 3, capped at [`MAX_BACKOFF_SECS`]. Shared by the +/// read-side probe retry and `put_pack`'s write-side retry so both +/// paths back off identically under gateway backpressure. +/// +/// `saturating_pow` + the cap keep this safe (no overflow, no absurd +/// sleep) if a caller ever raises its attempt ceiling; the current +/// callers only reach `attempt` 3, well below the cap. fn retry_backoff(attempt: u32) -> Duration { - Duration::from_secs(2u64.pow(attempt)) + Duration::from_secs(2u64.saturating_pow(attempt).min(MAX_BACKOFF_SECS)) } +/// Upper bound on a single [`retry_backoff`] sleep. 60s is far above +/// the 8s the current callers reach, so it changes no present +/// behavior — it only defuses overflow/absurd-sleep if an attempt +/// ceiling is raised later. +const MAX_BACKOFF_SECS: u64 = 60; + /// GET the repo state at `current_id`; if not found, walk /// `legacy_wasm_hashes`, computing the legacy contract key for the /// same `params_bytes` and probing each. Returns the first state we @@ -338,15 +388,16 @@ fn retry_backoff(attempt: u32) -> Duration { /// message says "all N+1 probes timed out", which is actionable. /// /// Transient failures are retried up to [`PROBE_MAX_ATTEMPTS`] times -/// with exponential backoff ([`retry_backoff`]). A retry fires only -/// when *every* probe outcome is transient ([`outcomes_all_transient`]): -/// gateway backpressure ("contract queue full, try again later") and -/// per-op timeouts clear on their own, so a single blip should not -/// fail the operation — `put_pack` already retried the same class on -/// the write side (2026-06-16 mirror exit-128 incident). An -/// authoritative NotFound/Empty short-circuits the retry loop so -/// genuine "no state on the network" still fails fast with the precise -/// message. +/// with exponential backoff ([`retry_backoff`]). A retry fires when at +/// least one probe outcome is transient and none is a hard transport +/// error ([`outcomes_worth_retrying`]): gateway backpressure +/// ("contract queue full, try again later") and per-op timeouts clear +/// on their own, so a single blip should not fail the operation — +/// `put_pack` already retried the same class on the write side +/// (2026-06-16 mirror exit-128 incident). An all-authoritative +/// NotFound/Empty result fails fast (genuine "no state on the +/// network"), and a dead-connection transport error aborts the retry +/// loop rather than burning the backoff budget on a gone socket. pub async fn get_state_with_legacy_fallback( web_api: &mut WebApi, current_id: ContractInstanceId, @@ -367,17 +418,17 @@ pub async fn get_state_with_legacy_fallback( { Ok(found) => return Ok(found), Err(outcomes) => { - // Stop on the final attempt, or as soon as any outcome - // is non-transient (authoritative NotFound/Empty or a - // dead-connection transport error) — retrying those - // only delays surfacing a real problem. - if attempt >= PROBE_MAX_ATTEMPTS || !outcomes_all_transient(&outcomes) { + // Stop on the final attempt, or when the failure is not + // worth retrying (all-authoritative NotFound/Empty, or a + // dead-connection transport error) — retrying those only + // delays surfacing a real problem. + if attempt >= PROBE_MAX_ATTEMPTS || !outcomes_worth_retrying(&outcomes) { return Err(format_fallback_failure(&outcomes, timeout)); } let backoff = retry_backoff(attempt); tracing::warn!( "repo-state probe attempt {attempt}/{PROBE_MAX_ATTEMPTS} failed \ - (transient host backpressure); retrying in {backoff:?}" + (transient: host backpressure or timeout); retrying in {backoff:?}" ); tokio::time::sleep(backoff).await; attempt += 1; @@ -387,10 +438,19 @@ pub async fn get_state_with_legacy_fallback( } /// Max number of times [`get_state_with_legacy_fallback`] runs the -/// full probe sequence when failures are transient. 4 attempts with -/// [`retry_backoff`] spends at most 2+4+8 = 14s sleeping across the -/// retries — cheap for a read-side probe, and well inside the mirror -/// job's 30-minute budget. +/// full probe sequence when failures are transient. +/// +/// Backoff sleeps add at most 2+4+8 = 14s across the 4 attempts. The +/// dominant cost, though, is the probes themselves: a `Timeout` +/// outcome is retryable but each timed-out probe burns a full per-op +/// `timeout` (default 180s), and every attempt re-probes the current +/// key plus all `legacy_wasm_hashes`. So the all-timeout worst case is +/// roughly `PROBE_MAX_ATTEMPTS × (1 + legacy_keys) × timeout + 14s`, +/// NOT 14s. With freenet-core's mirror (0 legacy keys) that is +/// `4 × 180 + 14 ≈ 12 min`, safely inside the job's 30-minute cap. If +/// `legacy_wasm_hashes` grows (or this constant is raised), revisit +/// that budget — and the push path's `put_pack` retries run *after* +/// this probe within the same job. const PROBE_MAX_ATTEMPTS: u32 = 4; /// Run one full probe pass: try the current key, then each legacy key. @@ -810,11 +870,14 @@ pub async fn update_state( /// `validate_state` enforces `BLAKE3(state) == parameters` so any peer /// can verify content addressing without a signature. /// -/// Retries up to 3 times with exponential backoff on transient host -/// errors. Pack contracts are content-addressed, so retries are -/// idempotent: a second PUT of the same bytes resolves to the same -/// contract key, and the contract's `update_state` accepts a no-op -/// re-PUT of identical canonical bytes. +/// Retries up to 3 times with exponential backoff ([`retry_backoff`]) +/// on *any* error — unlike the read-side probe, which retries only +/// transient outcomes, this write path retries unconditionally because +/// pack contracts are content-addressed and re-PUTs are idempotent: a +/// second PUT of the same bytes resolves to the same contract key, and +/// the contract's `update_state` accepts a no-op re-PUT of identical +/// canonical bytes. So a retry is always safe and never wrong, even if +/// the error turns out to be permanent (it just fails again). pub async fn put_pack( web_api: &mut WebApi, pack_wasm: &[u8], @@ -1578,11 +1641,11 @@ mod tests { )); } - /// `outcomes_all_transient` gates the probe retry. The 2026-06-16 + /// `outcomes_worth_retrying` gates the probe retry. The 2026-06-16 /// incident was a single current-key probe (0 legacy keys) that /// failed transiently — it MUST be retryable. #[test] - fn outcomes_all_transient_retries_single_backpressure_probe() { + fn outcomes_worth_retrying_retries_single_backpressure_probe() { let outcomes = vec![( "current key X".to_string(), ProbeOutcome::OtherError( @@ -1591,68 +1654,103 @@ mod tests { .to_string(), ), )]; - assert!(outcomes_all_transient(&outcomes)); + assert!(outcomes_worth_retrying(&outcomes)); } /// All-timeout (the classic transient gateway slowdown) is /// retryable across the current + legacy probes. #[test] - fn outcomes_all_transient_retries_all_timeouts() { + fn outcomes_worth_retrying_retries_all_timeouts() { let outcomes = vec![ ("current key X".to_string(), ProbeOutcome::Timeout), ("legacy key 0 (Y)".to_string(), ProbeOutcome::Timeout), ]; - assert!(outcomes_all_transient(&outcomes)); + assert!(outcomes_worth_retrying(&outcomes)); } - /// A single authoritative NotFound (or Empty) anywhere in the set - /// makes the whole probe non-retryable: the contract genuinely - /// resolved with no state, so we must fail fast with the precise - /// message rather than mask data loss behind retries. + /// Regression for the freenet-git#56 Codex finding: when the + /// current key is authoritatively NotFound (the normal migration + /// case) but a legacy key fails transiently, the legacy probe is + /// exactly the one worth retrying — the current key's NotFound says + /// nothing about whether the legacy key holds the state. The + /// authoritative NotFound must NOT short-circuit the retry. #[test] - fn outcomes_all_transient_short_circuits_on_authoritative_outcome() { - let with_not_found = vec![ - ("current key X".to_string(), ProbeOutcome::Timeout), - ("legacy key 0 (Y)".to_string(), ProbeOutcome::NotFound), + fn outcomes_worth_retrying_retries_transient_legacy_after_current_not_found() { + let legacy_timeout = vec![ + ("current key X".to_string(), ProbeOutcome::NotFound), + ("legacy key 0 (Y)".to_string(), ProbeOutcome::Timeout), ]; - assert!(!outcomes_all_transient(&with_not_found)); + assert!(outcomes_worth_retrying(&legacy_timeout)); - let with_empty = vec![ + let legacy_backpressure = vec![ + ("current key X".to_string(), ProbeOutcome::NotFound), ( - "current key X".to_string(), + "legacy key 0 (Y)".to_string(), ProbeOutcome::OtherError("contract queue full, try again later".to_string()), ), + ]; + assert!(outcomes_worth_retrying(&legacy_backpressure)); + } + + /// An all-authoritative result (every key resolved as NotFound / + /// Empty, no transient anywhere) is NOT retried: the state is + /// genuinely absent, so we fail fast with the precise message + /// rather than mask data loss behind retries. + #[test] + fn outcomes_worth_retrying_fails_fast_on_all_authoritative() { + let outcomes = vec![ + ("current key X".to_string(), ProbeOutcome::NotFound), ("legacy key 0 (Y)".to_string(), ProbeOutcome::Empty), ]; - assert!(!outcomes_all_transient(&with_empty)); + assert!(!outcomes_worth_retrying(&outcomes)); } - /// A non-retryable transport error (e.g. a dead connection) is not - /// transient, so a probe set containing one is not retried. + /// A hard transport error (dead connection) aborts the retry even + /// when a transient sibling is present: retrying over the same gone + /// socket is futile and would just burn the backoff budget before + /// failing anyway. #[test] - fn outcomes_all_transient_rejects_non_retryable_transport_error() { - let outcomes = vec![( + fn outcomes_worth_retrying_aborts_on_hard_transport_error() { + let alone = vec![( "current key X".to_string(), ProbeOutcome::OtherError("send GET: connection reset".to_string()), )]; - assert!(!outcomes_all_transient(&outcomes)); + assert!(!outcomes_worth_retrying(&alone)); + + let with_transient_sibling = vec![ + ("current key X".to_string(), ProbeOutcome::Timeout), + ( + "legacy key 0 (Y)".to_string(), + ProbeOutcome::OtherError("send GET: connection reset".to_string()), + ), + ]; + assert!(!outcomes_worth_retrying(&with_transient_sibling)); } /// Empty input is never retryable — there is nothing to suggest a - /// transient cause (defends the `!outcomes.is_empty()` guard, which - /// otherwise lets `Iterator::all` vacuously return true). + /// transient cause (defends the "at least one transient" rule from + /// treating "no outcomes" as retryable). #[test] - fn outcomes_all_transient_empty_is_not_retryable() { - assert!(!outcomes_all_transient(&[])); + fn outcomes_worth_retrying_empty_is_not_retryable() { + assert!(!outcomes_worth_retrying(&[])); } /// `retry_backoff` is the exponential 2s/4s/8s schedule shared by - /// the probe retry and `put_pack`. Pinned so a change to either - /// retry site keeps a single, predictable backoff policy. + /// the probe retry and `put_pack`, capped at `MAX_BACKOFF_SECS`. + /// Pinned so a change to either retry site keeps a single, + /// predictable backoff policy, and so the cap (overflow/absurd-sleep + /// guard) can't silently regress. #[test] - fn retry_backoff_is_exponential() { + fn retry_backoff_is_exponential_and_capped() { assert_eq!(retry_backoff(1), Duration::from_secs(2)); assert_eq!(retry_backoff(2), Duration::from_secs(4)); assert_eq!(retry_backoff(3), Duration::from_secs(8)); + // Above the cap, and far past the u64::pow overflow point, the + // backoff saturates instead of panicking/wrapping. + assert_eq!(retry_backoff(40), Duration::from_secs(MAX_BACKOFF_SECS)); + assert_eq!( + retry_backoff(u32::MAX), + Duration::from_secs(MAX_BACKOFF_SECS) + ); } }