-
Notifications
You must be signed in to change notification settings - Fork 8
feat(conversations): expose disconnect method for clean process exit #340
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: main
Are you sure you want to change the base?
Changes from all commits
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -467,6 +467,22 @@ export class ConversationService extends BaseService implements ConversationServ | |||||
| return this._sessionManager.onConnectionStatusChanged(handler); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Disconnects the WebSocket and releases all session resources. | ||||||
| * | ||||||
| * Immediately closes the WebSocket connection and clears all per-conversation | ||||||
| * socket tracking. Call this after ending all sessions to allow the process to exit. | ||||||
|
Collaborator
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. "Call this after ending all sessions" implies
Collaborator
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. have the same description in models as well when you add, because in public docs, comments from models get picked up.
Collaborator
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. also add a line somehting like this: "In Node.js, the WebSocket keeps the event loop alive until disconnected, call this to allow the process to exit cleanly." because In a browser, the runtime handles socket cleanup on page unload, so this is a no-op there. |
||||||
| * | ||||||
| * @example | ||||||
| * ```typescript | ||||||
| * conversationalAgent.conversations.endSession(conversationId); | ||||||
| * conversationalAgent.conversations.disconnect(); | ||||||
| * ``` | ||||||
| */ | ||||||
| disconnect(): void { | ||||||
|
Collaborator
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.
|
||||||
| this._sessionManager.disconnect(); | ||||||
| } | ||||||
|
|
||||||
| // ==================== Private Methods ==================== | ||||||
|
|
||||||
| private _getEvents(): ConversationEventHelperManager { | ||||||
|
|
||||||
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.
Let's not bump up the version with this change. We might have a few other changes we want to ship with the next version. We'll raise a separate PR for the version change.