Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Sep 23, 2025

PR Type

Enhancement


Description

  • Enhanced WebSocket close event logging with status details

  • Added proper close code and reason for normal closure


Diagram Walkthrough

flowchart LR
  A["WebSocket onclose event"] --> B["Log close event with status"]
  C["Socket disconnect"] --> D["Close with code 1000 and reason"]
Loading

File Walkthrough

Relevant files
Enhancement
realtime-chat-service.js
Enhanced WebSocket close handling with status logging       

src/lib/services/realtime-chat-service.js

  • Modified onclose handler to accept and log close event parameter
  • Updated socket.close() call to include proper close code (1000) and
    reason
+3/-3     

@iceljc iceljc merged commit 31cfbc8 into SciSharp:main Sep 23, 2025
1 of 2 checks passed
@qodo-merge-pro
Copy link

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

Console Noise

Logging the entire close event object on every close may be verbose in production; consider logging selective fields (code, reason, wasClean) or gating behind a debug flag.

socket.onclose = (e) => {
    console.log("Websocket closed", e);
};
Close Reason Support

Some browsers ignore custom close reasons unless size limits are respected; validate that the provided reason string is short and that downstream expects code 1000 to avoid regressions.

socket.close(1000, "Normal Closure");
socket = null;

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Ensure resource cleanup on any close

Call realtimeChat.stop() within the socket.onclose handler to ensure all
resources are properly released when the connection closes, preventing potential
resource leaks.

src/lib/services/realtime-chat-service.js [94-96]

 socket.onclose = (e) => {
     console.log("Websocket closed", e);
+    // Call stop() to ensure all resources are released, even on unexpected closures.
+    realtimeChat.stop();
 };
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a potential resource leak where resources like mediaStream are not cleaned up on unexpected WebSocket closures, and provides a robust solution by calling the stop() function from the onclose handler.

High
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant