Add typed StatusError for non-429 API failures #1

Merged

eliribble merged 1 commit from typed-status-error into main

2026-09-17 01:41:43 +00:00

eliribble commented

2026-09-17 00:58:23 +00:00

Owner

responseError returned a formatted string for every non-429 failure, so callers could only classify a failure by matching English prose. The quad download path detects an unlicensed mosaic with strings.Contains(err.Error(), "Insufficient Privileges"), which misses the body the vendor documents for GET /mosaics/{id}/quads/{quad}/full: FirstMessage only understood the general/field envelope, so the flat {"message": "..."} document was embedded as raw JSON and never matched.

Change

responseError now returns *StatusError{StatusCode, Method, Path, Body} for every non-429 status; the 429 *RateLimitError branch stays first.
New StatusCode(err) (int, bool), errors.As-based, so callers can switch on 401/403/404 instead of on prose. A *RateLimitError is deliberately not a *StatusError.
StatusError.Error() keeps the historical METHOD path: status N: msg text (production logs depend on it) by re-parsing Body and falling back to the raw body when nothing parses.
APIError additionally decodes a top-level {"message": "..."} document; FirstMessage() precedence is general, then field, then top-level message. The flat body therefore renders as status 401: Insufficient Privileges instead of embedding the raw JSON.

Tests

Over the real doRedirect path against an httptest TLS server:

401/403/404/500 with both documented body shapes, asserting the typed status, the raw Body, and the exact Error() string.
500 with {"general":[{"message":"Internal Server Error"}]}.
429 asserting *RateLimitError, status 429, and RetryAfter (42s honoured; 1s default with no retry-in).
Non-StatusError and *RateLimitError yield ok=false; a wrapped *StatusError still unwraps.
Regression pin for the 2026-09-15 production body: {"message":"Insufficient Privileges"} at 401 renders GET basemaps/v1/mosaics/m/quads/q/full: status 401: Insufficient Privileges.
Every assertion fails against the pre-change code: the flat body rendered as status 401: {"message":"Insufficient Privileges"} and no *StatusError was returned.

Reviewer note (pre-existing, not introduced here)

go test ./... reports setup failures for cmd/planet and example/items-and-assets because they import the module path source.gleipnir.technology/Gleipnir/planet, which this module does not provide. This reproduces on a pristine checkout of main (verified from git archive HEAD), so the module root package is the meaningful target: go test . is green.

`responseError` returned a formatted string for every non-429 failure, so callers could only classify a failure by matching English prose. The quad download path detects an unlicensed mosaic with `strings.Contains(err.Error(), "Insufficient Privileges")`, which misses the body the vendor documents for `GET /mosaics/{id}/quads/{quad}/full`: `FirstMessage` only understood the general/field envelope, so the flat `{"message": "..."}` document was embedded as raw JSON and never matched. ## Change - `responseError` now returns `*StatusError{StatusCode, Method, Path, Body}` for every non-429 status; the 429 `*RateLimitError` branch stays first. - New `StatusCode(err) (int, bool)`, errors.As-based, so callers can switch on 401/403/404 instead of on prose. A `*RateLimitError` is deliberately not a `*StatusError`. - `StatusError.Error()` keeps the historical `METHOD path: status N: msg` text (production logs depend on it) by re-parsing Body and falling back to the raw body when nothing parses. - `APIError` additionally decodes a top-level `{"message": "..."}` document; `FirstMessage()` precedence is general, then field, then top-level message. The flat body therefore renders as `status 401: Insufficient Privileges` instead of embedding the raw JSON. ## Tests Over the real `doRedirect` path against an httptest TLS server: - 401/403/404/500 with both documented body shapes, asserting the typed status, the raw `Body`, and the exact `Error()` string. - 500 with `{"general":[{"message":"Internal Server Error"}]}`. - 429 asserting `*RateLimitError`, status 429, and RetryAfter (42s honoured; 1s default with no `retry-in`). - Non-StatusError and `*RateLimitError` yield `ok=false`; a wrapped `*StatusError` still unwraps. - Regression pin for the 2026-09-15 production body: `{"message":"Insufficient Privileges"}` at 401 renders `GET basemaps/v1/mosaics/m/quads/q/full: status 401: Insufficient Privileges`. - Every assertion fails against the pre-change code: the flat body rendered as `status 401: {"message":"Insufficient Privileges"}` and no `*StatusError` was returned. ## Reviewer note (pre-existing, not introduced here) `go test ./...` reports setup failures for `cmd/planet` and `example/items-and-assets` because they import the module path `source.gleipnir.technology/Gleipnir/planet`, which this module does not provide. This reproduces on a pristine checkout of `main` (verified from `git archive HEAD`), so the module root package is the meaningful target: `go test .` is green.

eliribble added 1 commit

2026-09-17 00:58:23 +00:00

Add typed StatusError for non-429 API failures 1c4165840e

responseError returned a formatted string for every non-429 failure, so
callers could only classify a failure by matching English prose. The quad
download path detects an unlicensed mosaic by looking for "Insufficient
Privileges" in the error text, which misses the body the vendor documents
for GET /mosaics/{id}/quads/{quad}/full: FirstMessage only understood the
general/field envelope, so the flat {"message": "..."} document was
embedded in the error as raw JSON and never matched.

responseError now returns *StatusError carrying the status, method, path
and raw body, and StatusCode reports the status of a (possibly wrapped)
*StatusError so callers can switch on 401/403/404 instead of on prose.
Error() keeps the historical "METHOD path: status N: msg" text that
production logs depend on by re-parsing the body, and APIError also decodes
a top-level message (general, then field, then message), so the flat body
now renders as "status 401: Insufficient Privileges".

429 still returns *RateLimitError with its parsed Retry-After, and is
deliberately not a *StatusError.

eliribble merged commit e336e44b7a into main

2026-09-17 01:41:43 +00:00

eliribble deleted branch typed-status-error

2026-09-17 01:41:43 +00:00

eliribble referenced this pull request from a commit

2026-09-17 01:41:44 +00:00

Merge pull request 'Add typed StatusError for non-429 API failures' (#1) from typed-status-error into main