Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5f53da1d6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!this.services.rtdWebSocketManager.isSocketClosed) { | ||
| this.services.rtdWebSocketManager.close(false, 'Unregistering the SDK'); |
There was a problem hiding this comment.
Guard RTD websocket close when no RTD connection exists
This unconditional RTD close path can throw during deregister() for agents without realtime transcription enabled. rtdWebSocketManager.initWebSocket() is only called when aiFeature.realtimeTranscripts.enable is true, but a fresh WebSocketManager starts with isSocketClosed = false and websocket = {}, so calling close() here leads to this.websocket.close() on a non-WebSocket object and aborts deregistration. Please gate this close call on actual RTD connection initialization/open state (or make the manager default to a closed state until connected).
Useful? React with 👍 / 👎.
| constructor(options: {webex: WebexSDK; connectionConfig: SubscribeRequest}) { | ||
| const {webex, connectionConfig} = options; | ||
| this.webSocketManager = new WebSocketManager({webex}); | ||
| this.rtdWebSocketManager = new WebSocketManager({webex}); |
There was a problem hiding this comment.
Add reconnect wiring for the RTD websocket manager
The new RTD manager is created but not integrated into reconnection handling: ConnectionService only subscribes to 'message'/'socketClose' on the primary webSocketManager, and WebSocketManager itself does not perform retries. In practice, if the RTD socket drops once (for example, a transient network loss), transcript delivery will stop for the rest of the session unless the SDK is fully re-registered. Please attach equivalent reconnect logic for rtdWebSocketManager.
Useful? React with 👍 / 👎.
COMPLETES
This pull request addresses
New websocket implementation for real time data
by making the following changes
New websocket implementation for real time data
Change Type
The following scenarios were tested
https://app.vidcast.io/share/c2d427f6-2b17-40ea-8eda-7ffe73e58483?playerMode=vidcast
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.