fix(ws): handle multi-server upgrade subscription and safe proxy shutdown - #1237
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR fixes WebSocket proxying across multiple servers by replacing global boolean tracking with per-server subscription and lifecycle management. The middleware now tracks which servers have had their ChangesMulti-server WebSocket and shutdown handling
Sequence DiagramsequenceDiagram
participant Server1
participant Server2
participant HttpProxyMiddleware
participant WebSocketUpgrade as WebSocket Upgrade<br/>(per server)
participant GracefulShutdown as Graceful Shutdown<br/>(per server)
Server1->>HttpProxyMiddleware: Attach middleware via use()
Server2->>HttpProxyMiddleware: Attach middleware via use()
Server1->>HttpProxyMiddleware: First upgrade request on Server1
HttpProxyMiddleware->>WebSocketUpgrade: Check WeakSet for Server1
WebSocketUpgrade->>HttpProxyMiddleware: Not subscribed, subscribe now
HttpProxyMiddleware->>WebSocketUpgrade: Register upgrade handler on Server1
Server2->>HttpProxyMiddleware: First upgrade request on Server2
HttpProxyMiddleware->>WebSocketUpgrade: Check WeakSet for Server2
WebSocketUpgrade->>HttpProxyMiddleware: Not subscribed, subscribe now
HttpProxyMiddleware->>WebSocketUpgrade: Register upgrade handler on Server2
Server1->>HttpProxyMiddleware: close()
HttpProxyMiddleware->>GracefulShutdown: Remove Server1 from activeServers
GracefulShutdown->>HttpProxyMiddleware: activeServers not empty, defer proxy.close()
Server2->>HttpProxyMiddleware: close()
HttpProxyMiddleware->>GracefulShutdown: Remove Server2 from activeServers
GracefulShutdown->>HttpProxyMiddleware: activeServers empty, call proxy.close()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
commit: |
fixes: #1149
Summary by CodeRabbit