fix: include upstream status and body in WS pool dial error (#2994)#3012
fix: include upstream status and body in WS pool dial error (#2994)#3012thiscantbeserious wants to merge 1 commit intomaximhq:mainfrom
Conversation
…2994) The pool's dial() was discarding the *http.Response returned by gorilla's Dial, so every WS handshake failure logged the same opaque "websocket: bad handshake" with no upstream context. Capture resp, read up to 512 bytes with io.LimitReader, close the body, and include resp.StatusCode plus the snippet in the wrapped error. New format: "failed to dial upstream websocket <url> (upstream status <N>: <body>): <err>" Adds four table-driven tests covering: status+body in error, nil-response fallback (no panic), body truncation at 512 bytes, and body-closed (no resource leak). Closes maximhq#2994 Extracted from maximhq#2775 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Superseded by #3018 (merged 2026-04-24), which bundles the fix for this issue and several other native WS reliability bugs. Closing this draft as redundant. |
1 participant
Summary
Fixes #2994. When the gorilla WS dial rejects the upgrade, the upstream
*http.Responsecarries the status and body needed to diagnose the failure (auth failures, proxy errors, 4xx from the backend). The current code discards the response and returns only the opaquewebsocket: bad handshakeerror, leaving operators with no signal in logs.Changes
(*Pool).dial, capturerespfromwebsocket.DefaultDialer.Dial.respis non-nil on error, read up to 512 bytes viaio.LimitReader, close the body, and wrap the error with(upstream status N: snippet).respis nil (no upstream reached).Type of change
Affected areas
How to test
Coverage moved 59.7% to 62.6%. Four new tests cover: dial error with upstream status+body, nil-response fallback, body truncation at 512 bytes, response body closed.
Breaking changes
Related issues
Closes #2994. Extracted from #2775, the PR branch contains this fix as a side effect of the OAuth feature work and it has been pulled out here as a focused change.
Checklist