-
Notifications
You must be signed in to change notification settings - Fork 397
Realtime ws connection #4813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: task-refactor
Are you sure you want to change the base?
Realtime ws connection #4813
Changes from all commits
75d17ae
1f4059d
49f4996
731461b
b34ee67
b6f935d
cf3e799
f354f57
a85fffe
52e5e12
e5f53da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,8 @@ export default class Services { | |
| public readonly dialer: ReturnType<typeof aqmDialer>; | ||
| /** WebSocket manager for handling real-time communications */ | ||
| public readonly webSocketManager: WebSocketManager; | ||
| /** RTD WebSocket manager for handling real-time transcription */ | ||
| public readonly rtdWebSocketManager: WebSocketManager; | ||
| /** Connection service for managing websocket connections */ | ||
| public readonly connectionService: ConnectionService; | ||
| /** Singleton instance of the Services class */ | ||
|
|
@@ -39,6 +41,7 @@ export default class Services { | |
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new RTD manager is created but not integrated into reconnection handling: Useful? React with 👍 / 👎. |
||
| const aqmReq = new AqmReqs(this.webSocketManager); | ||
| this.config = new AgentConfigService(); | ||
| this.agent = routingAgent(aqmReq); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This unconditional RTD close path can throw during
deregister()for agents without realtime transcription enabled.rtdWebSocketManager.initWebSocket()is only called whenaiFeature.realtimeTranscripts.enableis true, but a freshWebSocketManagerstarts withisSocketClosed = falseandwebsocket = {}, so callingclose()here leads tothis.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 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adrressed