fix(http3): run init on the AddConn path

[Mootjelh]

Fixes #457.

AddConn is the second way into getClient, and only roundTripOpt ran init. So a transport whose first use is AddConn reaches dial with QUICConfig, transport and newClientConn all still nil, and dial runs in its own goroutine, so the nil quic.Transport takes the process with it.

Reproduced on master, and it is the stack from the issue:

panic: runtime error: invalid memory address or nil pointer dereference
github.com/quic-go/quic-go.(*Transport).dial(0x0, ...)
	quic-go@v0.61.0/transport.go:247
github.com/quic-go/quic-go.(*Transport).DialEarly(...)
github.com/imroc/req/v3/internal/http3.(*Transport).dial.func1(...)
	internal/http3/transport.go:397
github.com/imroc/req/v3/internal/http3.(*Transport).dial(...)
	internal/http3/transport.go:407
github.com/imroc/req/v3/internal/http3.(*Transport).getClient.func1()
	internal/http3/transport.go:341

That is tr := &Transport{} followed by one AddConn.

Why there is nothing to port from quic-go

You mentioned in the thread that the upstream fix would need porting by hand. There is no upstream fix: quic-go's http3 transport has no AddConn at all, so roundTripOpt is its only door into getClient and init always runs first. AddConn is this repo's, for the alt-svc path, and it opened a second door.

Why it is intermittent

The normal request path calls t.t3.RoundTripOnlyCachedConn(req) on every https request once HTTP/3 is enabled, and that goes through roundTripOpt, so init has usually already run by the time handlePendingAltSvc calls AddConn in its goroutine. An end to end test here, a server answering Alt-Svc: h3=":443" to a normal client, survives for that reason. It only bites when nothing has run init yet, which is why it shows up on short lived clients under load.

The fix

initOnce.Do at the top of getClient, which is the one point both paths pass through. Outside the mutex, the way roundTripOpt does it, because init opens a UDP socket.

Test

TestAddConnInitializesTheTransport hands the transport a Dial func that records the *quic.Config it is given and returns an error, so nothing touches the network. On master the config is nil, since init never ran:

--- FAIL: TestAddConnInitializesTheTransport
    transport_test.go:170: dialed with a nil quic.Config, so init did not run before AddConn dialed

It passes with the change, and the bare reproduction above now returns instead of panicking. The test fails first if the dial never happens, so it cannot pass by the stub going uncalled.

go test ./... gives the same two failures with and without the change, TestTraceInfo and TestSetFile. Both are on master here as well, and TestSetFile is a Windows error string difference.

Credit, and one thing left out

@danielboros reported this and put the same initOnce.Do call in their fork. They also added an ErrNilNewClientConn guard in dial. I have left that out: once init runs on this path newClientConn cannot be nil, and it would add an exported error for a state that can no longer happen. Happy to add it if you would rather have the belt and braces.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[imroc]

Thanks for the clean reproduction and write-up. I verified the fix locally (the fork CI hasn't run, as it needs manual approval):

Bug confirmation

• AddConn is req's own addition for the alt-svc path (handlePendingAltSvc calls t.t3.AddConn from its own goroutine) — quic-go's http3 transport has no AddConn, so there is indeed nothing to port from upstream; the second door into getClient is ours.
• On master, roundTripOpt was the only getClient caller that ran initOnce (getClient is otherwise reached only from doRoundTripOpt and AddConn). A first-use-AddConn transport reaches the dial goroutine with t.transport, t.QUICConfig and t.newClientConn all nil, and t.transport.DialEarly on a nil *quic.Transport takes the process down. The panic is real and matches the stack in #457.
• The intermittency explanation also checks out: every https request goes through RoundTripOnlyCachedConn → roundTripOpt first, which usually runs init before the alt-svc goroutine gets to AddConn.

Fix assessment

• initOnce.Do at the top of getClient is the right chokepoint: both roundTripOpt and AddConn funnel through it, so any future entry point inherits the guarantee too.
• Running it outside t.mutex matches roundTripOpt's existing pattern, and init() doesn't touch t.mutex, so there is no deadlock or lock-ordering concern.
• Reading t.initErr after initOnce.Do(...) is race-free (the sync.Once happens-before edge covers it); initErr is written exactly once inside the Once.
• The now-redundant Do in roundTripOpt is harmless.

Verification (Linux, Go 1.27.0, commit a1b6d12)

• go build ./... && go vet ./... && go test ./... — all green. The two failures mentioned in the description (TestTraceInfo, TestSetFile) don't reproduce on Linux; they look environment-specific and unrelated to this change.
• TestAddConnInitializesTheTransport passes with the fix; with internal/http3/transport.go reverted to master it fails with dialed with a nil quic.Config — independently confirming the regression test catches the bug.
• The test itself is well constructed: no network (the Dial stub records the config and returns an error), a 10s guard so it can't pass by the stub never being called, and assertions on init-mutated fields (MaxIncomingStreams == -1, exactly one Version).

Leaving out the ErrNilNewClientConn belt-and-braces guard looks right to me: once init runs on this path newClientConn can no longer be nil, and an exported error for an unreachable state would just be extra API surface.

Since this touches internal/http3/ (our modified quic-go copy), per repo policy I'm leaving the merge decision to the maintainer — but the analysis and verification above all check out from my side.