From dca4a2a7af3fd3e299e2db5ae9132607d7ac8d68 Mon Sep 17 00:00:00 2001
From: nesquena-hermes <[email protected]>
Date: Sun, 31 May 2026 01:49:44 +0000
Subject: [PATCH] fix(#3210 review): convert bare urlopen TimeoutError to
 ValueError in _joplin_api_get
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 api/routes.py                     |  8 +++++++-
 tests/test_webui_notes_sources.py | 24 ++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/api/routes.py b/api/routes.py
index 16ac8297c..74d157409 100644
--- a/api/routes.py
+++ b/api/routes.py
@@ -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)
diff --git a/tests/test_webui_notes_sources.py b/tests/test_webui_notes_sources.py
index bac05eb58..f70d4d135 100644
--- a/tests/test_webui_notes_sources.py
+++ b/tests/test_webui_notes_sources.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