From 54d1f4c79830a7900a5af7042f53e54767daab39 Mon Sep 17 00:00:00 2001
From: redshift <213178690+1ftredsh@users.noreply.github.com>
Date: Tue, 8 Sep 2026 13:24:44 +0200
Subject: [PATCH] test(ehbp): cover bearer timeout path; tighten
 EhbpTimeoutError signature

Addresses two review items on #700.

- Add a bearer-path regression test. `forward_ehbp_request`'s trailing
  `except Exception` rewrites any error into a 500; only the
  `except UpstreamError: raise` above it preserves the 504. Deleting those
  two lines now fails this test and nothing else (verified locally).
- Drop `EhbpTimeoutError`'s unused `status_code` parameter and accept
  `details`, which `create_upstream_error_response` already forwards.
- Format the touched tests: `ruff format --check` was failing on
  tests/unit/test_ehbp_timeout.py.
---
 routstr/core/exceptions.py         |   8 ++-
 tests/unit/test_ehbp_timeout.py    | 100 +++++++++++++++++++++--------
 tests/unit/test_tinfoil_trailer.py |   9 +++
 3 files changed, 90 insertions(+), 27 deletions(-)

diff --git a/routstr/core/exceptions.py b/routstr/core/exceptions.py
index 6e7d90a0..2fc34bb6 100644
--- a/routstr/core/exceptions.py
+++ b/routstr/core/exceptions.py
@@ -40,13 +40,17 @@ class EhbpTimeoutError(UpstreamError):
     Distinct from a generic :class:`UpstreamError` so callers can map the
     failure to a ``504 Gateway Timeout`` with a stable ``UPSTREAM_TIMEOUT``
     code instead of a misleading ``500`` internal server error.
+
+    ``details`` carries optional structured, redaction-safe context and is
+    forwarded to the client by ``create_upstream_error_response``.
     """
 
-    def __init__(self, message: str, status_code: int = 504):
+    def __init__(self, message: str, details: dict[str, object] | None = None):
         super().__init__(
             message,
-            status_code=status_code,
+            status_code=504,
             code="UPSTREAM_TIMEOUT",
+            details=details,
         )
 
 
diff --git a/tests/unit/test_ehbp_timeout.py b/tests/unit/test_ehbp_timeout.py
index 760a491e..c9e749cb 100644
--- a/tests/unit/test_ehbp_timeout.py
+++ b/tests/unit/test_ehbp_timeout.py
@@ -4,7 +4,7 @@ from unittest.mock import AsyncMock, MagicMock
 
 import pytest
 
-from routstr.core.exceptions import EhbpTimeoutError
+from routstr.core.exceptions import EhbpTimeoutError, UpstreamError
 from routstr.upstream import ehbp as ehbp_module
 
 # ---------------------------------------------------------------------------
@@ -22,28 +22,8 @@ async def _request() -> MagicMock:
     return request
 
 
-@pytest.mark.asyncio
-async def test_x_cashu_timeout_refunds_and_returns_504(
-    monkeypatch: pytest.MonkeyPatch,
-) -> None:
-    monkeypatch.setattr(
-        ehbp_module,
-        "recieve_token",
-        AsyncMock(return_value=(1000, "msat", None)),
-    )
-    monkeypatch.setattr(
-        ehbp_module, "store_cashu_transaction", AsyncMock(return_value=None)
-    )
-    send_cashu_refund_mock = AsyncMock(return_value="refund-token")
-    monkeypatch.setattr(
-        ehbp_module, "send_cashu_refund", send_cashu_refund_mock
-    )
-    monkeypatch.setattr(
-        ehbp_module,
-        "forward_with_trailer",
-        AsyncMock(side_effect=EhbpTimeoutError("EHBP upstream timed out")),
-    )
-
+def _ehbp_upstream_mocks() -> tuple[MagicMock, MagicMock]:
+    """Upstream and model mocks sufficient to reach the forwarding call."""
     profile = MagicMock()
     profile.client_target_url_header = None
     profile.allow_client_target_override = False
@@ -64,6 +44,30 @@ async def test_x_cashu_timeout_refunds_and_returns_504(
     model_obj = MagicMock()
     model_obj.id = "tinfoil-kimi-k2-6"
     model_obj.forwarded_model_id = "kimi-k2-6"
+    return upstream, model_obj
+
+
+@pytest.mark.asyncio
+async def test_x_cashu_timeout_refunds_and_returns_504(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setattr(
+        ehbp_module,
+        "recieve_token",
+        AsyncMock(return_value=(1000, "msat", None)),
+    )
+    monkeypatch.setattr(
+        ehbp_module, "store_cashu_transaction", AsyncMock(return_value=None)
+    )
+    send_cashu_refund_mock = AsyncMock(return_value="refund-token")
+    monkeypatch.setattr(ehbp_module, "send_cashu_refund", send_cashu_refund_mock)
+    monkeypatch.setattr(
+        ehbp_module,
+        "forward_with_trailer",
+        AsyncMock(side_effect=EhbpTimeoutError("EHBP upstream timed out")),
+    )
+
+    upstream, model_obj = _ehbp_upstream_mocks()
 
     response = await ehbp_module.forward_ehbp_x_cashu_request(
         request=await _request(),
@@ -76,6 +80,52 @@ async def test_x_cashu_timeout_refunds_and_returns_504(
 
     assert response.status_code == 504
     assert response.headers["X-Cashu"] == "refund-token"
-    send_cashu_refund_mock.assert_awaited_once_with(
-        1000, "msat", None, "req-123"
+    send_cashu_refund_mock.assert_awaited_once_with(1000, "msat", None, "req-123")
+
+
+# ---------------------------------------------------------------------------
+# forward_ehbp_request — the bearer path must let the timeout through, so
+# proxy.py can answer 504 instead of flattening it to a generic 500
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_bearer_timeout_propagates_504(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """A timed-out bearer request must not be rewritten to a 500.
+
+    ``forward_ehbp_request`` ends in a bare ``except Exception`` that turns any
+    error into ``UpstreamError(..., status_code=500)``. The ``except
+    UpstreamError: raise`` above it is the only thing preserving the 504 that
+    ``proxy.py`` returns to the client, so this test pins that handler.
+    """
+    monkeypatch.setattr(
+        ehbp_module,
+        "forward_with_trailer",
+        AsyncMock(
+            side_effect=EhbpTimeoutError(
+                "EHBP upstream inference.tinfoil.sh timed out after 60s connecting"
+            )
+        ),
     )
+    upstream, model_obj = _ehbp_upstream_mocks()
+    key = MagicMock()
+    key.hashed_key = "abcdef1234567890"
+
+    with pytest.raises(EhbpTimeoutError) as exc_info:
+        await ehbp_module.forward_ehbp_request(
+            request=await _request(),
+            path="v1/chat/completions",
+            headers={},
+            request_body=b"opaque",
+            upstream=upstream,
+            key=key,
+            max_cost_for_model=5000,
+            session=MagicMock(),
+            model_obj=model_obj,
+        )
+
+    assert exc_info.value.status_code == 504
+    assert exc_info.value.code == "UPSTREAM_TIMEOUT"
+    assert isinstance(exc_info.value, UpstreamError)
diff --git a/tests/unit/test_tinfoil_trailer.py b/tests/unit/test_tinfoil_trailer.py
index 5a6d68a7..3e4d3e0f 100644
--- a/tests/unit/test_tinfoil_trailer.py
+++ b/tests/unit/test_tinfoil_trailer.py
@@ -195,4 +195,13 @@ def test_ehbp_timeout_error_metadata() -> None:
     exc = EhbpTimeoutError("boom")
     assert exc.status_code == 504
     assert exc.code == "UPSTREAM_TIMEOUT"
+    assert exc.details is None
     assert isinstance(exc, UpstreamError)
+
+
+def test_ehbp_timeout_error_forwards_details() -> None:
+    """``details`` must survive so the response builder can forward it."""
+    exc = EhbpTimeoutError("boom", details={"phase": "connect"})
+    assert exc.details == {"phase": "connect"}
+    assert exc.status_code == 504
+    assert exc.code == "UPSTREAM_TIMEOUT"