fix(api): drop request timeout from WebSocket upgrade routes - #1433
fix(api): drop request timeout from WebSocket upgrade routes#1433wizzomafizzo wants to merge 1 commit into
Conversation
- The WebSocket route group wrapped the upgrade handler in chi's Timeout middleware. The handler returns only when the connection closes, so every session older than 30s ended with a 504 header written to the hijacked connection and a net/http warning on stderr, which is now captured into core.stderr.log and shipped with every log bundle. - Nothing on the WebSocket path reads the upgrade request's context; melody ignores it and per-message deadlines come from the dispatcher. The group now matches the SSE group, which already carries no request timeout. - Move the group into mountWebSocketRoutes and add a regression test that asserts the upgrade request context carries no deadline on /api, /api/v0 and /api/v0.1.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughWebSocket route registration now uses a dedicated helper for ChangesWebSocket route handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to WebSocket upgrade routes no longer inherit the request timeout that caused long-lived sessions to emit errors on hijacked connections. Existing route protections and version behavior remain in place, with coverage for all supported upgrade paths. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
Timeoutmiddleware. The handler returns only when the connection closes, so every session older than 30s ended with a 504 header written to the hijacked connection and ahttp: response.WriteHeader on hijacked connectionwarning on stderr. Since fix(readers): recover reader auto-detect and make its failures diagnosable #1424 that stderr is captured intocore.stderr.logand shipped with every log bundle.mountWebSocketRoutesso the real wiring can be tested, and a regression test asserts the upgrade request context carries no deadline on/api,/api/v0and/api/v0.1. It fails with the middleware in place.Closes #1430
Summary by CodeRabbit
Bug Fixes
Tests