fix(engine): avoid duplicate WriteHeader on failed websocket upgrade#172
Merged
zishang520 merged 1 commit intoJul 2, 2026
Conversation
MaxwellKuo47
force-pushed
the
fix/engine-duplicate-writeheader-upgrade
branch
from
759389c to
a740a5f
Compare
On a failed upgrade the Upgrader.Error callback wrote the response via http.Error, then emitAbortRequest wrote it again, logging "superfluous response.WriteHeader call" on every failed upgrade. Let emitAbortRequest be the sole responder so the header is written once and the client still gets the engine.io error payload.
MaxwellKuo47
force-pushed
the
fix/engine-duplicate-writeheader-upgrade
branch
from
a740a5f to
3229d78
Compare
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
3 participants
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.
Fixes #171.
On a failed WebSocket upgrade the response header is written twice on the same
ResponseWriter, so net/http logshttp: superfluous response.WriteHeader call ... performWrite (http-context.go:214)once per failed upgrade. See #171 for the full trace.The gorilla
Upgrader.Errorcallback inHandleUpgradecallshttp.Error(w, …)(write #1, straight to the underlying writer, bypassingHttpContext's write-once guard); the followingemitAbortRequest→abortRequest→ctx.Write→performWriteis write #2.Change
Removed the
http.Errorfrom theErrorcallback.emitAbortRequest(called right afterws.Upgradefails) is the canonical responder — it writes the engine.io JSON error and emitsconnection_error— so a failed upgrade now writes the header exactly once, and clients get the proper engine.io error payload instead of gorilla's plain-text body. The WebTransport path (OnWebTransportSession) has no such callback and is unaffected.Test
Added
TestFailedUpgradeWritesHeaderOnce: it drives a failed upgrade throughServeHTTPwith aResponseWriterthat countsWriteHeadercalls and asserts it is called exactly once. Fails before the change (2 calls), passes after.go test -race ./...inservers/engineis green.