fix(transport): run the HTTP/1.1 fallback once, whichever branch a request took - #110
Open
burruplambert wants to merge 1 commit into
Open
fix(transport): run the HTTP/1.1 fallback once, whichever branch a request took#110burruplambert wants to merge 1 commit into
burruplambert wants to merge 1 commit into
Conversation
…branch a request took
In auto mode, presets with H3 support go through raceH3H2, and raceH3H2
carried its own copies of the two fallbacks doAuto already has: HTTP/1.1 on
the connection that ALPN downgraded, then HTTP/1.1 on a fresh connection. When
one of those internal fallbacks failed, doAuto saw an ordinary (non-ALPN) error
and ran its own fresh-connection fallback on top. So against a failing host an
H3-capable preset made two HTTP/1.1 attempts where a preset without H3 support
made one, and a request whose ALPN-negotiated HTTP/1.1 exchange was cut off
was quietly re-sent on a new connection for one family of presets and returned
as an error for the other. None of that is documented (the auto-negotiation
page describes one fallback), and retries are what WithRetry is for.
raceH3H2 now does what its name says: race the probes, make one attempt on the
winner, return whatever that attempt returned, including an *ALPNMismatchError
with its live TLS connection. doAuto owns every fallback. Its existing ALPN
handler right after the race call, which was unreachable in practice because
raceH3H2 consumed the ALPN error itself, now serves those, and its bottom
fresh-connection fallback runs once. Both doAuto branches make identical
attempts. The ProtocolAuto "nothing learned" signal introduced by the previous
commit is gone with the fallback that needed it.
Behaviour change, deliberately, on failing hosts only:
- one fewer HTTP/1.1 attempt before the error comes back (H3-capable
presets: 3 connections instead of 4 against a host that drops every
connection; presets without H3: 2, unchanged);
- a failed request on a live ALPN-negotiated HTTP/1.1 connection is returned
to the caller instead of being retried on a fresh connection (1 connection
instead of 2; presets without H3 already behaved this way).
Requests that succeed today succeed the same way. Anyone relying on the
accidental second attempt has WithRetry, which is deliberate and
preset-independent.
Tests (transport/fallback_once_test.go), against real in-process servers,
counting accepted TCP connections:
- TestAutoFreshH1FallbackRunsOnce: a server that drops every connection;
firefox-148 sees 2 attempts and chrome-146 sees 3 (one extra for the H2
probe). Previously chrome-146 made 4.
- TestAutoALPNDowngradeFailureIsNotRetried: an http/1.1-only server that
hijacks and closes without answering; both presets see exactly 1
connection, the error is returned, nothing is cached. Previously
chrome-146 made 2 and succeeded on the second.
The protocol-cache tests from the previous commit still pass unchanged.
|
@burruplambert is attempting to deploy a commit to the sardanioss' projects Team on Vercel. A member of the Team first needs to authorize it. |
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.
In auto mode, presets with H3 support go through raceH3H2, which carried its own copies of the two HTTP/1.1 fallbacks doAuto already has. When one of those internal fallbacks failed, doAuto saw an ordinary error and ran its own fresh connection fallback on top. Against a failing host an H3 capable preset made two HTTP/1.1 attempts where a preset without H3 support made one, and a request cut off on an ALPN negotiated HTTP/1.1 connection was quietly re-sent for one family of presets and returned as an error for the other. The auto negotiation docs describe one fallback, and retries are what WithRetry is for.
raceH3H2 now does what its name says: race the probes, make one attempt on the winner, return what that attempt returned, including an ALPNMismatchError with its live TLS connection. doAuto owns every fallback, and both of its branches make identical attempts.
Behaviour change on failing hosts only:
Requests that succeed today succeed the same way.
Tests in transport/fallback_once_test.go run against in-process servers and count accepted TCP connections. A server that drops every connection: firefox-148 makes 2 attempts and chrome-146 makes 3, where chrome-146 previously made 4. An http/1.1 only server that closes without answering: both presets make exactly 1 connection and the error is returned, where chrome-146 previously made 2.