fix(media): coalesce forced refreshes and preserve auth on expired downloads

Merged
JL
24d ago
+507-210

Fixes #1658.

Concurrent forced media-connection refreshes currently bypass the shared flight. A download rejected with 403/404/410 also clears the cached connection, so unrelated uploads must acquire credentials again. This patch shares pending forced refreshes and returns those terminal CDN refusals without refreshing or invalidating the connection.

Flights are scoped to the connection generation. A retired flight cannot supply or publish credentials for a new connection, and dropping its lease cannot remove a newer flight. Force still bypasses a completed cached value. Auth invalidation retires the earlier pending query and fences out its publication, so upload recovery cannot reuse that query. Further concurrent invalidations of the empty cache share the replacement. Bounded download 401 recovery remains.

Regression evidence

On main 39d3b776531230abf2f22e2c7b8544df6debc291, with only the new fixtures added:

  • forced_refresh_burst_shares_one_iq fails with 8 wire IQs, expected 1. Every caller reaches the wire or joins the pending flight before responses are released.
  • expired_download_burst_preserves_upload_connection fails with MediaSession { force_refresh: true, source: NotConnected } instead of the original CDN 403. The fixture gates four downloads, mixes buffered/writer APIs over both buffered and streaming HTTP, and starts a sticker upload using the same cached connection. It covers 403/404/410, retained auth, one CDN attempt per download, and upload success.

Additional deterministic tests cover shared refresh refusal and retry, usable cached credentials during/after refresh failure, cancellation of leaders and waiters, sequential force, reconnect isolation, retirement of a pre-rejection IQ, and suppression of its publication if the retry is cancelled before starting. Existing 401 recovery and typed-error tests remain, with fixtures updated where they previously required retrying terminal statuses.

Protocol evidence

Static inspection on 2026-10-05, not an authenticated run:

  • whatspec snapshot, schema 4.3.0, WA 2.3000.1047483476.
  • Exact archive, SHA-256 22243b9c2ff18a66cea8fd66551821fe0f9f09786d107e2be8284637db86c349, verified against the release digest. All 579 files matched the lock by SHA-256 and size; set hash 05609307e68b0f6ccfd2a121a573049999e38b752cfe7155ebbe5c661805751b.
  • Full modules extracted with Acorn; one definition of each inspected module. In 4oJRvhb3yiE.js, SHA-256 f588ee2f033b87e018a3be34a37ec1347d8b6b8700de539e6be31481b29c4690: WAWebMediaHosts bytes 442182–447826 routes forced and ordinary queries through the same WAMemoizeConcurrent call. WAWebMmsClientMmsDownload bytes 844506–849931 classifies expired 403 and 404/410 as not found; other 403s are forbidden. WAWebMmsClient bytes 849933–857443 does not force a refresh on those failures.
  • In the same WAWebMediaHosts module, the E507 handler restores the previous connection and schedules another query using the server-provided backoff; forced refresh is suppressed during that interval. This PR does not implement E507 scheduling.
  • WAWebMmsClientIsErrorRetryable, bytes 939923–940475 in bundle SHA-256 09d91fdc4089c50ab1d35dd6c796ee82ef4c3296710316d20383c85738b871b8, excludes those terminal errors. WAMemoizeConcurrent, bytes 1203677–1204378 in bundle SHA-256 082e0ef2faaa6247e6e17a225bb1685b7e583e946f7b0dc17ee58c431c1a07a8, shares the pending promise.

Limits and validation

The client defect is reproducible. The production 429's precise request and server threshold remain unconfirmed. The linked ZapFast reproduction also has consumer-side picker concurrency and re-upload behavior; this patch does not eliminate real server rate limits, add speculative 429 backoff, or claim complete Web retry parity. Web classifies 401 as retryable and obtains hosts without forcing a refresh on the next attempt; the Rust library's bounded forced 401 recovery is deliberately retained here, rather than presented as Web parity. The auth-invalidation fence preserves the library's recovery boundary; it is not a claim that Web applies that additional fence.

Local validation on 2967f39d: 2,528 library tests passed with 3 existing ignored tests (--test-threads=2), 50 doctests passed with 15 existing ignored examples, touched-crate Clippy with -D warnings passed, the published-crate MSRV check at Rust 1.94.1 passed, and the no-default-features WASM release build passed. The informational semver job reports the same 24/3/7 major-check failures in unchanged wacore/wacore-binary/waproto as base f1080df1 (base job, PR job); no semver configuration was changed. Current-head CI has passed the native suite (6,972 tests; 38 existing skips), doctests, E2E (168 tests; 11 existing skips), all crate feature matrices, MSRV, independent native/MSRV API consumers, WASM release, Clippy, Rustdoc, generated-artifact checks, Miri, Cargo Deny, and CodSpeed. The binary-size gate passed against validated main f1080df1: stripped size +4 KiB (+0.04%), no budget override. Greptile and CodeRabbit completed review of 2967f39d with no remaining findings; CodeRabbit approved it. The Codex code review triggered by marking ready completed with no findings. The separate Codex security review could not run because the paying account reached its usage limit. The all-features job also passed build, Clippy, and all three test groups (2,593 + 308 + 3,162 tests; 8 existing skips). All required checks are complete on 2967f39d; the only failed check is the documented pre-existing informational semver advisory. No unresolved review threads remain. No new dependency, public API, generic framework, or check-budget changes.

Only src/mediaconn.rs and src/download.rs change. A full git merge-tree --write-tree integration against #1656 at c42dd44c59acff7825eae7d09e940a08c71d91a2 succeeds without conflicts, producing tree b9e11eec099f5db8cdead9105e1224dd90a30324. In a detached worktree containing that combined tree, all 46 download tests and 12 media-connection tests pass. This is targeted integration validation, not a full combined-tree CI run; the integration exercise did not alter either branch or perform a merge. Diagnostics and validation-before-IQ/HTTP are preserved.

A contributor's account comparison reports that the narrower download-only policy from closed #1660 avoided 429s and allowed a sticker send in one run. Its retry policy matches this PR's 401-only refresh behavior. This independently reported observation supports the download change; it does not establish the exact server throttle mechanism or invalidate the separate deterministic forced-refresh concurrency regression. No additional protocol or E507 scope is inferred.