From 79f526b1b21c08c85fcff64f9fc00361e3f49871 Mon Sep 17 00:00:00 2001 From: Ed Date: Sun, 31 May 2026 02:12:03 +0200 Subject: [PATCH] fix(helpers): use ssl.SSLError instead of broad OSError in disconnect tuple MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OSError is too broad — it masks real errors like file-not-found. ssl.SSLError specifically catches SSL-level disconnects without swallowing unrelated OSError subtypes. Closes the test_excludes_broad_oserror CI failure. --- api/helpers.py | 7 ++----- tests/test_broken_pipe_cascade.py | 9 +++++++++ tests/test_session_lost_response_regression.py | 3 ++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/api/helpers.py b/api/helpers.py index d51f518c..e9abdbc1 100644 --- a/api/helpers.py +++ b/api/helpers.py @@ -4,6 +4,7 @@ Hermes Web UI -- HTTP helper functions. import json as _json import os import re as _re +import ssl from pathlib import Path from api.config import IMAGE_EXTS, MD_EXTS @@ -11,16 +12,12 @@ from api.config import IMAGE_EXTS, MD_EXTS # Treat stalled/closed HTTP clients as normal disconnects. Long-lived SSE # connections often end this way when a browser tab sleeps, a phone switches # networks, or Tailscale leaves the socket half-closed. -# -# ssl.SSLError subclasses OSError, so the bare OSError arm already catches -# SSL-level disconnects. We keep OSError explicitly for socket-level errors -# (errno 32 EPIPE, 54 ECONNRESET, 104 ECONNABORTED, 110 ETIMEDOUT). _CLIENT_DISCONNECT_ERRORS = ( BrokenPipeError, ConnectionResetError, ConnectionAbortedError, TimeoutError, - OSError, + ssl.SSLError, ) diff --git a/tests/test_broken_pipe_cascade.py b/tests/test_broken_pipe_cascade.py index a1e2c1b0..88a09ac7 100644 --- a/tests/test_broken_pipe_cascade.py +++ b/tests/test_broken_pipe_cascade.py @@ -226,11 +226,14 @@ class TestServerDisconnectHandling(unittest.TestCase): # Patch handle_get to raise BrokenPipeError import server as _server_mod orig_handle_get = _server_mod.handle_get + orig_check_auth = _server_mod.check_auth _server_mod.handle_get = _fake_handle_get + _server_mod.check_auth = lambda h, p: True try: Handler.do_GET(handler) finally: _server_mod.handle_get = orig_handle_get + _server_mod.check_auth = orig_check_auth # send_response should NEVER be called for the 500 — client is gone handler.send_response.assert_not_called() @@ -284,11 +287,14 @@ class TestServerDisconnectHandling(unittest.TestCase): import server as _server_mod orig_handle_get = _server_mod.handle_get + orig_check_auth = _server_mod.check_auth _server_mod.handle_get = _fake_handle_get + _server_mod.check_auth = lambda h, p: True try: Handler.do_GET(handler) finally: _server_mod.handle_get = orig_handle_get + _server_mod.check_auth = orig_check_auth handler.send_response.assert_not_called() @@ -302,11 +308,14 @@ class TestServerDisconnectHandling(unittest.TestCase): import server as _server_mod orig_handle_get = _server_mod.handle_get + orig_check_auth = _server_mod.check_auth _server_mod.handle_get = _fake_handle_get + _server_mod.check_auth = lambda h, p: True try: Handler.do_GET(handler) finally: _server_mod.handle_get = orig_handle_get + _server_mod.check_auth = orig_check_auth handler.send_response.assert_not_called() diff --git a/tests/test_session_lost_response_regression.py b/tests/test_session_lost_response_regression.py index 9f8b63c7..f0d56291 100644 --- a/tests/test_session_lost_response_regression.py +++ b/tests/test_session_lost_response_regression.py @@ -326,7 +326,8 @@ def test_messages_js_names_browser_sse_disconnect_separately(): def test_server_treats_broken_pipe_as_client_disconnect_not_500(): server_py = (models.Path(__file__).parent.parent / "server.py").read_text(encoding="utf-8") - assert "except (BrokenPipeError, ConnectionResetError, ConnectionAbortedError):" in server_py + # server.py now uses the centralized _CLIENT_DISCONNECT_ERRORS tuple from api.helpers + assert "_CLIENT_DISCONNECT_ERRORS" in server_py assert "do not convert it into a misleading server 500" in server_py