mirror of
https://github.com/Routstr/routstr-core.git
synced 2026-10-05 12:28:22 +00:00
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.
This commit is contained in:
@@ -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,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user