From 39ec591c485c0d679b0ebec75a2d844d9ef83895 Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Wed, 21 Oct 2026 11:43:33 -0600 Subject: [PATCH] fix(api)!: send service and DRM errors as 422, not 502 Cloudflare replaces an origin 502 body with its own page, so clients got "502: Bad gateway" in place of the error and resent each failed licence. BREAKING CHANGE: SERVICE_ERROR and DRM_ERROR now return HTTP 422 instead of 502. --- docs/dev/rest-api/endpoints.md | 33 +++++----------------------- docs/dev/rest-api/errors.md | 22 +++++++++++-------- docs/dev/rest-api/quickstart.md | 4 +++- tests/remote/e2e/test_live_search.py | 2 +- tests/remote/unit/test_errors.py | 8 ++++--- unshackle/core/api/errors.py | 4 ++-- unshackle/core/api/handlers.py | 2 +- 7 files changed, 30 insertions(+), 45 deletions(-) diff --git a/docs/dev/rest-api/endpoints.md b/docs/dev/rest-api/endpoints.md index 5428b194..a633bcbf 100644 --- a/docs/dev/rest-api/endpoints.md +++ b/docs/dev/rest-api/endpoints.md @@ -182,7 +182,7 @@ For a service with `anonymous_search`, the server does not log in and ignores `c | `400` | `INVALID_PARAMETERS` | Missing `query`. | | `400` | `INVALID_SERVICE` | Unknown or disallowed service. | | `400` | `INVALID_PROXY` | Proxy could not be resolved. | -| `502` | `SERVICE_ERROR` | Service failed to initialise, or does not support search. | +| `422` | `SERVICE_ERROR` | Service failed to initialise, or does not support search. | ### `POST /api/list-titles` @@ -1141,7 +1141,7 @@ Get the HLS segments a service drops (ads, bumpers, dub cards). The server runs | `200` | - | Unwanted segment URIs, or `null`. | | `400` | `INVALID_INPUT` | Missing `track_id`. | | `404` | `TRACK_NOT_FOUND` | An unknown track ID was requested. | -| `500` | `SERVICE_ERROR` | The playlist request or the filter failed. | +| `422` | `SERVICE_ERROR` | The playlist request or the filter failed. | ### `POST /api/session/{session_id}/samples` @@ -1159,7 +1159,7 @@ The client sends at most 8 MiB of samples per request, and the route accepts a b | `413` | - | The body is larger than 16 MiB. | | `403` | `FORBIDDEN` | The API key has no `server_cdm` grant for the service. | | `404` | `TRACK_NOT_FOUND` | An unknown track ID was requested. | -| `500` | `SERVICE_ERROR` | The service's decrypter failed. | +| `422` | `SERVICE_ERROR` | The service's decrypter failed. | ### `POST /api/session/{session_id}/license` @@ -1211,7 +1211,7 @@ A slow answer arrives as a [slow response](#slow-responses). | `400` | `INVALID_PARAMETERS` | Unsupported `drm_type`. | | `404` | `TRACK_NOT_FOUND` | Unknown track ID. | | `404` | `NO_CONTENT` | Server CDM produced no keys. | -| `502` | `SERVICE_ERROR` | The service aborted during licensing. | +| `422` | `SERVICE_ERROR` | The service aborted during licensing. | ### `POST /api/session/{session_id}/keys/bad` @@ -1288,30 +1288,7 @@ The server serialises every structured error raised inside a handler to the same ### Error codes and default status -| Error code | HTTP status | -| --- | --- | -| `INVALID_INPUT` | 400 | -| `INVALID_SERVICE` | 400 | -| `INVALID_PROXY` | 400 | -| `INVALID_PARAMETERS` | 400 | -| `AUTH_FAILED` | 401 | -| `FORBIDDEN` | 403 | -| `SERVER_CDM_CAPPED` | 403 | -| `GEOFENCE` | 403 | -| `NOT_FOUND` | 404 | -| `NO_CONTENT` | 404 | -| `JOB_NOT_FOUND` | 404 | -| `SESSION_NOT_FOUND` | 404 | -| `TRACK_NOT_FOUND` | 404 | -| `CONFLICT` | 409 | -| `RATE_LIMITED` | 429 | -| `INTERNAL_ERROR` | 500 | -| `DOWNLOAD_ERROR` | 500 | -| `WORKER_ERROR` | 500 | -| `SERVICE_ERROR` | 502 | -| `DRM_ERROR` | 502 | -| `NETWORK_ERROR` | 503 | -| `SERVICE_UNAVAILABLE` | 503 | +The [error code catalogue](errors.md#error-code-catalogue) gives the default HTTP status of each error code. When a generic (uncategorized) exception reaches a handler, unshackle inspects its message and type to pick a code. For example, messages mentioning "connection" or "timeout" become `NETWORK_ERROR` (503, retryable), "rate limit" or "429" become `RATE_LIMITED` (429, retryable), and anything unrecognized collapses to `INTERNAL_ERROR` (500). diff --git a/docs/dev/rest-api/errors.md b/docs/dev/rest-api/errors.md index f7e99353..82ecc8c0 100644 --- a/docs/dev/rest-api/errors.md +++ b/docs/dev/rest-api/errors.md @@ -92,6 +92,10 @@ but the defaults hold for the overwhelming majority of responses. ### Client errors (4xx) +`SERVICE_ERROR` and `DRM_ERROR` are upstream failures, but they use `422`, not `502`. +A proxy such as Cloudflare replaces the body of a `502` or `504` with its own page, and +the JSON error is then lost. + | Code | Status | Meaning | |---|---|---| | `INVALID_INPUT` | 400 | A required field is missing, or the request body could not be parsed. Also used as the generic "your input was malformed" code. | @@ -108,6 +112,8 @@ but the defaults hold for the overwhelming majority of responses. | `SESSION_NOT_FOUND` | 404 | The remote-dl session does not exist or has expired. | | `TRACK_NOT_FOUND` | 404 | A referenced track ID is not present in the remote session. | | `CONFLICT` | 409 | The target resource is in a state that disallows the action (retrying a running job, prioritising a non-queued job, clearing cache during an active download). | +| `SERVICE_ERROR` | 422 | The streaming service's API returned an error, or the service does not support the requested operation. | +| `DRM_ERROR` | 422 | DRM or license acquisition failed. | | `RATE_LIMITED` | 429 | The streaming service is rate-limiting requests. Retryable. | !!! warning "Not every `429` is `RATE_LIMITED`" @@ -126,8 +132,6 @@ but the defaults hold for the overwhelming majority of responses. | `INTERNAL_ERROR` | 500 | An unexpected server-side error. This is the fallback for any exception the server could not categorise. | | `DOWNLOAD_ERROR` | 500 | The download process itself failed. | | `WORKER_ERROR` | 500 | The download worker subprocess errored out. | -| `SERVICE_ERROR` | 502 | The streaming service's API returned an error, or the service does not support the requested operation. | -| `DRM_ERROR` | 502 | DRM or license acquisition failed. Retryable. | | `NETWORK_ERROR` | 503 | A network connectivity problem (connection, timeout, DNS, TLS). Retryable. | | `SERVICE_UNAVAILABLE` | 503 | The service is temporarily unavailable or under maintenance. Retryable. | @@ -156,7 +160,7 @@ The classification is keyword-based, checked in order: | `geofence`, `geoblock`, `region`, `not available in`, `territory` | `GEOFENCE` (403) | no | | `not found`, `404`, `does not exist`, `invalid id` | `NOT_FOUND` (404) | no | | `rate limit`, `too many requests`, `429`, `throttle` | `RATE_LIMITED` (429) | yes | -| `drm`, `license`, `widevine`, `playready`, `decrypt` | `DRM_ERROR` (502) | no | +| `drm`, `license`, `widevine`, `playready`, `decrypt` | `DRM_ERROR` (422) | no | | `service unavailable`, `503`, `maintenance`, `temporarily unavailable` | `SERVICE_UNAVAILABLE` (503) | yes | | `invalid`, `malformed`, `validation` (or `ValueError`, `ValidationError`) | `INVALID_INPUT` (400) | no | | anything else | `INTERNAL_ERROR` (500) | no | @@ -262,16 +266,16 @@ For anyone writing a consumer of the API, a durable strategy is: body. 2. **Branch on `error_code`.** For structured errors, this is the stable identifier to switch on. Do not use `message` as the identifier. -3. **Respect `retryable`.** When it is `true`, back off and try the request again. The - reference client retries `429`, `500`, `502`, `503`, and `504` with exponential - backoff. When absent, fix the request rather than repeating it. +3. **Respect `retryable`.** When it is `true`, back off and try the request again. + When absent, fix the request rather than repeating it. 4. **Fall back gracefully.** For an unrecognised code, show the `message` and the HTTP status. !!! note "Reference consumer" unshackle's own remote-download client treats any status of `400` or above as fatal, and a `200` body with `"status": "error"` and an `error_code` too: it logs - `Server error []: ` and exits. It also wraps - requests in a retry adapter (five attempts, backing off on `429`, `500`, `502`, - `503`, and `504`). This is a reasonable template for your own client's error + `Server error []: ` and exits. It retries a request once, + after one second, only when a `502`, `503` or `504` arrives with a body that is + not JSON: that is a page from the proxy in front of the server, not an answer + from the server. This is a reasonable template for your own client's error handling. diff --git a/docs/dev/rest-api/quickstart.md b/docs/dev/rest-api/quickstart.md index 8e1db805..e344d5d4 100644 --- a/docs/dev/rest-api/quickstart.md +++ b/docs/dev/rest-api/quickstart.md @@ -441,10 +441,12 @@ The `error_code` maps to an HTTP status. The ones you will meet most often: | `NOT_FOUND` | 404 | Title or resource does not exist | | `JOB_NOT_FOUND` | 404 | No job with that `job_id` | | `CONFLICT` | 409 | Illegal state transition (e.g. retry a running job) | +| `SERVICE_ERROR` | 422 | The service raised an error | | `RATE_LIMITED` | 429 | Upstream throttling (retryable) | | `INTERNAL_ERROR` | 500 | Unexpected server-side failure | -| `SERVICE_ERROR` | 502 | The service raised an error | | `NETWORK_ERROR` | 503 | Connection/timeout reaching the service (retryable) | +The [error code catalogue](errors.md#error-code-catalogue) lists every code. + !!! tip "Retry politely" Codes marked retryable (`NETWORK_ERROR`, `RATE_LIMITED`, `SERVICE_UNAVAILABLE`) reflect transient conditions. Back off and retry rather than hammering the server. diff --git a/tests/remote/e2e/test_live_search.py b/tests/remote/e2e/test_live_search.py index d0d5fa73..149880c7 100644 --- a/tests/remote/e2e/test_live_search.py +++ b/tests/remote/e2e/test_live_search.py @@ -20,7 +20,7 @@ def test_search_returns_results(http_session, server_url: str, service_case) -> ) if r.status_code in (400, 401, 403): pytest.skip(f"{service} search not available: {r.status_code} {r.text[:200]}") - if r.status_code == 502 and "not supported" in r.text.lower(): + if r.status_code == 422 and "not supported" in r.text.lower(): pytest.skip(f"{service} does not implement search") assert r.status_code == 200, r.text body = r.json() diff --git a/tests/remote/unit/test_errors.py b/tests/remote/unit/test_errors.py index 4b56ed10..546522b1 100644 --- a/tests/remote/unit/test_errors.py +++ b/tests/remote/unit/test_errors.py @@ -43,8 +43,8 @@ def test_api_error_default_http_status_per_code() -> None: APIErrorCode.INTERNAL_ERROR: 500, APIErrorCode.DOWNLOAD_ERROR: 500, APIErrorCode.WORKER_ERROR: 500, - APIErrorCode.SERVICE_ERROR: 502, - APIErrorCode.DRM_ERROR: 502, + APIErrorCode.SERVICE_ERROR: 422, + APIErrorCode.DRM_ERROR: 422, APIErrorCode.NETWORK_ERROR: 503, APIErrorCode.SERVICE_UNAVAILABLE: 503, } @@ -52,6 +52,8 @@ def test_api_error_default_http_status_per_code() -> None: assert {c.name for c in cases} == {c.name for c in APIErrorCode} for code, expected in cases.items(): assert APIError(code, "x").http_status == expected, code + # the edge proxy replaces a 502 or 504 body, so no code may default to either + assert not {502, 504} & set(cases.values()) def test_api_error_explicit_http_status_overrides_default() -> None: @@ -164,7 +166,7 @@ def test_handle_api_exception_categorizes_generic() -> None: (Exception("Not available in your region"), 403, "GEOFENCE", False), (Exception("Title not found"), 404, "NOT_FOUND", False), (Exception("HTTP 429 too many requests"), 429, "RATE_LIMITED", True), - (Exception("DRM license fetch failed"), 502, "DRM_ERROR", False), + (Exception("DRM license fetch failed"), 422, "DRM_ERROR", False), (Exception("503 service unavailable"), 503, "SERVICE_UNAVAILABLE", True), (ValueError("malformed body"), 400, "INVALID_INPUT", False), (RuntimeError("totally novel failure xyz"), 500, "INTERNAL_ERROR", False), diff --git a/unshackle/core/api/errors.py b/unshackle/core/api/errors.py index 3f64eb25..4466264c 100644 --- a/unshackle/core/api/errors.py +++ b/unshackle/core/api/errors.py @@ -94,10 +94,10 @@ class APIError(Exception): APIErrorCode.SESSION_NOT_FOUND: 404, APIErrorCode.TRACK_NOT_FOUND: 404, APIErrorCode.CONFLICT: 409, + APIErrorCode.SERVICE_ERROR: 422, + APIErrorCode.DRM_ERROR: 422, APIErrorCode.RATE_LIMITED: 429, APIErrorCode.INTERNAL_ERROR: 500, - APIErrorCode.SERVICE_ERROR: 502, - APIErrorCode.DRM_ERROR: 502, APIErrorCode.NETWORK_ERROR: 503, APIErrorCode.SERVICE_UNAVAILABLE: 503, APIErrorCode.DOWNLOAD_ERROR: 500, diff --git a/unshackle/core/api/handlers.py b/unshackle/core/api/handlers.py index 0dc36e3e..612cfbf3 100644 --- a/unshackle/core/api/handlers.py +++ b/unshackle/core/api/handlers.py @@ -5123,7 +5123,7 @@ def handle_proxy_license( ) # A service raises when the upstream licence server rejects the challenge. - # Surface it as a structured licence error, not an uncaught 500 the edge turns into a 502. + # Surface it as a structured licence error, not an uncaught 500. try: if drm_type == "widevine" and certificate: license_blob = service.get_widevine_service_certificate(