Potential bugs or inefficiencies around the HTTP/3 upgrade mechanism

[passcod]

🤖 The cancellation-strike mechanism added in #23 treats a cancelled HTTP/3 attempt as weak evidence: N consecutive strikes within a rolling ~60s window demote an origin, and any successful HTTP/3 response clears the count. That reasoning holds for sequential cancellations. Review found three patterns it does not cover.

None is an outage — every failure mode is either "TCP instead of HTTP/3 for 300s" or "stays on HTTP/3 when it should have demoted" — so these are hardening items, not blockers.

1. Concurrent aborts reach the threshold instantly (false positive)

The window bounds strike spacing, not concurrency, and the success-reset only helps if a success actually interleaves. A Promise.all of four GETs to one origin under a shared AbortSignal.timeout, or a batch cancelled on user navigation, produces four strikes within microseconds with no opportunity for a reset.

A perfectly healthy origin is then demoted for upgradeFailedTtl (300s). A caller doing this every minute effectively loses HTTP/3 permanently: 300s on TCP, failed expires, the origin is re-advertised, the next burst re-demotes it.

This means the design's acceptance criterion "an origin whose h3 works normally is never demoted by interleaved caller aborts" does not hold in general — it holds for the sequential case the design reasoned about.

Two cheap hardenings, either of which closes most of it:

• Debounce: ignore a strike arriving within ~250ms of the previous one. A simultaneous burst is one event, not a run.
• Minimum in-flight duration: record Instant::now() in H3AttemptGuard::new and only strike if the attempt was in flight beyond some floor. An attempt aborted after 20ms says nothing about the UDP path; one aborted after 5s of silence does.

The second is the more principled of the two: it directly encodes "we waited long enough to have learned something".

2. Retry backoff longer than the window never demotes (false negative)

Strikes decay between attempts, so a retry loop whose backoff exceeds ~60s never accumulates a run and the default config never demotes. The originally reported incident lasted 6.5+ minutes across "multiple retry cycles"; if that consumer's backoff exceeds the window, the default settings would not have helped them.

upgradeCancelStrikes: 1 gives immediate demotion and is now documented as the answer, but the window itself is deliberately not a knob. Worth reconsidering whether it should be, or whether the strike count should decay more slowly than it accumulates.

3. Redirects misattribute strikes

reqwest follows redirects inside next.run, but the middleware holds the initial request URL. A cancellation while following a cross-origin redirect records a strike against the first origin, whose HTTP/3 attempt actually succeeded. Pre-existing behaviour for confirm_h3/record_h3_failure; new for strikes. Low impact, noted for completeness.

Suggested first step

Add the minimum-in-flight-duration floor. It is a couple of lines in H3AttemptGuard, it addresses pattern 1 directly, and it makes the "a strike means we waited and learned nothing" claim true rather than merely intended.

[passcod]

🤖 Parking this. Having worked through it, none of the three items justifies a fix now, and every fix available to us today is an uncalibrated heuristic. Recording the reasoning and what would let us do it properly.

Re-examining the concurrency item

The original framing — that four simultaneous aborts are "one event, not a run" — does not hold up. Four requests to the same origin each opened their own stream and each independently failed to produce response headers before the deadline. That is four observations, and arguably stronger evidence than four sequential failures, since it rules out a per-request fluke.

What concurrency actually breaks is narrower: the temporal premise of the threshold. The intent was "three strikes within a minute of each other implies a sustained incident" — the spacing was doing work. A burst reaches the threshold in microseconds without demonstrating anything sustained. That is a gap between intent and implementation, not a miscount.

Framed that way the harm shrinks considerably:

• Broken path + concurrent batch → demotes immediately, which is better than waiting out three sequential retries.
• Healthy origin + impatient caller (a batch under a 500ms deadline against a server that takes 800ms) → demotes a working origin for upgradeFailedTtl. But the caller then gets TCP, and since the server is the slow part, TCP is no worse. Arguably not harm at all.

The residual real cost is flapping: an origin that serves patient requests fine gets held on TCP because impatient batches keep knocking it down, re-arming every upgradeFailedTtl. Worth noting that this is inference, not an observed symptom — nobody has reported it.

Why the available fixes aren't good enough

The guard runs from a Drop impl. What can it observe? Almost nothing:

• reqwest's HTTP/3 pool is mod pool; — not even pub(crate) outward — and reqwest exposes no public API for HTTP/3 connection state.
• A Response offers version(), remote_addr() and extensions, but only on success and only after the fact.
• So at drop time we cannot know whether a QUIC connection existed, whether the handshake completed, or whether a single packet ever flowed.

That leaves elapsed wall-clock and whatever faith itself recorded. Checking the candidates against the cases:

Case	Recent h3 successes?	Elapsed vs origin's normal latency
Broken UDP path	none	far longer
Healthy origin, impatient caller, all abort	none	shorter — the caller gave up early
Healthy origin, mixed patient + impatient	yes	shorter

• An absolute in-flight floor separates none of them, because what distinguishes the cases is the caller's deadline relative to the server's legitimate response time.
• A grace period after a demonstrated success separates the mixed case, so it would fix the flapping — but flapping is the unobserved one.
• A relative latency floor (strike only when elapsed exceeds a multiple of the origin's observed h3 latency) separates all three and would make the semantics honest by construction. It needs a multiplier, and a high-water-mark-versus-EWMA choice, that we would be inventing without production data.

Shipping a heuristic tuned on guesswork risks misfiring differently rather than less, on a mechanism whose worst outcome today is "uses TCP".

The redirect item is unfixable here

AltSvcMiddleware captures the initial request URL, and reqwest follows redirects inside next.run. A cancellation therefore has no response and no way to learn which hop it reached, so a strike can only ever be attributed to the initial origin. This is fixable for confirm_h3/record_h3_failure, which do have response.url(), but that is a separate change with its own semantics and no demonstrated need.

The long-backoff item is narrower than stated

The strike window is a TTL refreshed on each strike, so strikes only need to land within 60s of each other, not inside a fixed bucket. An exponential backoff of 1,2,4,8,16,32s gives gaps of 11-26s, well inside the window. The originally reported incident (15 retries over 6.5 minutes, ~26s average gap) would have accumulated normally. It only breaks once the backoff itself exceeds 60s, and upgradeCancelStrikes: 1 already covers that.

What we would need to do this properly

In rough order of how much it would help:

1. reqwest owning Alt-Svc entirely — the reqwest#1138 endgame, where reqwest makes the upgrade decision, detects failure and falls back. faith's AltSvcCache, the Drop guard and this whole strike heuristic would go away, and with them this entire class of bug. This is the outcome to want.
2. Whether a cancelled attempt held a live pooled connection or was still connecting. This is the single most valuable signal: a cancellation while waiting on a connection that has been working says nothing about the path, whereas one stuck in the QUIC handshake is real evidence. It would replace the whole heuristic with an observation.
3. Connection liveness at cancellation time — last-activity timestamp, or the packet/RTT stats quinn already tracks via Connection::stats(). Distinguishes "path is dead" from "server is slow", which is exactly the distinction the heuristic is guessing at.
4. A disposition hook for dropped attempts, so a middleware can learn how an attempt ended rather than inferring it from a Drop impl.

None of these exist today, and 2-4 arguably shouldn't be added if 1 lands.

What would justify revisiting

• A report of HTTP/3 flapping: origins repeatedly demoted and re-armed, visible as alternating protocol versions for a healthy origin.
• A report of HTTP/3 never being used against a server that supports it, traced to spurious strikes.
• A caller that has to set upgradeCancelStrikes: 0 to get usable behaviour — that would mean the default heuristic is wrong, not just imprecise.
• reqwest gaining any of the capabilities above.

Meanwhile the escape hatches are real: upgradeCancelStrikes: 0 disables the heuristic outright, and upgradeAttemptTimeout provides a fallback path that needs no cooperation from the caller.