fix(ws): handle response stream errors on failed WS upgrade#116
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
=======================================
Coverage 96.11% 96.12%
=======================================
Files 8 8
Lines 644 645 +1
Branches 244 243 -1
=======================================
+ Hits 619 620 +1
Misses 24 24
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When a WebSocket upgrade request hits a target that responds with a
non-upgrade HTTP response (e.g., 502) and the upstream connection resets
mid-body, the `res` stream emits an unhandled error that crashes the
process. Add `res.on("error", onOutgoingError)` before piping, and guard
against writing to already-destroyed sockets.
Ref: http-party/node-http-proxy#1439
7f63ba2 to
23cafec
Compare
Summary
res.on("error", onOutgoingError)beforeres.pipe(socket)in bothws-incoming.ts(ProxyServer) andws.ts(proxyUpgrade) to catch upstream response stream errors during failed WebSocket upgrades!socket.destroyed && socket.writablecheckssettledrejection logic from the writable guard inproxyUpgradeso the promise always rejects on non-upgrade responsesFixes the crash described in upstream http-party/node-http-proxy#1439: when the target returns a non-upgrade HTTP response (e.g., 502) and the connection resets mid-body, the unhandled
errorevent on the response stream crashes the process.Test plan
test/ws.test.ts— proxyUpgrade with upstream that destroys socket mid-responsetest/http-proxy.test.ts— ProxyServer.ws() with same scenariotest/ws-destroyed-socket.test.ts— tests for writing to destroyed sockets before upstream responds