diff --git a/docs/api/endpoints.md b/docs/api/endpoints.md index 5ead333e..1cbf8590 100644 --- a/docs/api/endpoints.md +++ b/docs/api/endpoints.md @@ -264,7 +264,12 @@ 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 and `error.code = UPSTREAM_RATE_LIMIT`; the `529` + (a provider 5xx) is reported as `424` + `error.code = UPSTREAM_UNAVAILABLE` + with `X-Routstr-Error-Scope: upstream` and the provider's own status in + `error.upstream_status` — this node is healthy, so a 5xx here must not be + read as node health. **Enabling the provider:** diff --git a/docs/api/errors.md b/docs/api/errors.md index 82bb5be1..fb2c40d7 100644 --- a/docs/api/errors.md +++ b/docs/api/errors.md @@ -51,11 +51,59 @@ 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) + +Routstr fronts third-party inference providers. When one of them fails, *this +node* is still healthy: it accepted the request, authenticated it, reserved +payment, and reverted the reservation once the last candidate failed. Such +failures are reported deliberately as a **non-5xx** status so a client does not +mark the node down, drop it from rotation, or refuse to retry an upstream blip. + +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` — the + retry hint is worth more than the status class. This includes a rate limit a + provider wrapped in a 5xx envelope: the status becomes `429` and the code + stays `UPSTREAM_RATE_LIMIT`. +- **Provider-side 4xx** (`400`/`401`/`403`/`404`/`422`) passes through unchanged: + that is the provider's verdict on the *request*, not a health signal. + +Genuine node faults are deliberately untouched: an unreachable mint, a database +failure, or an internal exception while talking to a provider still answers +`500` and carries **no** `X-Routstr-Error-Scope` header. That is what makes +"node healthy, upstream failed" distinguishable from "node broken" from the +response alone. + ## Error Types ### Authentication Errors @@ -329,14 +377,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 +397,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 +419,9 @@ Retry-After: 45 } ``` -**Status:** 504 +**Status:** 424 (deliberately non-5xx: the timeout happened on the provider +hop, not on this node) +**Header:** `X-Routstr-Error-Scope: upstream` **Resolution:** Retry with shorter prompt or max_tokens ### Content Policy @@ -416,7 +473,9 @@ def retry_with_backoff( # Check if error is retryable if hasattr(e, 'status_code'): - if e.status_code in [429, 502, 503, 504]: + # 424 is the upstream-attribution status: it is retryable + # exactly like the 5xx statuses it replaced. + 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 +500,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..9b09bf9a --- /dev/null +++ b/routstr/core/error_scope.py @@ -0,0 +1,93 @@ +"""Attribution scope for upstream-caused failures. + +routstr-core fronts third-party inference providers. When one of them fails, +this node is still healthy: it accepted the request, authenticated it, reserved +payment, and reverted the reservation once the last candidate failed. Forwarding +the provider's 5xx verbatim makes a caller conclude *this node* is down, so it +marks the node down, drops it from rotation, or refuses to retry an upstream +blip. + +Upstream-attributed failures therefore answer a deliberate non-5xx status — +``424 Failed Dependency`` — carrying the stable ``UPSTREAM_UNAVAILABLE`` +``error.code``, the ``X-Routstr-Error-Scope: upstream`` response header, and the +provider's own status preserved in ``error.details.upstream_status``. Rate +limits keep ``429`` with their existing ``UPSTREAM_RATE_LIMIT`` code: the retry +hint is worth more there than the status class, and clients already branch on +that pair. + +Genuine node faults are deliberately untouched: an unreachable mint, a DB +failure, or an unhandled exception still answers ``500`` and carries no scope +header, so "node healthy, upstream failed" and "node broken" stay +distinguishable from the response alone. +""" + +from __future__ import annotations + +#: Stable, machine-readable classification for an upstream-attributable +#: failure (``error.code``). Mirrors the ``UPSTREAM_RATE_LIMIT`` precedent: a +#: non-numeric code clients can branch on without parsing provider messages. +UPSTREAM_UNAVAILABLE = "UPSTREAM_UNAVAILABLE" + +#: HTTP status returned for upstream-attributable failures. 424 is deliberately +#: not 5xx so callers stop reading an upstream blip as this node's health. +UPSTREAM_ERROR_STATUS = 424 + +#: Header naming the failure's attribution scope. Present (value ``upstream``) +#: on every upstream-caused error path; absent on node faults. +ERROR_SCOPE_HEADER = "X-Routstr-Error-Scope" +ERROR_SCOPE_UPSTREAM = "upstream" +ERROR_SCOPE_NODE = "node" + + +def _is_rate_limit_code(code: object) -> bool: + """Return whether ``code`` is the upstream rate-limit classification.""" + # Imported lazily: ``routstr.upstream`` imports the proxy/payment stack, and + # this module is imported from both sides of it. + 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: + """Return the HTTP status a caller sees for an upstream-attributable failure. + + ``status_code`` is the provider's own status (or the status the failure was + classified with). Rate limits keep 429; every other 5xx collapses to + :data:`UPSTREAM_ERROR_STATUS`; 4xx statuses pass through unchanged because + they are the provider's verdict on the *request*, not a node-health signal. + """ + 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`` a caller sees for an upstream-attributable failure.""" + 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: + """Return ``details`` with the provider's status preserved when it differs. + + Only recorded when the caller-visible status no longer equals the status the + upstream hop produced — otherwise the information is already on the status + line and would just be noise. + """ + 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 b8dcdc38..4c42f125 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__) @@ -18,6 +19,18 @@ class UpstreamError(Exception): string-matching the message. ``details`` holds optional structured, redaction-safe context. Both default to ``None`` for backwards compatibility. + + ``scope`` records who the failure belongs to. ``"upstream"`` (the default, + since this class describes a provider hop that failed) means the node + stayed healthy and the failure is reported to the caller as a non-5xx + ``424`` with ``error.code = UPSTREAM_UNAVAILABLE`` (see + :mod:`routstr.core.error_scope`). ``"node"`` means a fault inside this + process that merely surfaced while talking to a provider; those keep their + status so a node failure is never disguised as an upstream one. + + ``status_code`` always stays the provider's own status (or the status the + failure was classified with) — the caller-visible mapping happens once, at + response construction. """ def __init__( @@ -26,11 +39,13 @@ class UpstreamError(Exception): status_code: int = 502, code: str | None = None, details: dict[str, object] | None = None, + scope: str = ERROR_SCOPE_UPSTREAM, ): self.message = message self.status_code = status_code self.code = code self.details = details + self.scope = scope super().__init__(message) @@ -38,8 +53,10 @@ 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. The status is the upstream-failure status + (424, deliberately non-5xx): the timeout happened on the provider hop, and + reporting it as 504 told callers this node was the broken one. ``details`` carries optional structured, redaction-safe context and is forwarded to the client by ``create_upstream_error_response``. @@ -48,7 +65,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..2fba52b4 100644 --- a/routstr/core/main.py +++ b/routstr/core/main.py @@ -289,6 +289,10 @@ app.add_middleware( expose_headers=[ "x-routstr-request-id", "x-cashu", + # Attribution scope for upstream-caused failures. Browser clients must + # be able to read it, otherwise they cannot tell an upstream blip + # (424, X-Routstr-Error-Scope: upstream) from a node fault (500). + "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..85cb8750 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,16 @@ 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`` marks the failure as attributable + to a specific hop (``upstream``) and is emitted as the + :data:`ERROR_SCOPE_HEADER` response header. """ error_obj: dict[str, object] = { "message": redact_org_ids(message), @@ -669,6 +680,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 +694,7 @@ def create_error_response( ), status_code=status_code, media_type="application/json", - headers={"X-Cashu": token} if token else {}, + headers=headers, ) @@ -687,13 +703,37 @@ 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 failures (the default) are reported as + :data:`UPSTREAM_ERROR_STATUS` with ``error.code = UPSTREAM_UNAVAILABLE`` and + the :data:`ERROR_SCOPE_HEADER` header, so a caller can tell "this node is + healthy, one provider hop failed" from "this node is broken" — the latter + keeps answering 500 and carries no scope header. The provider's own status + stays discoverable in ``error.details.upstream_status`` and in the message. + + Node-scoped failures and rate limits keep their own status: a node fault + must never be disguised as an upstream one, and 429 carries a retry hint + clients already act on. + """ + 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 2ada5d3a..5a9c1d0a 100644 --- a/routstr/proxy.py +++ b/routstr/proxy.py @@ -476,7 +476,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( @@ -702,7 +702,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"): @@ -888,8 +888,10 @@ 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] + # Retry on another candidate when the failure is retryable: 424 + # (upstream-attributed failure), 502 (upstream error), 429 (rate + # limit), or a provider-side 4xx. + 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 488d07b1..7eb802a9 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,14 @@ class BaseUpstreamProvider: error_code = UPSTREAM_RATE_LIMIT error_details = rate_limit.as_details() + # An upstream 5xx is this provider's failure, not this node's: report it + # as 424 + UPSTREAM_UNAVAILABLE so a caller never reads it as node + # health, and mark the attribution with a header for clients that only + # look at status/headers. 4xx passes through unchanged. + 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 +1028,29 @@ 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: ``UPSTREAM_RATE_LIMIT`` for rate limits, and + # ``UPSTREAM_UNAVAILABLE`` (plus the provider's own status) when an + # upstream 5xx was re-reported as 424. 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 +1063,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 +1077,7 @@ class BaseUpstreamProvider: return Response( content=json.dumps(envelope).encode(), - status_code=status_code, + status_code=client_status, headers=headers, media_type="application/json", ) @@ -3384,7 +3406,14 @@ 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) + # Node-scoped: an internal fault surfaced while talking to the + # provider, so it must keep answering 500 instead of being + # reported as an upstream failure. + raise UpstreamError( + "An unexpected server error occurred", + status_code=500, + scope=ERROR_SCOPE_NODE, + ) supports_ehbp: bool = False @@ -3662,7 +3691,14 @@ 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) + # Node-scoped: an internal fault surfaced while talking to the + # provider, so it must keep answering 500 instead of being + # reported as an upstream failure. + raise UpstreamError( + "An unexpected server error occurred", + status_code=500, + scope=ERROR_SCOPE_NODE, + ) async def forward_get_request( self, @@ -4494,15 +4530,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): @@ -4652,12 +4694,21 @@ 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-scoped: the token was spent on a request this node + # accepted, so the failure belongs to the provider hop, not to + # node health. Report it as 424 + UPSTREAM_UNAVAILABLE while + # preserving whatever status the failure carried. + 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) @@ -4791,15 +4842,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"): @@ -5403,12 +5460,21 @@ class BaseUpstreamProvider: # must not surface as a retryable mint_unreachable (spent-token retry # bait). Redemption classification only applies while not redeemed. if redeemed: + # Upstream-scoped: the token was spent on a request this node + # accepted, so the failure belongs to the provider hop, not to + # node health. Report it as 424 + UPSTREAM_UNAVAILABLE while + # preserving whatever status the failure carried. + 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 90b92719..f5d23e51 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 ( @@ -1031,7 +1039,13 @@ async def forward_ehbp_request( "traceback": tb, }, ) - raise UpstreamError("An unexpected server error occurred", status_code=500) + # Node-scoped: an internal fault surfaced while talking to the enclave, + # so it must keep answering 500 rather than be reported as upstream. + raise UpstreamError( + "An unexpected server error occurred", + status_code=500, + scope=ERROR_SCOPE_NODE, + ) async def forward_ehbp_x_cashu_request( @@ -1127,15 +1141,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 @@ -1241,9 +1259,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 @@ -1259,9 +1278,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: @@ -1283,8 +1303,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 @@ -1351,7 +1372,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/tinfoil.py b/routstr/upstream/tinfoil.py index 0928cbc9..ce620e9b 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,34 @@ 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: + # The attestation host is an upstream hop too: a 5xx there + # says the provider is broken, not this node. Report it with + # the upstream-failure status so a caller does not mark the + # node down, and keep the host's status discoverable. + 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..17c9e9e3 100644 --- a/tests/unit/test_ehbp_timeout.py +++ b/tests/unit/test_ehbp_timeout.py @@ -1,14 +1,24 @@ 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 +# +# The EHBP hop is an upstream: a timeout there must not be reported as a 5xx +# (which would read as *this node* being down). See +# CORE-UPSTREAM-5XX-NOT-NODE-DOWN. # --------------------------------------------------------------------------- @@ -48,7 +58,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 +88,34 @@ async def test_x_cashu_timeout_refunds_and_returns_504( upstream=upstream, ) - assert response.status_code == 504 + # 424, not 504: the timeout belongs to the upstream hop, and a 5xx would + # tell the caller this node is broken. + 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 +143,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..bb6e4b03 100644 --- a/tests/unit/test_messages_litellm_dispatch.py +++ b/tests/unit/test_messages_litellm_dispatch.py @@ -1597,7 +1597,8 @@ 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 (upstream-attributed, CORE-UPSTREAM-5XX-NOT-NODE-DOWN), + never a retryable cashu_mint_unreachable.""" provider = _make_provider() model = _make_model() request = _make_request() @@ -1623,9 +1624,14 @@ async def test_x_cashu_transport_error_after_redemption_is_not_retryable( model_obj=model, ) - assert response.status_code == 502 + # The token is spent, so the failure belongs to the provider hop: the + # caller gets a non-5xx 424 with the attribution header instead of a 502 + # that reads as this node being down. + 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..fa9669b4 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,15 @@ 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", + # An upstream 5xx is re-reported as 424 (CORE-UPSTREAM-5XX-NOT-NODE-DOWN); + # a 429 keeps its retry hint; 200 passes through untouched. + [(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 +456,12 @@ 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: + # Upstream-attributed: the caller can see it is not this node. + 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 +504,12 @@ 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. The failure belongs to the pinned upstream, so the + # caller sees 424 + the scope header rather than a 503 that reads as this + # node being down. + 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 +568,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 +708,79 @@ 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() + + +# --------------------------------------------------------------------------- # +# Acceptance: CORE-UPSTREAM-5XX-NOT-NODE-DOWN +# +# Upstream-attributable failures must be reported as 424 + UPSTREAM_UNAVAILABLE +# with the X-Routstr-Error-Scope: upstream header, must stay retryable across +# candidates, and must never make a node fault look like an upstream one. +# --------------------------------------------------------------------------- # + + +@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..887041ca 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,145 @@ def test_attestation_upstream_selection_is_tinfoil_only() -> None: assert proxy_module._select_unauthenticated_get_upstreams( "attestationjunk", [non_tinfoil, tinfoil] ) == [non_tinfoil, tinfoil] + + +# --------------------------------------------------------------------------- # +# Acceptance: CORE-UPSTREAM-5XX-NOT-NODE-DOWN on the unauthenticated GET path +# +# An upstream 5xx on these paths must be reported as 424 + UPSTREAM_UNAVAILABLE +# with the X-Routstr-Error-Scope: upstream header, must stay retryable across +# candidates, and must never be re-labelled as a node fault. +# --------------------------------------------------------------------------- # + + +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..ccdc8b6a 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,16 @@ async def test_plain_text_error_is_normalized( _make_request(), "v1/messages", upstream ) - assert response.status_code == 503 + # An upstream 5xx is reported as 424 so the caller does not read it as this + # node's health (CORE-UPSTREAM-5XX-NOT-NODE-DOWN); the provider's own + # status stays discoverable. + 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 +139,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 +167,154 @@ 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" + + +# --------------------------------------------------------------------------- # +# Acceptance: CORE-UPSTREAM-5XX-NOT-NODE-DOWN +# +# An upstream-attributable failure must answer 424 + error.code +# UPSTREAM_UNAVAILABLE + header X-Routstr-Error-Scope: upstream, and keep the +# provider's own status in error.upstream_status. A genuine node fault must +# keep answering 500 with no scope header. 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 + # The provider's own status survives so operators can still tell what the + # upstream actually did. + 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..548861e7 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,12 @@ 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. + + The 502 was indistinguishable from a node-side gateway failure, so callers + marked this node down for an upstream blip (CORE-UPSTREAM-5XX-NOT-NODE-DOWN). + """ from routstr.core.exceptions import UpstreamError from routstr.payment.helpers import create_upstream_error_response @@ -239,11 +249,13 @@ 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 + # The provider's own status is preserved for operators. + assert payload["error"]["details"]["upstream_status"] == 502 # --------------------------------------------------------------------------- # @@ -307,7 +319,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 +328,13 @@ 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 + # Still upstream-attributable, so the scope header is present… + assert response.headers[ERROR_SCOPE_HEADER] == ERROR_SCOPE_UPSTREAM + # …and the raw 5xx the provider actually sent stays discoverable. + assert payload["error"]["upstream_status"] == 500 serialized = json.dumps(payload) assert RAW_ORG_ID not in serialized assert "org-[REDACTED]" in serialized