From 21489023fc4b64fa922e0c2efe46f9b66aa17bd8 Mon Sep 17 00:00:00 2001 From: 9qeklajc Date: Fri, 25 Sep 2026 02:01:13 +0200 Subject: [PATCH] fix: report upstream 5xx as 424 UPSTREAM_UNAVAILABLE instead of node-down --- docs/api/endpoints.md | 4 +- docs/api/errors.md | 68 +++++++- docs/api/overview.md | 7 +- routstr/core/error_scope.py | 60 +++++++ routstr/core/exceptions.py | 15 +- routstr/core/main.py | 1 + routstr/payment/helpers.py | 45 ++++- routstr/proxy.py | 16 +- routstr/upstream/base.py | 86 ++++++--- routstr/upstream/ehbp.py | 34 +++- routstr/upstream/gemini_messages.py | 2 + routstr/upstream/tinfoil.py | 31 ++++ tests/unit/test_ehbp_timeout.py | 28 ++- tests/unit/test_messages_litellm_dispatch.py | 6 +- tests/unit/test_model_path_routing.py | 101 ++++++++++- .../test_proxy_tinfoil_attestation_routing.py | 146 ++++++++++++++++ tests/unit/test_tinfoil_trailer.py | 6 +- tests/unit/test_upstream_error_response.py | 165 +++++++++++++++++- tests/unit/test_upstream_rate_limit.py | 22 ++- 19 files changed, 764 insertions(+), 79 deletions(-) create mode 100644 routstr/core/error_scope.py diff --git a/docs/api/endpoints.md b/docs/api/endpoints.md index 5ead333e..d8b610ca 100644 --- a/docs/api/endpoints.md +++ b/docs/api/endpoints.md @@ -264,7 +264,9 @@ Billing is input-token based (output tokens are free on Jev); the response's - TypeSafe's `GET /v1/models` lists aliases only; the node additionally seeds the known versioned ids so they can be requested directly. - TypeSafe answers `429 Too Many Requests` and `529 Overloaded` when throttled. - Both are forwarded as upstream errors; retry with exponential backoff. + Both are forwarded as upstream errors; retry with exponential backoff. The + `429` keeps its status; the `529` is reported as `424` + (see [Upstream attribution](errors.md#upstream-attribution-424-failed-dependency)). **Enabling the provider:** diff --git a/docs/api/errors.md b/docs/api/errors.md index 82bb5be1..5f151bb6 100644 --- a/docs/api/errors.md +++ b/docs/api/errors.md @@ -51,11 +51,50 @@ legacy status behavior. | 403 | Forbidden | Access denied to resource | | 404 | Not Found | Endpoint or resource doesn't exist | | 422 | Unprocessable Entity | Validation errors | -| 429 | Too Many Requests | Rate limit exceeded | -| 500 | Internal Server Error | Server-side error | -| 502 | Bad Gateway | Upstream API error | +| 424 | Failed Dependency | An upstream inference provider failed. This node is healthy — see [Upstream attribution](#upstream-attribution-424-failed-dependency) | +| 429 | Too Many Requests | Rate limit exceeded (this node or an upstream provider) | +| 500 | Internal Server Error | Server-side error on this node | +| 502 | Bad Gateway | Gateway-level failure | | 503 | Service Unavailable | Temporary outage | +### Upstream attribution (424 Failed Dependency) + +When an upstream provider fails, this node is still healthy, so the failure is +reported as a **non-5xx** status. Clients should not mark the node down for it. + +An upstream-attributable failure answers: + +- **Status:** `424` +- **`error.code`:** `UPSTREAM_UNAVAILABLE` +- **Header:** `X-Routstr-Error-Scope: upstream` +- **`error.upstream_status`:** the provider's own status (e.g. `503`). Failures + built by the payment helpers carry it in `error.details.upstream_status` + instead + +```http +HTTP/1.1 424 Failed Dependency +X-Routstr-Error-Scope: upstream +Content-Type: application/json + +{ + "error": { + "type": "upstream_error", + "message": "Service Unavailable", + "code": "UPSTREAM_UNAVAILABLE", + "upstream_status": 503 + } +} +``` + +Two exceptions keep their own status: + +- **Rate limits** answer `429` with `error.code = UPSTREAM_RATE_LIMIT`, even + when the provider wrapped them in a 5xx. +- **Provider-side 4xx** (`400`/`401`/`403`/`404`/`422`) passes through unchanged. + +Node faults (unreachable mint, database failure, internal exception) still +answer `500` with **no** `X-Routstr-Error-Scope` header. + ## Error Types ### Authentication Errors @@ -329,14 +368,18 @@ Retry-After: 45 ### Upstream Errors -#### Model Overloaded +#### Upstream Unavailable + +A provider returned a 5xx (overloaded, bad gateway, timeout, or a provider-side +outage). This node is healthy and your reservation has been reverted. ```json { "error": { "type": "upstream_error", "message": "Model is currently overloaded", - "code": "model_overloaded", + "code": "UPSTREAM_UNAVAILABLE", + "upstream_status": 503, "details": { "model": "gpt-4", "retry_after": 5 @@ -345,8 +388,11 @@ Retry-After: 45 } ``` -**Status:** 503 -**Resolution:** Retry request after delay +**Status:** 424 +**Header:** `X-Routstr-Error-Scope: upstream` +**Resolution:** Retry after a short backoff. If the node is configured with +alternative providers for the model, it already retried them before answering — +try another model or provider path if the failure persists. #### Upstream Timeout @@ -364,7 +410,8 @@ Retry-After: 45 } ``` -**Status:** 504 +**Status:** 424 +**Header:** `X-Routstr-Error-Scope: upstream` **Resolution:** Retry with shorter prompt or max_tokens ### Content Policy @@ -416,7 +463,7 @@ def retry_with_backoff( # Check if error is retryable if hasattr(e, 'status_code'): - if e.status_code in [429, 502, 503, 504]: + if e.status_code in [424, 429, 502, 503, 504]: # Calculate delay with jitter delay = min( base_delay * (2 ** attempt) + random.uniform(0, 1), @@ -441,6 +488,9 @@ Group errors for handling: class ErrorHandler: # Errors that should be retried RETRYABLE_ERRORS = { + 'UPSTREAM_UNAVAILABLE', # upstream 5xx, reported as HTTP 424 + 'UPSTREAM_RATE_LIMIT', # HTTP 429 + 'UPSTREAM_TIMEOUT', # EHBP upstream timeout, reported as HTTP 424 'rate_limit', 'upstream_timeout', 'model_overloaded', diff --git a/docs/api/overview.md b/docs/api/overview.md index c8069953..8a4a445d 100644 --- a/docs/api/overview.md +++ b/docs/api/overview.md @@ -91,7 +91,7 @@ All errors follow a consistent format: | `not_found` | 404 | Resource not found | | `rate_limit_exceeded` | 429 | Too many requests | | `internal_error` | 500 | Server error | -| `upstream_error` | 502 | Upstream API error | +| `upstream_error` | 424 | Upstream API error — the provider failed, this node is healthy. Carries `error.code = UPSTREAM_UNAVAILABLE`, the `X-Routstr-Error-Scope: upstream` header, and the provider's own status in `error.upstream_status`. Rate limits stay `429` + `UPSTREAM_RATE_LIMIT`. See [Error Handling](errors.md#upstream-attribution-424-failed-dependency) | ## Endpoint Categories @@ -268,9 +268,10 @@ X-Webhook-Signature: sha256=... | 402 | Payment required | | 403 | Forbidden | | 404 | Not found | +| 424 | Upstream provider failed (`X-Routstr-Error-Scope: upstream`) | | 429 | Rate limited | -| 500 | Server error | -| 502 | Upstream error | +| 500 | Server error (no scope header) | +| 502 | Gateway failure | | 503 | Service unavailable | ## CORS Support diff --git a/routstr/core/error_scope.py b/routstr/core/error_scope.py new file mode 100644 index 00000000..9cc8306a --- /dev/null +++ b/routstr/core/error_scope.py @@ -0,0 +1,60 @@ +"""Attribution scope for upstream-caused failures. + +An upstream 5xx forwarded verbatim makes callers think this node is down. +Upstream failures are therefore reported as ``424`` with +``error.code = UPSTREAM_UNAVAILABLE``, the ``X-Routstr-Error-Scope: upstream`` +header, and the provider's status in ``upstream_status``. Rate limits keep +``429``. Node faults keep ``500`` and carry no scope header. +""" + +from __future__ import annotations + +UPSTREAM_UNAVAILABLE = "UPSTREAM_UNAVAILABLE" +# Deliberately not 5xx: an upstream blip must not read as node health. +UPSTREAM_ERROR_STATUS = 424 + +ERROR_SCOPE_HEADER = "X-Routstr-Error-Scope" +ERROR_SCOPE_UPSTREAM = "upstream" +ERROR_SCOPE_NODE = "node" + + +def _is_rate_limit_code(code: object) -> bool: + # Lazy import: routstr.upstream imports this module. + from ..upstream.rate_limit import UPSTREAM_RATE_LIMIT + + return code == UPSTREAM_RATE_LIMIT + + +def client_status_for_upstream_error( + status_code: int | None, code: object = None +) -> int: + """Map an upstream status to the one the caller sees: 429 and 4xx pass + through, 5xx (or unknown) becomes :data:`UPSTREAM_ERROR_STATUS`.""" + if _is_rate_limit_code(code): + return 429 + if not status_code or status_code >= 500: + return UPSTREAM_ERROR_STATUS + return status_code + + +def client_code_for_upstream_error( + status_code: int | None, code: str | int | None +) -> str | int | None: + """Return the ``error.code`` matching :func:`client_status_for_upstream_error`.""" + if _is_rate_limit_code(code): + return code + if not status_code or status_code >= 500: + return UPSTREAM_UNAVAILABLE + return code + + +def upstream_status_details( + details: dict[str, object] | None, upstream_status: int | None +) -> dict[str, object] | None: + """Add ``upstream_status`` to ``details`` when the caller sees a different status.""" + merged: dict[str, object] = dict(details) if details else {} + if upstream_status and upstream_status != client_status_for_upstream_error( + upstream_status + ): + merged["upstream_status"] = upstream_status + return merged or None diff --git a/routstr/core/exceptions.py b/routstr/core/exceptions.py index 88e0d370..6827284e 100644 --- a/routstr/core/exceptions.py +++ b/routstr/core/exceptions.py @@ -5,6 +5,7 @@ from fastapi.encoders import jsonable_encoder from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse +from .error_scope import ERROR_SCOPE_UPSTREAM, UPSTREAM_ERROR_STATUS from .logging import get_logger logger = get_logger(__name__) @@ -23,6 +24,11 @@ class UpstreamError(Exception): the upstream itself answered with, as opposed to a status this proxy chose for a transport failure, timeout or internal fault. Callers use it to decide whether a status is safe to retry. + + ``scope`` is ``"upstream"`` for provider failures, reported to the caller + as ``424`` (see :mod:`routstr.core.error_scope`), or ``"node"`` for local + faults, which keep their status. ``status_code`` stays the provider's own + status; the caller-visible mapping happens at response construction. """ def __init__( @@ -32,12 +38,14 @@ class UpstreamError(Exception): code: str | None = None, details: dict[str, object] | None = None, from_upstream_response: bool = False, + scope: str = ERROR_SCOPE_UPSTREAM, ): self.message = message self.status_code = status_code self.code = code self.details = details self.from_upstream_response = from_upstream_response + self.scope = scope super().__init__(message) @@ -45,8 +53,9 @@ class EhbpTimeoutError(UpstreamError): """Raised when an EHBP upstream times out waiting for a response. 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. + failure to a stable ``UPSTREAM_TIMEOUT`` code instead of a misleading + ``500`` internal server error. Reported as ``424``: the timeout happened on + the provider hop, not this node. ``details`` carries optional structured, redaction-safe context and is forwarded to the client by ``create_upstream_error_response``. @@ -55,7 +64,7 @@ class EhbpTimeoutError(UpstreamError): def __init__(self, message: str, details: dict[str, object] | None = None): super().__init__( message, - status_code=504, + status_code=UPSTREAM_ERROR_STATUS, code="UPSTREAM_TIMEOUT", details=details, ) diff --git a/routstr/core/main.py b/routstr/core/main.py index 5ce22d22..bf1f4eea 100644 --- a/routstr/core/main.py +++ b/routstr/core/main.py @@ -289,6 +289,7 @@ app.add_middleware( expose_headers=[ "x-routstr-request-id", "x-cashu", + "x-routstr-error-scope", "x-routstr-cost-msats", "x-routstr-cost-usd", "x-routstr-input-cost-msats", diff --git a/routstr/payment/helpers.py b/routstr/payment/helpers.py index d088151f..4582031b 100644 --- a/routstr/payment/helpers.py +++ b/routstr/payment/helpers.py @@ -15,6 +15,14 @@ from PIL import Image from sqlmodel.ext.asyncio.session import AsyncSession from ..core import get_logger +from ..core.error_scope import ( + ERROR_SCOPE_HEADER, + ERROR_SCOPE_NODE, + ERROR_SCOPE_UPSTREAM, + client_code_for_upstream_error, + client_status_for_upstream_error, + upstream_status_details, +) from ..core.exceptions import UpstreamError from ..core.redaction import redact_org_ids from ..core.settings import settings @@ -654,13 +662,15 @@ def create_error_response( token: str | None = None, code: str | int | None = None, details: dict[str, object] | None = None, + error_scope: str | None = None, ) -> Response: """Create a standardized error response. ``code`` is a stable, machine-readable classification (e.g. ``UPSTREAM_RATE_LIMIT``); when omitted it defaults to the HTTP status code for backwards compatibility. ``details`` carries optional structured, - redaction-safe context. + redaction-safe context. ``error_scope`` is sent as the + :data:`ERROR_SCOPE_HEADER` response header. """ error_obj: dict[str, object] = { "message": redact_org_ids(message), @@ -669,6 +679,11 @@ def create_error_response( } if details is not None: error_obj["details"] = details + headers: dict[str, str] = {} + if token: + headers["X-Cashu"] = token + if error_scope is not None: + headers[ERROR_SCOPE_HEADER] = error_scope return Response( content=json.dumps( { @@ -678,7 +693,7 @@ def create_error_response( ), status_code=status_code, media_type="application/json", - headers={"X-Cashu": token} if token else {}, + headers=headers, ) @@ -687,13 +702,29 @@ def create_upstream_error_response( request: Request, fallback_status: int = 502, ) -> Response: - """Build an error response from an :class:`UpstreamError`, preserving its - structured ``code``, ``details``, and original ``status_code``.""" + """Build an error response from an :class:`UpstreamError`. + + Upstream-scoped errors are mapped via :mod:`routstr.core.error_scope`; + node-scoped errors keep their own status. + """ + status_code = error.status_code or fallback_status + code = getattr(error, "code", None) + details = getattr(error, "details", None) + if getattr(error, "scope", ERROR_SCOPE_UPSTREAM) == ERROR_SCOPE_NODE: + return create_error_response( + "upstream_error", + str(error), + status_code, + request=request, + code=code, + details=details, + ) return create_error_response( "upstream_error", str(error), - error.status_code or fallback_status, + client_status_for_upstream_error(status_code, code), request=request, - code=getattr(error, "code", None), - details=getattr(error, "details", None), + code=client_code_for_upstream_error(status_code, code), + details=upstream_status_details(details, status_code), + error_scope=ERROR_SCOPE_UPSTREAM, ) diff --git a/routstr/proxy.py b/routstr/proxy.py index 887c81f2..09617add 100644 --- a/routstr/proxy.py +++ b/routstr/proxy.py @@ -482,7 +482,7 @@ async def _proxy( headers = upstream.prepare_headers(dict(request.headers)) response = await upstream.forward_get_request(request, path, headers) if ( - response.status_code in [502, 429] + response.status_code in [424, 502, 429] and i < len(selected_upstreams) - 1 ): logger.warning( @@ -708,7 +708,7 @@ async def _proxy( headers = upstream.prepare_headers(dict(request.headers)) response = await upstream.forward_get_request(request, path, headers) - if response.status_code in [502, 429] and i < len(candidates) - 1: + if response.status_code in [424, 502, 429] and i < len(candidates) - 1: error_message = "" try: if hasattr(response, "body"): @@ -927,8 +927,16 @@ async def _proxy( break if response.status_code != 200: - # Check if we should retry (502 Upstream Error or 429 Rate Limit) - should_retry = response.status_code in [502, 429, 400, 401, 403, 404] + # 424 is an upstream failure re-reported by error_scope. + should_retry = response.status_code in [ + 424, + 502, + 429, + 400, + 401, + 403, + 404, + ] if should_retry and i < len(candidates) - 1: error_message = "" try: diff --git a/routstr/upstream/base.py b/routstr/upstream/base.py index bc7244e1..efebc1c6 100644 --- a/routstr/upstream/base.py +++ b/routstr/upstream/base.py @@ -31,6 +31,14 @@ from ..core.db import ( from ..core.db import ( store_cashu_transaction_with_retry as store_cashu_transaction, ) +from ..core.error_scope import ( + ERROR_SCOPE_HEADER, + ERROR_SCOPE_NODE, + ERROR_SCOPE_UPSTREAM, + client_code_for_upstream_error, + client_status_for_upstream_error, + upstream_status_details, +) from ..core.exceptions import UpstreamError from ..core.redaction import redact_org_ids from ..payment.cost_calculation import ( @@ -943,6 +951,10 @@ class BaseUpstreamProvider: error_code = UPSTREAM_RATE_LIMIT error_details = rate_limit.as_details() + client_status = client_status_for_upstream_error(status_code, error_code) + client_code = client_code_for_upstream_error(status_code, error_code) + headers[ERROR_SCOPE_HEADER] = ERROR_SCOPE_UPSTREAM + logger.warning( "Upstream %s returned %s for model=%s path=%s: %s", self.provider_type, @@ -1012,23 +1024,27 @@ class BaseUpstreamProvider: # ``org-*`` regex preserves the surrounding JSON structure. redacted_text = redact_org_ids(body_bytes.decode("utf-8", errors="ignore")) redacted_body = redacted_text.encode() - # Surface the stable rate-limit classification on the forwarded - # body so callers can switch on ``error.code`` without parsing the - # provider-specific message. Fall back to the redacted bytes if the - # body is not a JSON object with an ``error`` mapping. - if rate_limit is not None: + # Surface the stable classification on the forwarded body so callers + # can switch on ``error.code`` without parsing the provider-specific + # message. Fall back to the redacted bytes if the body is not a JSON + # object with an ``error`` mapping. + if rate_limit is not None or client_status != status_code: try: parsed = json.loads(redacted_text) err = parsed.get("error") if isinstance(parsed, dict) else None if isinstance(err, dict): - err["code"] = UPSTREAM_RATE_LIMIT - err["details"] = error_details + if rate_limit is not None: + err["code"] = UPSTREAM_RATE_LIMIT + err["details"] = error_details + if client_status != status_code: + err["code"] = client_code + err["upstream_status"] = status_code redacted_body = json.dumps(parsed).encode() except (ValueError, AttributeError): pass return Response( content=redacted_body, - status_code=status_code, + status_code=client_status, headers=headers, media_type=media_type, ) @@ -1041,7 +1057,7 @@ class BaseUpstreamProvider: error_obj: dict[str, object] = { "message": message or "Upstream returned a non-JSON error response", "type": "upstream_error", - "code": error_code, + "code": client_code, "upstream_status": status_code, "upstream_content_type": content_type or None, "upstream_body_preview": body_preview or None, @@ -1055,7 +1071,7 @@ class BaseUpstreamProvider: return Response( content=json.dumps(envelope).encode(), - status_code=status_code, + status_code=client_status, headers=headers, media_type="application/json", ) @@ -3401,7 +3417,11 @@ class BaseUpstreamProvider: ) # Don't revert here — proxy.py owns payment revert to avoid double-revert - raise UpstreamError("An unexpected server error occurred", status_code=500) + raise UpstreamError( + "An unexpected server error occurred", + status_code=500, + scope=ERROR_SCOPE_NODE, + ) supports_ehbp: bool = False @@ -3680,7 +3700,11 @@ class BaseUpstreamProvider: ) # Don't revert here — proxy.py owns payment revert to avoid double-revert - raise UpstreamError("An unexpected server error occurred", status_code=500) + raise UpstreamError( + "An unexpected server error occurred", + status_code=500, + scope=ERROR_SCOPE_NODE, + ) async def forward_get_request( self, @@ -4512,15 +4536,21 @@ class BaseUpstreamProvider: "error": { "message": "Error forwarding request to upstream", "type": "upstream_error", - "code": response.status_code, + "code": client_code_for_upstream_error( + response.status_code, None + ), + "upstream_status": response.status_code, "refund_token": refund_token, } } ), - status_code=response.status_code, + status_code=client_status_for_upstream_error( + response.status_code + ), media_type="application/json", ) error_response.headers["X-Cashu"] = refund_token + error_response.headers[ERROR_SCOPE_HEADER] = ERROR_SCOPE_UPSTREAM return error_response if _x_cashu_path_has_settlement_handler(path): @@ -4670,12 +4700,17 @@ class BaseUpstreamProvider: # Post-redemption the token is spent; a forwarding failure must not # be reported as a retryable redemption error (see handle_x_cashu). if redeemed: + upstream_status = getattr(e, "status_code", None) return create_error_response( "upstream_error", "Payment succeeded but the upstream request failed", - 502, + client_status_for_upstream_error(upstream_status), request=request, - code="upstream_request_failed", + code=client_code_for_upstream_error( + upstream_status, getattr(e, "code", None) + ), + details=upstream_status_details(None, upstream_status), + error_scope=ERROR_SCOPE_UPSTREAM, ) classified = classify_redemption_error(e) @@ -4809,15 +4844,21 @@ class BaseUpstreamProvider: "error": { "message": "Error forwarding Responses API request to upstream", "type": "upstream_error", - "code": response.status_code, + "code": client_code_for_upstream_error( + response.status_code, None + ), + "upstream_status": response.status_code, "refund_token": refund_token, } } ), - status_code=response.status_code, + status_code=client_status_for_upstream_error( + response.status_code + ), media_type="application/json", ) error_response.headers["X-Cashu"] = refund_token + error_response.headers[ERROR_SCOPE_HEADER] = ERROR_SCOPE_UPSTREAM return error_response if path.startswith("responses"): @@ -5421,12 +5462,17 @@ class BaseUpstreamProvider: # must not surface as a retryable mint_unreachable (spent-token retry # bait). Redemption classification only applies while not redeemed. if redeemed: + upstream_status = getattr(e, "status_code", None) return create_error_response( "upstream_error", "Payment succeeded but the upstream request failed", - 502, + client_status_for_upstream_error(upstream_status), request=request, - code="upstream_request_failed", + code=client_code_for_upstream_error( + upstream_status, getattr(e, "code", None) + ), + details=upstream_status_details(None, upstream_status), + error_scope=ERROR_SCOPE_UPSTREAM, ) classified = classify_redemption_error(e) diff --git a/routstr/upstream/ehbp.py b/routstr/upstream/ehbp.py index 0b077709..bb8f97e4 100644 --- a/routstr/upstream/ehbp.py +++ b/routstr/upstream/ehbp.py @@ -31,6 +31,14 @@ from ..core.db import ( from ..core.db import ( store_cashu_transaction_with_retry as store_cashu_transaction, ) +from ..core.error_scope import ( + ERROR_SCOPE_HEADER, + ERROR_SCOPE_NODE, + ERROR_SCOPE_UPSTREAM, + UPSTREAM_ERROR_STATUS, + client_code_for_upstream_error, + client_status_for_upstream_error, +) from ..core.exceptions import EhbpTimeoutError, UpstreamError from ..core.settings import settings from ..payment.cost_calculation import ( @@ -1032,7 +1040,11 @@ async def forward_ehbp_request( "traceback": tb, }, ) - raise UpstreamError("An unexpected server error occurred", status_code=500) + raise UpstreamError( + "An unexpected server error occurred", + status_code=500, + scope=ERROR_SCOPE_NODE, + ) async def forward_ehbp_x_cashu_request( @@ -1128,15 +1140,19 @@ async def forward_ehbp_x_cashu_request( "error": { "message": "Error forwarding EHBP request to upstream", "type": "upstream_error", - "code": resp.status_code, + "code": client_code_for_upstream_error( + resp.status_code, None + ), + "upstream_status": resp.status_code, "refund_token": refund_token, } } ), - status_code=resp.status_code, + status_code=client_status_for_upstream_error(resp.status_code), media_type="application/json", ) error_response.headers["X-Cashu"] = refund_token + error_response.headers[ERROR_SCOPE_HEADER] = ERROR_SCOPE_UPSTREAM return error_response # Compute refund from actual usage when available — check both @@ -1242,9 +1258,10 @@ async def forward_ehbp_x_cashu_request( error_response = create_error_response( "upstream_timeout", str(e), - 504, + UPSTREAM_ERROR_STATUS, request=request, code="UPSTREAM_TIMEOUT", + error_scope=ERROR_SCOPE_UPSTREAM, ) error_response.headers["X-Cashu"] = refund_token return error_response @@ -1260,9 +1277,10 @@ async def forward_ehbp_x_cashu_request( return create_error_response( "upstream_timeout", str(e), - 504, + UPSTREAM_ERROR_STATUS, request=request, code="UPSTREAM_TIMEOUT", + error_scope=ERROR_SCOPE_UPSTREAM, ) except Exception as e: @@ -1284,8 +1302,9 @@ async def forward_ehbp_x_cashu_request( error_response = create_error_response( "upstream_error", "EHBP request failed after token redemption; refunded token", - 502, + UPSTREAM_ERROR_STATUS, request=request, + error_scope=ERROR_SCOPE_UPSTREAM, ) error_response.headers["X-Cashu"] = refund_token return error_response @@ -1352,7 +1371,8 @@ async def forward_ehbp_x_cashu_request( return create_error_response( "cashu_error" if not redeemed else "upstream_error", f"EHBP X-Cashu request failed: {error_message}", - 400 if not redeemed else 502, + 400 if not redeemed else UPSTREAM_ERROR_STATUS, request=request, token=x_cashu_token if not redeemed else None, + error_scope=None if not redeemed else ERROR_SCOPE_UPSTREAM, ) diff --git a/routstr/upstream/gemini_messages.py b/routstr/upstream/gemini_messages.py index d822f70f..8bc87db7 100644 --- a/routstr/upstream/gemini_messages.py +++ b/routstr/upstream/gemini_messages.py @@ -52,6 +52,7 @@ from typing import Any, Callable import httpx from ..core import get_logger +from ..core.error_scope import ERROR_SCOPE_NODE from ..core.exceptions import UpstreamError from ..payment.models import Model from .messages_dispatch import ( @@ -112,6 +113,7 @@ def _translate_anthropic_to_openai(body: dict, model: str) -> dict: raise UpstreamError( "Failed to translate Anthropic body to OpenAI format", status_code=500, + scope=ERROR_SCOPE_NODE, ) return dict(translated) diff --git a/routstr/upstream/tinfoil.py b/routstr/upstream/tinfoil.py index 0928cbc9..63467fa4 100644 --- a/routstr/upstream/tinfoil.py +++ b/routstr/upstream/tinfoil.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json from typing import TYPE_CHECKING, Optional import httpx @@ -7,6 +8,12 @@ from fastapi import Request from fastapi.responses import Response, StreamingResponse from pydantic.v1 import BaseModel +from ..core.error_scope import ( + ERROR_SCOPE_HEADER, + ERROR_SCOPE_UPSTREAM, + UPSTREAM_ERROR_STATUS, + UPSTREAM_UNAVAILABLE, +) from ..core.exceptions import UpstreamError from ..core.logging import get_logger from ..payment.models import Architecture, Model, Pricing @@ -138,6 +145,30 @@ class TinfoilUpstreamProvider(BaseUpstreamProvider): response_headers = dict(resp.headers) response_headers.pop("content-encoding", None) response_headers.pop("content-length", None) + if resp.status_code >= 500: + logger.warning( + "Tinfoil attestation upstream returned %s", + resp.status_code, + extra={"status_code": resp.status_code}, + ) + return Response( + content=json.dumps( + { + "error": { + "type": "upstream_error", + "code": UPSTREAM_UNAVAILABLE, + "message": ( + "Attestation upstream returned " + f"{resp.status_code}" + ), + "upstream_status": resp.status_code, + } + } + ), + status_code=UPSTREAM_ERROR_STATUS, + media_type="application/json", + headers={ERROR_SCOPE_HEADER: ERROR_SCOPE_UPSTREAM}, + ) return Response( content=resp.content, status_code=resp.status_code, diff --git a/tests/unit/test_ehbp_timeout.py b/tests/unit/test_ehbp_timeout.py index 7ae78cc4..97a7b7ee 100644 --- a/tests/unit/test_ehbp_timeout.py +++ b/tests/unit/test_ehbp_timeout.py @@ -1,14 +1,20 @@ from __future__ import annotations +import json from unittest.mock import AsyncMock, MagicMock import pytest +from routstr.core.error_scope import ( + ERROR_SCOPE_HEADER, + ERROR_SCOPE_UPSTREAM, + UPSTREAM_ERROR_STATUS, +) from routstr.core.exceptions import EhbpTimeoutError, UpstreamError from routstr.upstream import ehbp as ehbp_module # --------------------------------------------------------------------------- -# forward_ehbp_x_cashu_request — timeout fails closed with a refund + 504 +# forward_ehbp_x_cashu_request — timeout fails closed with a refund + 424 # --------------------------------------------------------------------------- @@ -48,7 +54,7 @@ def _ehbp_upstream_mocks() -> tuple[MagicMock, MagicMock]: @pytest.mark.asyncio -async def test_x_cashu_timeout_refunds_and_returns_504( +async def test_x_cashu_timeout_refunds_and_returns_424( monkeypatch: pytest.MonkeyPatch, ) -> None: monkeypatch.setattr( @@ -78,27 +84,32 @@ async def test_x_cashu_timeout_refunds_and_returns_504( upstream=upstream, ) - assert response.status_code == 504 + assert response.status_code == UPSTREAM_ERROR_STATUS + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM assert response.headers["X-Cashu"] == "refund-token" + body = json.loads(bytes(response.body)) + assert body["error"]["type"] == "upstream_timeout" + assert body["error"]["code"] == "UPSTREAM_TIMEOUT" 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 +# proxy.py can answer 424 instead of flattening it to a generic 500 # --------------------------------------------------------------------------- @pytest.mark.asyncio -async def test_bearer_timeout_propagates_504( +async def test_bearer_timeout_propagates_424( 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. + UpstreamError: raise`` above it is the only thing preserving the upstream + timeout status that ``proxy.py`` returns to the client, so this test pins + that handler. """ monkeypatch.setattr( ehbp_module, @@ -126,6 +137,7 @@ async def test_bearer_timeout_propagates_504( model_obj=model_obj, ) - assert exc_info.value.status_code == 504 + assert exc_info.value.status_code == UPSTREAM_ERROR_STATUS assert exc_info.value.code == "UPSTREAM_TIMEOUT" + assert exc_info.value.scope == ERROR_SCOPE_UPSTREAM assert isinstance(exc_info.value, UpstreamError) diff --git a/tests/unit/test_messages_litellm_dispatch.py b/tests/unit/test_messages_litellm_dispatch.py index 294f5c0c..c9b13bd2 100644 --- a/tests/unit/test_messages_litellm_dispatch.py +++ b/tests/unit/test_messages_litellm_dispatch.py @@ -1597,7 +1597,7 @@ async def test_x_cashu_transport_error_after_redemption_is_not_retryable( handler_name: str, forward_attr: str ) -> None: """A transport failure while forwarding (after the token is spent) maps to - 502 upstream_error, never a retryable cashu_mint_unreachable.""" + 424 + UPSTREAM_UNAVAILABLE, never a retryable cashu_mint_unreachable.""" provider = _make_provider() model = _make_model() request = _make_request() @@ -1623,9 +1623,11 @@ async def test_x_cashu_transport_error_after_redemption_is_not_retryable( model_obj=model, ) - assert response.status_code == 502 + assert response.status_code == 424 + assert response.headers["X-Routstr-Error-Scope"] == "upstream" body = json.loads(bytes(response.body)) assert body["error"]["type"] == "upstream_error" + assert body["error"]["code"] == "UPSTREAM_UNAVAILABLE" assert body["error"]["code"] != "cashu_mint_unreachable" diff --git a/tests/unit/test_model_path_routing.py b/tests/unit/test_model_path_routing.py index 4dfa2ef8..4faec94b 100644 --- a/tests/unit/test_model_path_routing.py +++ b/tests/unit/test_model_path_routing.py @@ -9,6 +9,12 @@ import pytest from routstr import proxy as proxy_module from routstr.auth import ReservationSnapshot from routstr.core.db import ApiKey +from routstr.core.error_scope import ( + ERROR_SCOPE_HEADER, + ERROR_SCOPE_NODE, + ERROR_SCOPE_UPSTREAM, + UPSTREAM_UNAVAILABLE, +) from routstr.upstream.model_paths import decode_model_path, encode_model_path MODEL_ID = "test-model" @@ -391,8 +397,13 @@ def test_model_path_header_is_not_forwarded() -> None: @pytest.mark.asyncio @pytest.mark.parametrize("path", ["v1/chat/completions", "v1/responses"]) -@pytest.mark.parametrize("status_code", [200, 429, 502]) -async def test_cashu_pin_reaches_http_transport(path: str, status_code: int) -> None: +@pytest.mark.parametrize( + "status_code,client_status", + [(200, 200), (429, 429), (502, 424)], +) +async def test_cashu_pin_reaches_http_transport( + path: str, status_code: int, client_status: int +) -> None: import httpx from fastapi.responses import Response @@ -443,7 +454,11 @@ async def test_cashu_pin_reaches_http_transport(path: str, status_code: int) -> response = await _run_proxy( request, [(model, upstream), (model, fallback)], path ) - assert response.status_code == status_code + assert response.status_code == client_status + if client_status == 424: + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + body = json.loads(bytes(response.body)) + assert body["error"]["code"] == UPSTREAM_UNAVAILABLE redeem.assert_awaited_once() assert len(sent) == 1 assert sent[0].url.host == "openrouter.ai" @@ -486,7 +501,10 @@ async def test_pinned_exception_does_not_fall_back() -> None: response = await _run_proxy( request, [(MagicMock(), first), (MagicMock(), fallback)] ) - assert response.status_code == 503 + # Pinned: no fallback. + assert response.status_code == 424 + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + assert json.loads(bytes(response.body))["error"]["code"] == UPSTREAM_UNAVAILABLE first.forward_request.assert_awaited_once() fallback.forward_request.assert_not_awaited() @@ -545,7 +563,8 @@ async def test_ehbp_pin_does_not_fall_back(cashu: bool) -> None: response = await _run_proxy( request, [(MagicMock(), selected), (MagicMock(), fallback)] ) - assert response.status_code == 503 + assert response.status_code == 424 + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM forward.assert_awaited_once() assert forward.await_args is not None assert forward.await_args.kwargs["upstream"] is selected @@ -684,3 +703,75 @@ async def test_pinned_recovery_preserves_routing_fields( assert response.status_code == 400 selected.forward_request.assert_awaited_once() fallback.forward_request.assert_not_awaited() + + +# --------------------------------------------------------------------------- # +# Upstream 5xx -> 424 + UPSTREAM_UNAVAILABLE + scope header; node faults stay 500. +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_upstream_424_fails_over_to_a_healthy_provider() -> None: + """An upstream-attributed 424 is still retryable: the caller only ever + sees the healthy provider's 200.""" + from routstr.core.exceptions import UpstreamError + + first, healthy = _make_upstream(1), _make_upstream(2) + first.forward_request.side_effect = UpstreamError("bad gateway", status_code=502) + request = _make_request( + {"authorization": "Bearer key"}, json.dumps({"model": MODEL_ID}).encode() + ) + + response = await _run_proxy(request, [(MagicMock(), first), (MagicMock(), healthy)]) + + assert response.status_code == 200 + first.forward_request.assert_awaited_once() + healthy.forward_request.assert_awaited_once() + # The caller never sees the upstream error body or any scope header. + assert ERROR_SCOPE_HEADER not in response.headers + + +@pytest.mark.asyncio +async def test_last_candidate_upstream_failure_reports_424() -> None: + """Every candidate failed on the provider hop: 424 + upstream scope, with + the provider's own status preserved for operators.""" + from routstr.core.exceptions import UpstreamError + + only = _make_upstream(1) + only.forward_request.side_effect = UpstreamError("bad gateway", status_code=502) + request = _make_request( + {"authorization": "Bearer key"}, json.dumps({"model": MODEL_ID}).encode() + ) + + response = await _run_proxy(request, [(MagicMock(), only)]) + + assert response.status_code == 424 + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + body = json.loads(bytes(response.body)) + assert body["error"]["type"] == "upstream_error" + assert body["error"]["code"] == UPSTREAM_UNAVAILABLE + assert body["error"]["details"]["upstream_status"] == 502 + + +@pytest.mark.asyncio +async def test_node_fault_stays_500_without_scope_header() -> None: + """A genuine node fault keeps its 500 and carries no scope header, so a + client can still tell this node is the broken one.""" + from routstr.core.exceptions import UpstreamError + + only = _make_upstream(1) + only.forward_request.side_effect = UpstreamError( + "An unexpected server error occurred", + status_code=500, + scope=ERROR_SCOPE_NODE, + ) + request = _make_request( + {"authorization": "Bearer key"}, json.dumps({"model": MODEL_ID}).encode() + ) + + response = await _run_proxy(request, [(MagicMock(), only)]) + + assert response.status_code == 500 + assert ERROR_SCOPE_HEADER not in response.headers + body = json.loads(bytes(response.body)) + assert body["error"]["code"] != UPSTREAM_UNAVAILABLE diff --git a/tests/unit/test_proxy_tinfoil_attestation_routing.py b/tests/unit/test_proxy_tinfoil_attestation_routing.py index c367a049..7480d3ac 100644 --- a/tests/unit/test_proxy_tinfoil_attestation_routing.py +++ b/tests/unit/test_proxy_tinfoil_attestation_routing.py @@ -1,13 +1,21 @@ from __future__ import annotations +import json from unittest.mock import AsyncMock, MagicMock +import httpx import pytest from fastapi import FastAPI from fastapi.responses import Response from httpx import ASGITransport, AsyncClient from routstr import proxy as proxy_module +from routstr.core.error_scope import ( + ERROR_SCOPE_HEADER, + ERROR_SCOPE_UPSTREAM, + UPSTREAM_ERROR_STATUS, + UPSTREAM_UNAVAILABLE, +) @pytest.fixture @@ -150,3 +158,141 @@ def test_attestation_upstream_selection_is_tinfoil_only() -> None: assert proxy_module._select_unauthenticated_get_upstreams( "attestationjunk", [non_tinfoil, tinfoil] ) == [non_tinfoil, tinfoil] + + +# --------------------------------------------------------------------------- # +# Unauthenticated GET: upstream 5xx -> 424 + scope header, still retryable. +# --------------------------------------------------------------------------- # + + +def _attributed_424() -> Response: + """The response a provider hands back for an upstream-attributed 5xx.""" + import json as _json + + return Response( + content=_json.dumps( + { + "error": { + "type": "upstream_error", + "code": UPSTREAM_UNAVAILABLE, + "message": "Attestation upstream returned 503", + "upstream_status": 503, + } + } + ).encode(), + status_code=UPSTREAM_ERROR_STATUS, + media_type="application/json", + headers={ERROR_SCOPE_HEADER: ERROR_SCOPE_UPSTREAM}, + ) + + +def _attestation_provider(forward: AsyncMock) -> MagicMock: + provider = MagicMock() + provider.provider_type = "tinfoil" + provider.prepare_headers = MagicMock(return_value={}) + provider.forward_get_request = forward + return provider + + +@pytest.mark.asyncio +async def test_unauthenticated_get_returns_attributed_424_when_all_fail( + monkeypatch: pytest.MonkeyPatch, proxy_app: FastAPI +) -> None: + tinfoil = _attestation_provider(AsyncMock(return_value=_attributed_424())) + monkeypatch.setattr(proxy_module, "_upstreams", [tinfoil]) + + async with AsyncClient( + transport=ASGITransport(app=proxy_app), # type: ignore[arg-type] + base_url="http://test", + ) as client: + response = await client.get("/attestation") + + assert response.status_code == UPSTREAM_ERROR_STATUS + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + payload = json.loads(response.content) + assert payload["error"]["code"] == UPSTREAM_UNAVAILABLE + assert payload["error"]["upstream_status"] == 503 + + +@pytest.mark.asyncio +async def test_unauthenticated_get_fails_over_past_an_attributed_424( + monkeypatch: pytest.MonkeyPatch, proxy_app: FastAPI +) -> None: + """An upstream-attributed 424 stays retryable: the caller sees the healthy + provider's response and never the upstream error.""" + failing = _attestation_provider(AsyncMock(return_value=_attributed_424())) + healthy = _attestation_provider( + AsyncMock(return_value=Response(status_code=200, content=b'{"ok":true}')) + ) + monkeypatch.setattr(proxy_module, "_upstreams", [failing, healthy]) + + async with AsyncClient( + transport=ASGITransport(app=proxy_app), # type: ignore[arg-type] + base_url="http://test", + ) as client: + response = await client.get("/attestation") + + assert response.status_code == 200 + assert response.content == b'{"ok":true}' + failing.forward_get_request.assert_awaited_once() + healthy.forward_get_request.assert_awaited_once() + assert ERROR_SCOPE_HEADER not in response.headers + + +@pytest.mark.asyncio +async def test_attestation_host_5xx_is_attributed_to_the_upstream( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The Tinfoil attestation hop itself maps its 5xx to 424 + upstream scope.""" + from routstr.upstream.tinfoil import TinfoilUpstreamProvider + + class _FakeClient: + async def __aenter__(self) -> "_FakeClient": + return self + + async def __aexit__(self, *_exc: object) -> bool: + return False + + async def get(self, _url: str, headers: dict | None = None) -> httpx.Response: + return httpx.Response(status_code=503, content=b"atc down") + + monkeypatch.setattr( + "routstr.upstream.tinfoil.httpx.AsyncClient", lambda **_kw: _FakeClient() + ) + provider = TinfoilUpstreamProvider(api_key="k") + + response = await provider._proxy_attestation({}) + + assert response.status_code == UPSTREAM_ERROR_STATUS + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + payload = json.loads(bytes(response.body)) + assert payload["error"]["type"] == "upstream_error" + assert payload["error"]["code"] == UPSTREAM_UNAVAILABLE + assert payload["error"]["upstream_status"] == 503 + + +@pytest.mark.asyncio +async def test_attestation_host_4xx_passes_through( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from routstr.upstream.tinfoil import TinfoilUpstreamProvider + + class _FakeClient: + async def __aenter__(self) -> "_FakeClient": + return self + + async def __aexit__(self, *_exc: object) -> bool: + return False + + async def get(self, _url: str, headers: dict | None = None) -> httpx.Response: + return httpx.Response(status_code=404, content=b"missing") + + monkeypatch.setattr( + "routstr.upstream.tinfoil.httpx.AsyncClient", lambda **_kw: _FakeClient() + ) + provider = TinfoilUpstreamProvider(api_key="k") + + response = await provider._proxy_attestation({}) + + assert response.status_code == 404 + assert bytes(response.body) == b"missing" diff --git a/tests/unit/test_tinfoil_trailer.py b/tests/unit/test_tinfoil_trailer.py index 3e4d3e0f..2b3d22f4 100644 --- a/tests/unit/test_tinfoil_trailer.py +++ b/tests/unit/test_tinfoil_trailer.py @@ -5,6 +5,7 @@ from unittest.mock import AsyncMock, MagicMock import pytest +from routstr.core.error_scope import ERROR_SCOPE_UPSTREAM, UPSTREAM_ERROR_STATUS from routstr.core.exceptions import EhbpTimeoutError, UpstreamError from routstr.upstream.tinfoil_trailer import forward_with_trailer @@ -193,9 +194,10 @@ async def test_forward_with_trailer_read_timeout_raises_ehbp_timeout( def test_ehbp_timeout_error_metadata() -> None: exc = EhbpTimeoutError("boom") - assert exc.status_code == 504 + assert exc.status_code == UPSTREAM_ERROR_STATUS assert exc.code == "UPSTREAM_TIMEOUT" assert exc.details is None + assert exc.scope == ERROR_SCOPE_UPSTREAM assert isinstance(exc, UpstreamError) @@ -203,5 +205,5 @@ 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.status_code == UPSTREAM_ERROR_STATUS assert exc.code == "UPSTREAM_TIMEOUT" diff --git a/tests/unit/test_upstream_error_response.py b/tests/unit/test_upstream_error_response.py index 62b9a32c..c053fc90 100644 --- a/tests/unit/test_upstream_error_response.py +++ b/tests/unit/test_upstream_error_response.py @@ -14,7 +14,17 @@ from unittest.mock import Mock import httpx import pytest +from routstr.core.error_scope import ( + ERROR_SCOPE_HEADER, + ERROR_SCOPE_NODE, + ERROR_SCOPE_UPSTREAM, + UPSTREAM_ERROR_STATUS, + UPSTREAM_UNAVAILABLE, +) +from routstr.core.exceptions import UpstreamError +from routstr.payment.helpers import create_upstream_error_response from routstr.upstream.base import BaseUpstreamProvider, _is_json_content_type +from routstr.upstream.rate_limit import UPSTREAM_RATE_LIMIT def _make_request(request_id: str = "req-123") -> Mock: @@ -105,10 +115,13 @@ async def test_plain_text_error_is_normalized( _make_request(), "v1/messages", upstream ) - assert response.status_code == 503 + assert response.status_code == UPSTREAM_ERROR_STATUS + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM assert response.media_type == "application/json" payload = json.loads(bytes(response.body)) assert payload["error"]["message"] == "Service Unavailable" + assert payload["error"]["code"] == UPSTREAM_UNAVAILABLE + assert payload["error"]["upstream_status"] == 503 @pytest.mark.asyncio @@ -123,10 +136,13 @@ async def test_empty_body_with_non_json_content_type_normalizes( _make_request(), "v1/messages", upstream ) - assert response.status_code == 502 + assert response.status_code == UPSTREAM_ERROR_STATUS + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM assert response.media_type == "application/json" payload = json.loads(bytes(response.body)) assert payload["error"]["type"] == "upstream_error" + assert payload["error"]["code"] == UPSTREAM_UNAVAILABLE + assert payload["error"]["upstream_status"] == 502 assert payload["error"]["upstream_body_preview"] is None @@ -148,3 +164,148 @@ async def test_json_error_body_is_passed_through_unchanged( assert response.status_code == 400 assert bytes(response.body) == json_body assert response.media_type == "application/json" + + +# --------------------------------------------------------------------------- # +# Upstream 5xx -> 424 + UPSTREAM_UNAVAILABLE + scope header; node faults stay +# 500 without it; rate limits keep 429. +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +@pytest.mark.parametrize("path", ["v1/chat/completions", "v1/messages", "v1/responses"]) +@pytest.mark.parametrize("upstream_status", [500, 502, 503, 504]) +async def test_upstream_5xx_is_attributed_to_the_upstream( + provider: BaseUpstreamProvider, path: str, upstream_status: int +) -> None: + body = json.dumps( + {"error": {"message": "provider exploded", "type": "server_error"}} + ).encode() + upstream = _make_upstream_response( + body=body, status_code=upstream_status, content_type="application/json" + ) + + response = await provider.forward_upstream_error_response( + _make_request(), path, upstream + ) + + assert response.status_code == UPSTREAM_ERROR_STATUS + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + payload: dict[str, Any] = json.loads(bytes(response.body)) + assert payload["error"]["code"] == UPSTREAM_UNAVAILABLE + assert payload["error"]["upstream_status"] == upstream_status + + +@pytest.mark.asyncio +async def test_upstream_5xx_non_json_body_keeps_scope_and_status( + provider: BaseUpstreamProvider, +) -> None: + """The envelope for a non-JSON 5xx carries the same attribution.""" + upstream = _make_upstream_response( + body=b"bad gateway", status_code=502, content_type="text/html" + ) + + response = await provider.forward_upstream_error_response( + _make_request(), "v1/chat/completions", upstream + ) + + assert response.status_code == UPSTREAM_ERROR_STATUS + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + payload: dict[str, Any] = json.loads(bytes(response.body)) + assert payload["error"]["type"] == "upstream_error" + assert payload["error"]["code"] == UPSTREAM_UNAVAILABLE + assert payload["error"]["upstream_status"] == 502 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("upstream_status", [400, 401, 403, 404, 422]) +async def test_provider_4xx_passes_through_unchanged( + provider: BaseUpstreamProvider, upstream_status: int +) -> None: + """A provider 4xx is its verdict on the request, not a node-health signal.""" + body = json.dumps( + {"error": {"message": "bad request", "type": "invalid_request_error"}} + ).encode() + upstream = _make_upstream_response( + body=body, status_code=upstream_status, content_type="application/json" + ) + + response = await provider.forward_upstream_error_response( + _make_request(), "v1/chat/completions", upstream + ) + + assert response.status_code == upstream_status + + +@pytest.mark.asyncio +async def test_upstream_rate_limit_keeps_429( + provider: BaseUpstreamProvider, +) -> None: + """429 + UPSTREAM_RATE_LIMIT is unchanged by the 424 mapping: the retry + hint is worth more than the status class.""" + body = json.dumps( + {"error": {"message": "Rate limit reached, please try again"}} + ).encode() + upstream = _make_upstream_response(body=body, status_code=429) + + response = await provider.forward_upstream_error_response( + _make_request(), "v1/chat/completions", upstream + ) + + assert response.status_code == 429 + payload: dict[str, Any] = json.loads(bytes(response.body)) + assert payload["error"]["code"] == UPSTREAM_RATE_LIMIT + + +def test_generic_upstream_error_response_reports_424() -> None: + """``create_upstream_error_response`` maps a plain upstream failure to 424.""" + err = UpstreamError("connection refused", status_code=502) + + response = create_upstream_error_response(err, _make_request()) + + assert response.status_code == UPSTREAM_ERROR_STATUS + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + payload: dict[str, Any] = json.loads(bytes(response.body)) + assert payload["error"]["type"] == "upstream_error" + assert payload["error"]["code"] == UPSTREAM_UNAVAILABLE + assert payload["error"]["details"]["upstream_status"] == 502 + + +def test_rate_limit_error_response_keeps_429_and_code() -> None: + err = UpstreamError( + "slow down", status_code=429, code=UPSTREAM_RATE_LIMIT, details={"a": 1} + ) + + response = create_upstream_error_response(err, _make_request()) + + assert response.status_code == 429 + payload: dict[str, Any] = json.loads(bytes(response.body)) + assert payload["error"]["code"] == UPSTREAM_RATE_LIMIT + assert payload["error"]["details"] == {"a": 1} + + +def test_5xx_wrapped_rate_limit_error_response_keeps_429() -> None: + """A rate limit wrapped in a provider 5xx still answers 429.""" + err = UpstreamError("slow down", status_code=500, code=UPSTREAM_RATE_LIMIT) + + response = create_upstream_error_response(err, _make_request()) + + assert response.status_code == 429 + payload: dict[str, Any] = json.loads(bytes(response.body)) + assert payload["error"]["code"] == UPSTREAM_RATE_LIMIT + + +def test_node_scoped_failure_stays_500_without_scope_header() -> None: + """A genuine node fault must never be disguised as an upstream one.""" + err = UpstreamError("mint unreachable", status_code=500, scope=ERROR_SCOPE_NODE) + + response = create_upstream_error_response(err, _make_request()) + + assert response.status_code == 500 + assert ERROR_SCOPE_HEADER not in response.headers + payload: dict[str, Any] = json.loads(bytes(response.body)) + assert payload["error"]["code"] != UPSTREAM_UNAVAILABLE + + +def test_upstream_error_defaults_to_upstream_scope() -> None: + assert UpstreamError("boom").scope == ERROR_SCOPE_UPSTREAM diff --git a/tests/unit/test_upstream_rate_limit.py b/tests/unit/test_upstream_rate_limit.py index 495f1e57..95b29d83 100644 --- a/tests/unit/test_upstream_rate_limit.py +++ b/tests/unit/test_upstream_rate_limit.py @@ -15,6 +15,11 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch import httpx import pytest +from routstr.core.error_scope import ( + ERROR_SCOPE_HEADER, + ERROR_SCOPE_UPSTREAM, + UPSTREAM_UNAVAILABLE, +) from routstr.core.redaction import redact_org_ids from routstr.upstream.base import BaseUpstreamProvider from routstr.upstream.rate_limit import ( @@ -231,7 +236,8 @@ def test_create_upstream_error_response_preserves_structure() -> None: assert "org-[REDACTED]" in serialized -def test_generic_upstream_error_still_defaults_to_502() -> None: +def test_generic_upstream_error_reports_424() -> None: + """An upstream-attributable failure is reported as 424, not 502.""" from routstr.core.exceptions import UpstreamError from routstr.payment.helpers import create_upstream_error_response @@ -239,11 +245,12 @@ def test_generic_upstream_error_still_defaults_to_502() -> None: response = create_upstream_error_response(err, _make_request()) - assert response.status_code == 502 + assert response.status_code == 424 + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM payload: dict[str, Any] = json.loads(bytes(response.body)) assert payload["error"]["type"] == "upstream_error" - assert payload["error"]["code"] == 502 - assert "details" not in payload["error"] + assert payload["error"]["code"] == UPSTREAM_UNAVAILABLE + assert payload["error"]["details"]["upstream_status"] == 502 # --------------------------------------------------------------------------- # @@ -307,7 +314,8 @@ async def test_5xx_wrapped_rate_limit_is_classified( provider: BaseUpstreamProvider, ) -> None: # Some providers wrap a rate-limit in a 5xx envelope; classification must - # key off the message marker, not only the 429 status. + # key off the message marker, not only the 429 status. The retry hint wins + # over the 424 mapping: a caller must still see a retryable 429. body = json.dumps({"error": {"message": RATE_LIMIT_MESSAGE}}).encode() upstream = _make_upstream_response(body=body, status_code=500) @@ -315,9 +323,11 @@ async def test_5xx_wrapped_rate_limit_is_classified( _make_request(), "v1/chat/completions", upstream ) - assert response.status_code == 500 + assert response.status_code == 429 payload: dict[str, Any] = json.loads(bytes(response.body)) assert payload["error"]["code"] == UPSTREAM_RATE_LIMIT + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + assert payload["error"]["upstream_status"] == 500 serialized = json.dumps(payload) assert RAW_ORG_ID not in serialized assert "org-[REDACTED]" in serialized