feat(data-connect): add automatic disconnect and cleanup logic to streaming, harden error handling#9808
Conversation
|
There was a problem hiding this comment.
Code Review
This pull request implements automatic stream disconnection for idle connections and handles authentication state changes. It introduces a 60-second idle timeout that triggers when no active subscriptions remain, and logic to immediately disconnect and reject pending requests if a user logs in, logs out, or changes identity. Review feedback highlights a potential null pointer exception when retrieving the user ID and suggests replacing the hardcoded timeout value with a named constant for better maintainability.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces idle connection management and improved error handling for the Data Connect stream transport. Key changes include a 60-second idle timeout that closes the connection when no subscriptions are active, immediate disconnection upon authentication changes (login, logout, or user change), and enhanced error message extraction in both REST and WebSocket layers. Additionally, the QueryManager now cleans up local subscriptions when an external WebSocket disconnection occurs. Review feedback focuses on refining the authentication change logic to prevent unnecessary resets during initial state resolution, removing redundant code in the connection closure process, centralizing hardcoded error strings, and ensuring idle timers are cleared when connections are closed manually.
| e && | ||
| typeof e === 'object' && | ||
| 'message' in e && | ||
| e.message === 'WebSocket disconnected externally' |
There was a problem hiding this comment.
The error message string "WebSocket disconnected externally" is hardcoded here and in the transport layer (e.g., in websocket.ts). This creates a maintenance risk where changing the message in one place will break the cleanup logic in the other. Consider using a shared constant for this error message to ensure consistency.
There was a problem hiding this comment.
I agree - error hardening will come in a future PR.
Description
✨ This PR improves error handling and implements robust cleanup logic for the streaming transport during disconnects and authentication changes.
Changes
Testing
websocketTransport.test.tsand related files to verify disconnect, cleanup, and error behavior.