mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-14 11:40:44 +00:00
fix(#3210 review): convert bare urlopen TimeoutError to ValueError in _joplin_api_get
Codex+Opus pre-release gate both flagged: TimeoutError is now in the consolidated _CLIENT_DISCONNECT_ERRORS dispatch set, so a bare socket-connect TimeoutError from Joplins urlopen(timeout=8) — which is NOT always URLError- wrapped — would escape _handle_notes_search and be swallowed by the dispatch disconnect handler as a fake client disconnect (silent empty response, no log). Catch (URLError, TimeoutError) at the route so it surfaces as a clean "not reachable" ValueError -> JSON error. Adds a regression test. Co-authored-by: someaka <someaka@users.noreply.github.com>
This commit is contained in:
+7
-1
@@ -13276,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)
|
||||
|
||||
@@ -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