fix chat stream and websocket close

[iceljc]

PR Type

Bug fix, Enhancement

---

Description

• Fix WebSocket close status and messages

• Refactor parameter validation with new utility

• Improve session management in chat stream

• Remove duplicate code across providers

---

Diagram Walkthrough

flowchart LR
  A["WebSocket Sessions"] -- "Fix close status" --> B["Proper Closure"]
  C["Parameter Validation"] -- "Extract utility" --> D["LlmUtility"]
  E["Chat Stream"] -- "Improve session mgmt" --> F["Using Statement"]
  D -- "Replace duplicates" --> G["OpenAI Providers"]

Loading

File Walkthrough

	Relevant files
Enhancement	7 files LlmUtility.cs Add parameter validation utility method                                    +14/-0    ChatStreamMiddleware.cs Improve session management with using statement                    +7/-11    AudioTranscriptionProvider.cs Replace validation with LlmUtility method                                +2/-12    ImageCompletionProvider.Edit.cs Replace validation with LlmUtility method                                +3/-3      ImageCompletionProvider.Generation.cs Replace validation with LlmUtility method                                +5/-5      ImageCompletionProvider.Variation.cs Replace validation with LlmUtility method                                +2/-2      ImageCompletionProvider.cs Remove duplicate parameter validation method                          +0/-10
Bug fix	2 files BotSharpRealtimeSession.cs Fix WebSocket close status and message                                      +1/-1      LlmRealtimeSession.cs Fix WebSocket close status and message                                      +1/-1
Configuration changes	1 files Using.cs Add LlmUtility namespace import                                                    +1/-0

---

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Resource Disposal Switching to a local 'using var session' removes the previous field-backed session disposal in the catch and after loop; ensure no code paths can leak the websocket or leave the session undisposed, especially when exceptions occur before 'using' scope exits or during AcceptWebSocketAsync failures. using var webSocket = await httpContext.WebSockets.AcceptWebSocketAsync(); await HandleWebSocket(services, agentId, conversationId, webSocket); } catch (Exception ex) { _logger.LogError(ex, $"Error when connecting Chat stream. ({ex.Message})"); } return; } } await _next(httpContext); } private async Task HandleWebSocket(IServiceProvider services, string agentId, string conversationId, WebSocket webSocket) { using var session = new BotSharpRealtimeSession(services, webSocket, new ChatSessionOptions { Close Reason Closing with a formatted message that includes provider can exceed typical close message size or leak internal details to clients; verify the reason string length and whether including provider info is acceptable for clients. public async Task DisconnectAsync() { if (_disposed || _websocket.State != WebSocketState.Open) { return; } await _websocket.CloseAsync(WebSocketCloseStatus.NormalClosure, $"Normal Closure from {nameof(BotSharpRealtimeSession)}-{_sessionOptions?.Provider}", CancellationToken.None); } Null Checks In 'ConnectToModel', 'session' is non-null by construction, so the null check is redundant; consider removing to simplify and avoid masking potential lifetime issues. private async Task ConnectToModel(IRealtimeHub hub, BotSharpRealtimeSession session, List<MessageState>? states = null) { await hub.ConnectToModel(responseToUser: async data => { if (session != null) { await session.SendEventAsync(data); } }, initStates: states);

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add exception handling for cleanup Wrap the logic in HandleWebSocket with a try...finally block to ensure convService.SaveStates() and session.DisconnectAsync() are always called for proper cleanup and state persistence, even if an exception occurs. src/Plugins/BotSharp.Plugin.ChatHub/ChatStreamMiddleware.cs [51-108] private async Task HandleWebSocket(IServiceProvider services, string agentId, string conversationId, WebSocket webSocket) { using var session = new BotSharpRealtimeSession(services, webSocket, new ChatSessionOptions { Provider = "BotSharp Chat Stream", BufferSize = 1024 * 32, JsonOptions = BotSharpOptions.defaultJsonOptions, Logger = _logger }); - var hub = services.GetRequiredService<IRealtimeHub>(); - var conn = hub.SetHubConnection(conversationId); - conn.CurrentAgentId = agentId; - InitEvents(conn); + var convService = services.GetRequiredService<IConversationService>(); + try + { + var hub = services.GetRequiredService<IRealtimeHub>(); + var conn = hub.SetHubConnection(conversationId); + conn.CurrentAgentId = agentId; + InitEvents(conn); - // load conversation and state - var convService = services.GetRequiredService<IConversationService>(); - var state = services.GetRequiredService<IConversationStateService>(); - convService.SetConversationId(conversationId, []); - await convService.GetConversationRecordOrCreateNew(agentId); + // load conversation and state + var state = services.GetRequiredService<IConversationStateService>(); + convService.SetConversationId(conversationId, []); + await convService.GetConversationRecordOrCreateNew(agentId); - await foreach (ChatSessionUpdate update in session.ReceiveUpdatesAsync(CancellationToken.None)) - { - var receivedText = update?.RawResponse; - if (string.IsNullOrEmpty(receivedText)) + await foreach (ChatSessionUpdate update in session.ReceiveUpdatesAsync(CancellationToken.None)) { - continue; - } + var receivedText = update?.RawResponse; + if (string.IsNullOrEmpty(receivedText)) + { + continue; + } - var (eventType, data) = MapEvents(conn, receivedText, conversationId); - if (eventType == "start") - { -#if DEBUG - _logger.LogCritical($"Start chat stream connection for conversation ({conversationId})"); -#endif - var request = InitRequest(data, conversationId); - await ConnectToModel(hub, session, request?.States); - } - else if (eventType == "media") - { - if (!string.IsNullOrEmpty(data)) + var (eventType, data) = MapEvents(conn, receivedText, conversationId); + if (eventType == "start") { - await hub.Completer.AppenAudioBuffer(data); + #if DEBUG + _logger.LogCritical($"Start chat stream connection for conversation ({conversationId})"); + #endif + var request = InitRequest(data, conversationId); + await ConnectToModel(hub, session, request?.States); + } + else if (eventType == "media") + { + if (!string.IsNullOrEmpty(data)) + { + await hub.Completer.AppenAudioBuffer(data); + } + } + else if (eventType == "disconnect") + { + #if DEBUG + _logger.LogCritical($"Disconnecting chat stream connection for conversation ({conversationId})"); + #endif + await hub.Completer.Disconnect(); + break; } } - else if (eventType == "disconnect") - { -#if DEBUG - _logger.LogCritical($"Disconnecting chat stream connection for conversation ({conversationId})"); -#endif - await hub.Completer.Disconnect(); - break; - } } - - convService.SaveStates(); - await session.DisconnectAsync(); + finally + { + convService.SaveStates(); + await session.DisconnectAsync(); + } } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that an exception within HandleWebSocket would skip crucial cleanup logic, leading to unsaved state and improperly closed connections, and proposes a robust try...finally solution.	Medium
Learned best practice	Safely close WebSocket with fallback Use a non-null close description to avoid null propagation and include a fallback when provider is missing. Also guard for CloseSent state to prevent invalid operations. src/Infrastructure/BotSharp.Core/Session/LlmRealtimeSession.cs [112-115] -if (_webSocket.State == WebSocketState.Open) +if (_webSocket.State == WebSocketState.Open || _webSocket.State == WebSocketState.CloseReceived) { - await _webSocket.CloseAsync(WebSocketCloseStatus.NormalClosure, $"Normal Closure from {nameof(LlmRealtimeSession)}-{_sessionOptions?.Provider}", CancellationToken.None); + var reason = $"Normal Closure from {nameof(LlmRealtimeSession)}-{_sessionOptions?.Provider ?? "Unknown"}"; + await _webSocket.CloseAsync(WebSocketCloseStatus.NormalClosure, reason, CancellationToken.None); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Prefer safe defaults and explicit normal closure reason when closing WebSockets; validate state before close.	Low
More