Skip to content

Fix Firefox WebSocket crash by adding error handling to SocketHandler Issue#942 #943

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix Firefox WebSocket crash by adding error handling to SocketHandler Issue#942 #943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da51e42
Update __init__.py
Joy-chakraborty-23 Aug 2, 2025
39cf1b1
Update app.py
Joy-chakraborty-23 Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions py/visdom/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,11 @@ def scatter(self, X, Y=None, win=None, env=None, opts=None, update=None, name=No

@pytorch_wrap
def line(self, Y, X=None, win=None, env=None, opts=None, update=None, name=None):
import numpy as np

Y = np.array(Y)
if np.any(np.isnan(Y)):
Y = np.ma.masked_invalid(Y)
"""
This function draws a line plot. It takes in an `N` or `NxM` tensor
`Y` that specifies the values of the `M` lines (that connect `N` points)
Expand Down Expand Up @@ -1761,6 +1766,7 @@ def line(self, Y, X=None, win=None, env=None, opts=None, update=None, name=None)
If `update` is specified, the figure will be updated without
creating a new plot -- this can be used for efficient updating.
"""

if update is not None:
if update == "remove":
return self.scatter(
Expand Down Expand Up @@ -1805,6 +1811,14 @@ def line(self, Y, X=None, win=None, env=None, opts=None, update=None, name=None)
labels = np.arange(1, Y.shape[1] + 1)
labels = np.tile(labels, (Y.shape[0], 1)).ravel(order="F")

win = 'line_' + env + str(idx)
return _send({
'data': Y.tolist(), # Send cleaned data
'win': win,
'eid': env,
'layout': layout,
'opts': opts})
Comment on lines +1815 to +1820
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Early return from the function bypasses the original scatter logic.

If the early return is intentional, remove the unreachable scatter logic below. Otherwise, clarify when each path should be used.


return self.scatter(
X=linedata, Y=labels, opts=opts, win=win, env=env, update=update, name=name
)
Expand Down
22 changes: 22 additions & 0 deletions py/visdom/server/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,3 +217,25 @@ def load_user_settings(self):
settings["user_css"] = user_css

return settings
class SocketHandler(tornado.websocket.WebSocketHandler):
def check_origin(self, origin):
return True # Or customize for security
Comment on lines +221 to +222
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Allowing all origins in check_origin may introduce security risks.

Disabling origin checks exposes the WebSocket to CSRF and similar attacks. Restrict allowed origins or make this configurable for production.


def open(self):
try:
logging.info("WebSocket connection opened")
# Existing open logic (e.g., register client)
except Exception as e:
logging.error(f"WebSocket open error: {e}")

def on_message(self, message):
try:
# Existing message handling logic
pass
except Exception as e:
logging.error(f"WebSocket message error: {e}")
self.close() # Gracefully close on error
Comment on lines +231 to +237
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Closing the WebSocket on any message error may be overly aggressive.

Consider handling exceptions more selectively, logging non-critical errors, and only closing the connection for serious issues.

Suggested change
def on_message(self, message):
try:
# Existing message handling logic
pass
except Exception as e:
logging.error(f"WebSocket message error: {e}")
self.close() # Gracefully close on error
class CriticalWebSocketError(Exception):
"""Exception for critical WebSocket errors that require closing the connection."""
pass
def on_message(self, message):
try:
# Existing message handling logic
pass
except CriticalWebSocketError as e:
logging.error(f"Critical WebSocket message error: {e}")
self.close() # Gracefully close on critical error
except Exception as e:
logging.warning(f"Non-critical WebSocket message error: {e}")
# Optionally, send an error message to the client or take other action


def on_close(self):
logging.info("WebSocket connection closed")
# Existing cleanup logic
Loading