Skip to content

fix: stop matching GOAWAY by a deprecated type - #415

Merged
taciturnaxolotl merged 1 commit into
mainfrom
fix/deprecated-goaway
Oct 6, 2026
Merged

taciturnaxolotl merged 1 commit into
mainfrom
fix/deprecated-goaway

Conversation

@taciturnaxolotl

Copy link
Copy Markdown
Member

Why

main has been failing lint since the HTTP/2 retry work landed:

errors.go:195:13: SA1019: golang.org/x/net/http2.GoAwayError is deprecated (staticcheck)

Three consecutive runs on main are red for this, so every PR opened since inherits a red check.

x/net deprecated the type without naming a replacement, and the transport still returns it, so there is nothing to migrate to.

Why deleting the check is safe

GoAwayError.Error() renders as:

http2: server sent GOAWAY and closed the connection; LastStreamID=7, ErrCode=NO_ERROR, debug="bye"

That begins with the sentence already sitting in http2ConnectionLostMessages, and matchConnectionLost uses strings.Contains. So the typed check and the fallback a few lines below were catching the same thing, and only the typed one is deprecated.

The existing TestIsTransportError/x/net_GoAwayError case feeds a real http2.GoAwayError through IsTransportError and still passes untouched. That is the evidence this is a tidy-up and not a behaviour change — the test was not adjusted to fit.

Matching by message was always going to be the long-term answer for GOAWAY anyway. The standard library's copy of http2 is internal, so its equivalent type can never be named here, and now x/net's cannot be either without a warning. Both spell the failure identically, so one fragment covers both — which is why the file already had the fragment.

StreamError and ConnectionError are untouched and still matched by type; neither is deprecated.

Testing

go test . passes. golangci-lint run --config=.golangci.yml ./... reports 0 issues locally, against the same v2 config CI uses.

x/net deprecated http2.GoAwayError without naming a replacement, which
has had the linter failing on main since the HTTP/2 retry work landed.

Nothing is lost by dropping the type check. GoAwayError's message begins
with the sentence already listed in http2ConnectionLostMessages, and
that list is matched with strings.Contains, so the same failure is still
recognised by the fallback a line below. The existing test that feeds a
real x/net GoAwayError through IsTransportError passes unchanged, which
is what says this is a tidy-up rather than a behaviour change.

Matching by message was always going to be the long-term answer here:
the standard library's copy of the package is internal, so its
equivalent type cannot be named at all, and now neither can x/net's
without a deprecation warning. Both spell the failure the same way.
@taciturnaxolotl
taciturnaxolotl enabled auto-merge (squash) October 6, 2026 22:44
@taciturnaxolotl
taciturnaxolotl merged commit b0fadc9 into main Oct 6, 2026
5 checks passed
@taciturnaxolotl
taciturnaxolotl deleted the fix/deprecated-goaway branch October 6, 2026 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant