Merge pull request #3233 from nesquena/release/stage-batchB2
Release stage-batchB2 → v0.51.182 (consolidated client-disconnect handling #3210)
This commit is contained in:
7 files changed
+498
-27
No files matched your search
+6
-1
@@ -3,8 +3,13 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.51.182] — 2026-05-31 — Release FB (stage-batchB2 — headless browser smoke gate + consolidated client-disconnect handling)
|
||||
|
||||
### Added
|
||||
- CI now runs a headless browser smoke test (`tests/browser_smoke.py`, `.github/workflows/browser-smoke.yml`) on every PR and push: it boots the real server agent-free and loads the key pages in Chromium, failing on any console error or uncaught JS exception. This catches the runtime-JS brick class (e.g. a `const` reassigned at runtime as in #3162, or a `function`/`window` name collision as in #2715/#2771) that `node --check`, ESLint, and the mocked test suite cannot see because they only manifest when a real browser executes the page. The smoke is credential-free — it strips `*_API_KEY` from the environment and drives no real model.
|
||||
- CI now runs a headless browser smoke test (`tests/browser_smoke.py`, `.github/workflows/browser-smoke.yml`) on every PR and push: it boots the real server agent-free and loads the key pages in Chromium, failing on any console error or uncaught JS exception. This catches the runtime-JS brick class (e.g. a `const` reassigned at runtime as in #3162, or a `function`/`window` name collision as in #2715/#2771) that `node --check`, ESLint, and the mocked test suite cannot see because they only manifest when a real browser executes the page. The smoke is credential-free — it strips `*_API_KEY` from the environment and drives no real model (#3231).
|
||||
|
||||
### Fixed
|
||||
- Client disconnects during response writes (browser tab close, SSE reconnect races, mobile network switches, half-closed sockets) are now handled through a single `_CLIENT_DISCONNECT_ERRORS` set and a `_safe_write` helper instead of ad-hoc per-call-site `try/except`, so an expected disconnect no longer surfaces as a misleading server 500 in the logs. The error-response path is itself wrapped so a disconnect while sending a 500 is swallowed quietly rather than cascading. A bare `TimeoutError` from the Joplin notes integration's `urlopen` is now converted to a clean "not reachable" error at the route rather than escaping to the dispatch-level disconnect handler (#3210).
|
||||
|
||||
## [v0.51.181] — 2026-05-30 — Release FA (stage-batchA — agent-cache eviction teardown + streaming finalize race)
|
||||
|
||||
|
||||
+34
-4
@@ -4,10 +4,23 @@ 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
|
||||
|
||||
|
||||
# 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.
|
||||
_CLIENT_DISCONNECT_ERRORS = (
|
||||
BrokenPipeError,
|
||||
ConnectionResetError,
|
||||
ConnectionAbortedError,
|
||||
TimeoutError,
|
||||
ssl.SSLError,
|
||||
)
|
||||
|
||||
|
||||
def require(body: dict, *fields) -> None:
|
||||
"""Phase D: Validate required fields. Raises ValueError with clean message."""
|
||||
missing = [f for f in fields if not body.get(f) and body.get(f) != 0]
|
||||
@@ -65,6 +78,25 @@ def _accepts_gzip(handler) -> bool:
|
||||
return 'gzip' in ae
|
||||
|
||||
|
||||
def _safe_write(handler, body: bytes) -> None:
|
||||
"""Write response body, ignoring expected client disconnect errors.
|
||||
|
||||
Logs disconnects at debug level so they are observable without
|
||||
polluting stdout/stderr during normal operation (SSE reconnects,
|
||||
tab closes, mobile network switches, etc.).
|
||||
"""
|
||||
try:
|
||||
handler.end_headers()
|
||||
handler.wfile.write(body)
|
||||
except _CLIENT_DISCONNECT_ERRORS as exc:
|
||||
import logging
|
||||
logging.getLogger("hermes.webui").debug(
|
||||
"Client disconnected mid-response (%s): %s",
|
||||
type(exc).__name__,
|
||||
getattr(handler, "path", "?"),
|
||||
)
|
||||
|
||||
|
||||
def j(handler, payload, status: int=200, extra_headers: dict=None) -> None:
|
||||
"""Send a JSON response.
|
||||
|
||||
@@ -89,8 +121,7 @@ def j(handler, payload, status: int=200, extra_headers: dict=None) -> None:
|
||||
if extra_headers:
|
||||
for k, v in extra_headers.items():
|
||||
handler.send_header(k, v)
|
||||
handler.end_headers()
|
||||
handler.wfile.write(body)
|
||||
_safe_write(handler, body)
|
||||
|
||||
|
||||
def t(handler, payload, status: int=200, content_type: str='text/plain; charset=utf-8') -> None:
|
||||
@@ -101,8 +132,7 @@ def t(handler, payload, status: int=200, content_type: str='text/plain; charset=
|
||||
handler.send_header('Content-Length', str(len(body)))
|
||||
handler.send_header('Cache-Control', 'no-store')
|
||||
_security_headers(handler)
|
||||
handler.end_headers()
|
||||
handler.wfile.write(body)
|
||||
_safe_write(handler, body)
|
||||
|
||||
|
||||
MAX_BODY_BYTES = 20 * 1024 * 1024 # 20MB limit for non-upload POST bodies
|
||||
|
||||
+8
-14
@@ -40,19 +40,6 @@ from api.session_events import (
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# 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. If these bubble to the
|
||||
# request handler, the server logs 500s and can leave CLOSE-WAIT sockets around
|
||||
# until the OS-level timeout fires.
|
||||
_CLIENT_DISCONNECT_ERRORS = (
|
||||
BrokenPipeError,
|
||||
ConnectionResetError,
|
||||
ConnectionAbortedError,
|
||||
TimeoutError,
|
||||
OSError,
|
||||
)
|
||||
|
||||
# ── Cron run tracking ────────────────────────────────────────────────────────
|
||||
# Track job IDs currently being executed so the frontend can poll status.
|
||||
_RUNNING_CRON_JOBS: dict[str, float] = {} # job_id → start_timestamp
|
||||
@@ -1054,6 +1041,7 @@ from api.helpers import (
|
||||
_sanitize_error,
|
||||
redact_session_data,
|
||||
_redact_text,
|
||||
_CLIENT_DISCONNECT_ERRORS,
|
||||
)
|
||||
from api.agent_health import build_agent_health_payload
|
||||
from api.gateway_chat import gateway_chat_config_status
|
||||
@@ -13288,7 +13276,13 @@ def _joplin_api_get(path: str, params: dict | None = None) -> dict:
|
||||
raw = response.read(2_000_000).decode("utf-8", errors="replace")
|
||||
except HTTPError as exc:
|
||||
raise ValueError(f"Joplin API returned HTTP {exc.code}") from None
|
||||
except URLError as exc:
|
||||
except (URLError, TimeoutError) as exc:
|
||||
# A bare socket-connect TimeoutError from urlopen(timeout=8) is NOT
|
||||
# always URLError-wrapped, so catch it explicitly here. Otherwise it
|
||||
# propagates past _handle_notes_search's `except ValueError` to the
|
||||
# request dispatch, where the consolidated client-disconnect handler
|
||||
# (#3210) would swallow it as a fake disconnect — silent empty response,
|
||||
# no log. Convert it to a normal "not reachable" ValueError instead.
|
||||
raise ValueError("Joplin API is not reachable") from None
|
||||
try:
|
||||
data = json.loads(raw)
|
||||
|
||||
@@ -164,7 +164,7 @@ def _build_csp_report_only_policy() -> str:
|
||||
|
||||
from api.auth import check_auth
|
||||
from api.config import HOST, PORT, STATE_DIR, SESSION_DIR, DEFAULT_WORKSPACE
|
||||
from api.helpers import j, get_profile_cookie
|
||||
from api.helpers import j, get_profile_cookie, _CLIENT_DISCONNECT_ERRORS
|
||||
from api.profiles import set_request_profile, clear_request_profile
|
||||
from api.routes import handle_delete, handle_get, handle_patch, handle_post, handle_put
|
||||
from api.startup import auto_install_agent_deps, fix_credential_permissions
|
||||
@@ -311,14 +311,22 @@ class Handler(BaseHTTPRequestHandler):
|
||||
result = handle_get(self, parsed)
|
||||
if result is False:
|
||||
return j(self, {'error': 'not found'}, status=404)
|
||||
except (BrokenPipeError, ConnectionResetError, ConnectionAbortedError):
|
||||
except _CLIENT_DISCONNECT_ERRORS:
|
||||
# The browser/client closed the socket while we were writing the
|
||||
# response. This is expected for probes, tab closes, and SSE
|
||||
# reconnect races; do not convert it into a misleading server 500.
|
||||
return
|
||||
except Exception as e:
|
||||
except Exception:
|
||||
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
|
||||
return j(self, {'error': 'Internal server error'}, status=500)
|
||||
try:
|
||||
j(self, {'error': 'Internal server error'}, status=500)
|
||||
except _CLIENT_DISCONNECT_ERRORS:
|
||||
# Client disconnected while we were sending the 500 — nothing to do.
|
||||
pass
|
||||
except Exception:
|
||||
# Unexpected failure while sending the error response itself.
|
||||
# Log it so we know something is wrong with our error handler.
|
||||
traceback.print_exc()
|
||||
finally:
|
||||
clear_request_profile()
|
||||
|
||||
@@ -342,14 +350,22 @@ class Handler(BaseHTTPRequestHandler):
|
||||
result = route_func(self, parsed)
|
||||
if result is False:
|
||||
return j(self, {'error': 'not found'}, status=404)
|
||||
except (BrokenPipeError, ConnectionResetError, ConnectionAbortedError):
|
||||
except _CLIENT_DISCONNECT_ERRORS:
|
||||
# The browser/client closed the socket while we were writing the
|
||||
# response. This is expected for probes, tab closes, and SSE
|
||||
# reconnect races; do not convert it into a misleading server 500.
|
||||
return
|
||||
except Exception as e:
|
||||
except Exception:
|
||||
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
|
||||
return j(self, {'error': 'Internal server error'}, status=500)
|
||||
try:
|
||||
j(self, {'error': 'Internal server error'}, status=500)
|
||||
except _CLIENT_DISCONNECT_ERRORS:
|
||||
# Client disconnected while we were sending the 500 — nothing to do.
|
||||
pass
|
||||
except Exception:
|
||||
# Unexpected failure while sending the error response itself.
|
||||
# Log it so we know something is wrong with our error handler.
|
||||
traceback.print_exc()
|
||||
finally:
|
||||
clear_request_profile()
|
||||
|
||||
|
||||
@@ -0,0 +1,401 @@
|
||||
"""
|
||||
Test for BrokenPipeError + SSL BAD_LENGTH cascading failure fix.
|
||||
|
||||
When a client disconnects mid-response:
|
||||
1. First write raises BrokenPipeError (or ssl.SSLError on TLS)
|
||||
2. Exception handler tries to send 500 JSON through same broken socket
|
||||
3. Without the fix, this produces a second exception and noisy traceback
|
||||
|
||||
The fix:
|
||||
- helpers._safe_write() catches _CLIENT_DISCONNECT_ERRORS (including ssl.SSLError)
|
||||
- server.py exception handlers wrap their 500-response j() calls in try/except
|
||||
"""
|
||||
import ssl
|
||||
import unittest
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import sys
|
||||
import os
|
||||
|
||||
# Ensure project root is on path
|
||||
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||
|
||||
from api.helpers import j, t, _safe_write, _CLIENT_DISCONNECT_ERRORS
|
||||
|
||||
|
||||
class MockHandler:
|
||||
"""Minimal mock of BaseHTTPRequestHandler for testing response helpers."""
|
||||
|
||||
def __init__(self, write_raises=None, end_headers_raises=None):
|
||||
self._write_raises = write_raises
|
||||
self._end_headers_raises = end_headers_raises
|
||||
self.headers = {}
|
||||
self._wfile = MagicMock()
|
||||
self._sent_headers = []
|
||||
self._response_status = None
|
||||
|
||||
@property
|
||||
def wfile(self):
|
||||
return self._wfile
|
||||
|
||||
def send_response(self, status):
|
||||
self._response_status = status
|
||||
|
||||
def send_header(self, key, value):
|
||||
self._sent_headers.append((key, value))
|
||||
|
||||
def end_headers(self):
|
||||
if self._end_headers_raises:
|
||||
raise self._end_headers_raises
|
||||
|
||||
def _wfile_write(self, data):
|
||||
if self._write_raises:
|
||||
raise self._write_raises
|
||||
return len(data)
|
||||
|
||||
|
||||
class TestSafeWrite(unittest.TestCase):
|
||||
"""Test _safe_write swallows client disconnect errors without raising."""
|
||||
|
||||
def _make_handler(self, end_headers_raises=None, write_raises=None):
|
||||
handler = MockHandler(
|
||||
end_headers_raises=end_headers_raises,
|
||||
write_raises=write_raises,
|
||||
)
|
||||
handler.wfile.write = handler._wfile_write
|
||||
return handler
|
||||
|
||||
def test_safe_write_success(self):
|
||||
handler = self._make_handler()
|
||||
_safe_write(handler, b"hello")
|
||||
self.assertEqual(handler._response_status, None) # send_response not called by _safe_write
|
||||
|
||||
def test_safe_write_broken_pipe(self):
|
||||
handler = self._make_handler(write_raises=BrokenPipeError())
|
||||
# Should NOT raise
|
||||
_safe_write(handler, b"hello")
|
||||
|
||||
def test_safe_write_connection_reset(self):
|
||||
handler = self._make_handler(write_raises=ConnectionResetError())
|
||||
_safe_write(handler, b"hello")
|
||||
|
||||
def test_safe_write_connection_aborted(self):
|
||||
handler = self._make_handler(write_raises=ConnectionAbortedError())
|
||||
_safe_write(handler, b"hello")
|
||||
|
||||
def test_safe_write_timeout(self):
|
||||
handler = self._make_handler(write_raises=TimeoutError())
|
||||
_safe_write(handler, b"hello")
|
||||
|
||||
def test_safe_write_ssl_bad_length(self):
|
||||
handler = self._make_handler(
|
||||
write_raises=ssl.SSLError("[BAD_LENGTH] write failed")
|
||||
)
|
||||
_safe_write(handler, b"hello")
|
||||
|
||||
def test_safe_write_end_headers_broken_pipe(self):
|
||||
handler = self._make_handler(end_headers_raises=BrokenPipeError())
|
||||
_safe_write(handler, b"hello")
|
||||
|
||||
def test_safe_write_end_headers_ssl_error(self):
|
||||
handler = self._make_handler(
|
||||
end_headers_raises=ssl.SSLError("[BAD_LENGTH]")
|
||||
)
|
||||
_safe_write(handler, b"hello")
|
||||
|
||||
def test_safe_write_other_error_propagates(self):
|
||||
handler = self._make_handler(write_raises=ValueError("unexpected"))
|
||||
with self.assertRaises(ValueError):
|
||||
_safe_write(handler, b"hello")
|
||||
|
||||
|
||||
class TestJsonHelper(unittest.TestCase):
|
||||
"""Test j() helper with disconnect errors."""
|
||||
|
||||
def _make_handler(self, end_headers_raises=None, write_raises=None):
|
||||
handler = MockHandler(
|
||||
end_headers_raises=end_headers_raises,
|
||||
write_raises=write_raises,
|
||||
)
|
||||
handler.wfile.write = handler._wfile_write
|
||||
return handler
|
||||
|
||||
def test_j_success(self):
|
||||
handler = self._make_handler()
|
||||
j(handler, {"ok": True}, status=200)
|
||||
self.assertEqual(handler._response_status, 200)
|
||||
self.assertTrue(any(h[0] == "Content-Type" for h in handler._sent_headers))
|
||||
|
||||
def test_j_broken_pipe_on_write(self):
|
||||
handler = self._make_handler(write_raises=BrokenPipeError())
|
||||
# Should NOT raise — headers sent, write fails silently
|
||||
j(handler, {"ok": True}, status=200)
|
||||
self.assertEqual(handler._response_status, 200)
|
||||
|
||||
def test_j_ssl_error_on_write(self):
|
||||
handler = self._make_handler(
|
||||
write_raises=ssl.SSLError("[BAD_LENGTH] write failed")
|
||||
)
|
||||
j(handler, {"ok": True}, status=200)
|
||||
self.assertEqual(handler._response_status, 200)
|
||||
|
||||
def test_j_broken_pipe_on_end_headers(self):
|
||||
handler = self._make_handler(end_headers_raises=BrokenPipeError())
|
||||
j(handler, {"ok": True}, status=200)
|
||||
self.assertEqual(handler._response_status, 200)
|
||||
|
||||
|
||||
class TestTextHelper(unittest.TestCase):
|
||||
"""Test t() helper with disconnect errors."""
|
||||
|
||||
def _make_handler(self, end_headers_raises=None, write_raises=None):
|
||||
handler = MockHandler(
|
||||
end_headers_raises=end_headers_raises,
|
||||
write_raises=write_raises,
|
||||
)
|
||||
handler.wfile.write = handler._wfile_write
|
||||
return handler
|
||||
|
||||
def test_t_success(self):
|
||||
handler = self._make_handler()
|
||||
t(handler, "hello", status=200)
|
||||
self.assertEqual(handler._response_status, 200)
|
||||
|
||||
def test_t_broken_pipe(self):
|
||||
handler = self._make_handler(write_raises=BrokenPipeError())
|
||||
t(handler, "hello", status=200)
|
||||
self.assertEqual(handler._response_status, 200)
|
||||
|
||||
def test_t_ssl_error(self):
|
||||
handler = self._make_handler(
|
||||
write_raises=ssl.SSLError("[BAD_LENGTH]")
|
||||
)
|
||||
t(handler, "hello", status=200)
|
||||
self.assertEqual(handler._response_status, 200)
|
||||
|
||||
|
||||
class TestClientDisconnectErrorsTuple(unittest.TestCase):
|
||||
"""Verify _CLIENT_DISCONNECT_ERRORS includes all expected types."""
|
||||
|
||||
def test_includes_broken_pipe(self):
|
||||
self.assertIn(BrokenPipeError, _CLIENT_DISCONNECT_ERRORS)
|
||||
|
||||
def test_includes_connection_reset(self):
|
||||
self.assertIn(ConnectionResetError, _CLIENT_DISCONNECT_ERRORS)
|
||||
|
||||
def test_includes_connection_aborted(self):
|
||||
self.assertIn(ConnectionAbortedError, _CLIENT_DISCONNECT_ERRORS)
|
||||
|
||||
def test_includes_timeout(self):
|
||||
self.assertIn(TimeoutError, _CLIENT_DISCONNECT_ERRORS)
|
||||
|
||||
def test_includes_ssl_error(self):
|
||||
self.assertIn(ssl.SSLError, _CLIENT_DISCONNECT_ERRORS)
|
||||
|
||||
def test_excludes_broad_oserror(self):
|
||||
"""OSError is too broad — it masks real errors like file-not-found."""
|
||||
self.assertNotIn(OSError, _CLIENT_DISCONNECT_ERRORS)
|
||||
|
||||
|
||||
class TestServerDisconnectHandling(unittest.TestCase):
|
||||
"""Test server.py skips 500 response when client disconnects."""
|
||||
|
||||
def _make_handler(self, route_raises=None):
|
||||
"""Build a Handler with mocked socket and route."""
|
||||
from server import Handler
|
||||
handler = Handler.__new__(Handler)
|
||||
handler.command = "GET"
|
||||
handler.path = "/api/test"
|
||||
handler._req_t0 = 0.0
|
||||
handler.headers = {}
|
||||
handler.wfile = MagicMock()
|
||||
handler.wfile.write = MagicMock()
|
||||
handler.send_response = MagicMock()
|
||||
handler.send_header = MagicMock()
|
||||
handler.end_headers = MagicMock()
|
||||
handler._route_raises = route_raises
|
||||
return handler
|
||||
|
||||
def test_do_get_skips_500_on_broken_pipe(self):
|
||||
from server import Handler
|
||||
handler = self._make_handler(route_raises=BrokenPipeError())
|
||||
|
||||
def _fake_handle_get(self, parsed):
|
||||
raise self._route_raises
|
||||
|
||||
# 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()
|
||||
|
||||
def test_handle_write_skips_500_on_connection_reset(self):
|
||||
from server import Handler
|
||||
handler = self._make_handler(route_raises=ConnectionResetError())
|
||||
handler.command = "POST"
|
||||
|
||||
def _fake_route(self, parsed):
|
||||
raise self._route_raises
|
||||
|
||||
import server as _server_mod
|
||||
orig_check_auth = _server_mod.check_auth
|
||||
_server_mod.check_auth = lambda h, p: True
|
||||
try:
|
||||
Handler._handle_write(handler, _fake_route)
|
||||
finally:
|
||||
_server_mod.check_auth = orig_check_auth
|
||||
|
||||
handler.send_response.assert_not_called()
|
||||
|
||||
def test_do_get_sends_500_on_real_error(self):
|
||||
from server import Handler
|
||||
handler = self._make_handler(route_raises=ValueError("real bug"))
|
||||
|
||||
def _fake_handle_get(self, parsed):
|
||||
raise self._route_raises
|
||||
|
||||
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
|
||||
|
||||
# Should send 500 for real errors
|
||||
handler.send_response.assert_called_once_with(500)
|
||||
|
||||
def test_do_get_skips_500_on_ssl_disconnect(self):
|
||||
"""SSL disconnect during route handling should not trigger 500."""
|
||||
from server import Handler
|
||||
handler = self._make_handler(route_raises=ssl.SSLError("[BAD_LENGTH]"))
|
||||
|
||||
def _fake_handle_get(self, parsed):
|
||||
raise self._route_raises
|
||||
|
||||
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()
|
||||
|
||||
def test_do_get_skips_500_on_timeout_disconnect(self):
|
||||
"""Timeout disconnect during route handling should not trigger 500."""
|
||||
from server import Handler
|
||||
handler = self._make_handler(route_raises=TimeoutError())
|
||||
|
||||
def _fake_handle_get(self, parsed):
|
||||
raise self._route_raises
|
||||
|
||||
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()
|
||||
|
||||
|
||||
class TestServer500ResponseSafety(unittest.TestCase):
|
||||
"""Test that 500-response failures are handled gracefully."""
|
||||
|
||||
def _make_handler(self, route_raises=None, json_write_raises=None):
|
||||
"""Build a Handler where the 500-response j() call may also fail."""
|
||||
from server import Handler
|
||||
handler = Handler.__new__(Handler)
|
||||
handler.command = "GET"
|
||||
handler.path = "/api/test"
|
||||
handler._req_t0 = 0.0
|
||||
handler.headers = {}
|
||||
handler.wfile = MagicMock()
|
||||
handler._json_write_raises = json_write_raises
|
||||
|
||||
def _raising_write(data):
|
||||
if handler._json_write_raises:
|
||||
raise handler._json_write_raises
|
||||
return len(data)
|
||||
|
||||
handler.wfile.write = _raising_write
|
||||
handler.send_response = MagicMock()
|
||||
handler.send_header = MagicMock()
|
||||
handler.end_headers = MagicMock()
|
||||
handler._route_raises = route_raises
|
||||
return handler
|
||||
|
||||
def test_500_response_survives_client_disconnect(self):
|
||||
"""If the 500-response itself hits a disconnect, don't crash."""
|
||||
from server import Handler
|
||||
handler = self._make_handler(
|
||||
route_raises=ValueError("real bug"),
|
||||
json_write_raises=BrokenPipeError(),
|
||||
)
|
||||
|
||||
def _fake_handle_get(self, parsed):
|
||||
raise self._route_raises
|
||||
|
||||
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 WAS called (we tried to send 500), but write failed
|
||||
handler.send_response.assert_called_once_with(500)
|
||||
|
||||
def test_500_response_logs_unexpected_failure(self):
|
||||
"""If the 500-response fails for a NON-disconnect reason, log it."""
|
||||
from server import Handler
|
||||
handler = self._make_handler(
|
||||
route_raises=ValueError("real bug"),
|
||||
json_write_raises=RuntimeError("json serializer exploded"),
|
||||
)
|
||||
|
||||
def _fake_handle_get(self, parsed):
|
||||
raise self._route_raises
|
||||
|
||||
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 WAS called (we tried to send 500), but write failed
|
||||
handler.send_response.assert_called_once_with(500)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -138,6 +138,30 @@ def test_joplin_get_note_validates_id_and_truncates_body(monkeypatch):
|
||||
assert "Preview truncated" in note["body"]
|
||||
|
||||
|
||||
def test_joplin_api_get_converts_bare_timeout_to_valueerror(monkeypatch):
|
||||
"""Regression (#3210 review): a bare socket TimeoutError from urlopen must
|
||||
surface as a ValueError, not propagate.
|
||||
|
||||
`urlopen(timeout=8)` can raise a bare ``TimeoutError`` (not URLError-wrapped)
|
||||
on a slow/unreachable Joplin server. After the consolidated client-disconnect
|
||||
handling landed, ``TimeoutError`` is in the request dispatch's
|
||||
``_CLIENT_DISCONNECT_ERRORS`` set — so if it escaped ``_joplin_api_get`` it
|
||||
would be swallowed as a fake client disconnect (silent empty response, no
|
||||
log). It must be converted to a "not reachable" ValueError at the route.
|
||||
"""
|
||||
import pytest
|
||||
from api import routes
|
||||
|
||||
def fake_urlopen(request, timeout):
|
||||
raise TimeoutError("timed out")
|
||||
|
||||
monkeypatch.setattr(routes, "_joplin_connection_from_config", lambda: ("http://127.0.0.1:41184", "secret-token"))
|
||||
monkeypatch.setattr("urllib.request.urlopen", fake_urlopen)
|
||||
|
||||
with pytest.raises(ValueError, match="not reachable"):
|
||||
routes._joplin_api_get("/search", {"query": "hello world"})
|
||||
|
||||
|
||||
def test_joplin_api_get_sends_header_and_query_token_for_clip_search_compat(monkeypatch):
|
||||
from api import routes
|
||||
|
||||
|
||||
Reference in new issue
Block a user