diff --git a/AGENTS.md b/AGENTS.md index 846c97b..cbb9bc8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -162,6 +162,24 @@ parsed), `ServerError` (5xx), and `MaintenanceError`. `except NotFoundError` for batch callers or fork four ways for one integer, and `MaintenanceError` stays the *one* body-keyed mapping. `_index()` tests `isinstance(value, int)` rather than truthiness, since index 0 is a real answer. +- **`LightOnAPIError.fields`** is the 422 envelope's per-field validation errors, + `{field: [{error, detail}]}`, `{}` on any other failure. The top-level `detail` of a + validation error is a **constant sentence**, so without this every 422 stringifies + identically and a log of them names nothing (reported from production: 26 failures, + 26 identical rows, two unrelated causes). Parsed in the base `__init__` by `_fields()`, + same reasoning as `index`: every subclass inherits it and `from_response` stays the + single construction point. Also **folded into the message** by `_field_summary()`, as a + one-line `(choices: must be a non-empty list; title: may not be blank)` group placed + after `detail` and **before** `(action N)`, so the whole error stays one log row and + the batch position stays last. Raw passthrough of the wire shape, no curated model: + the machine-readable `error` code beside each `detail` is worth keeping, and pydantic + in the exception tree could fail validation *while* an error is being built. Every + level of `_fields()` is defensive for that reason (unexpected shape degrades to `{}`, + never raises), and an entry with no `detail` falls back to its `error` code so a named + field always has a cause. Not a new exception class: `fields` rides on 400/422 alike + and `MaintenanceError` stays the one body-keyed *mapping*, this is a body-derived + *attribute* like `index`. The envelope's `error`/`doc_url`/`code` keys stay unsurfaced; + `.body` is the untouched payload and is now documented as such. ## Resource management: active-record diff --git a/README.md b/README.md index 23fbfe8..43bea9d 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ This SDK wraps the LightOn API. Create an account and get an API key on [console - [Content types](#content-types) - [API keys](#api-keys) - [Client configuration](#client-configuration) +- [Errors](#errors) - [Agent Frameworks](#agent-frameworks) ## Quick start @@ -908,10 +909,11 @@ except LightOnAPIError as e: print("already applied:", actions[:e.index]) ``` -`index` is `None` on any non-batch error. Past 50 actions `batch_facets()` raises a -`ValueError` before the request goes out: split the list yourself rather than have -the SDK guess where to cut, since chunking would restore the per-chunk reindex the -endpoint exists to avoid. +`index` is `None` on any non-batch error. See [Errors](#errors) for the rest of what an +exception carries, including the per-field causes of a 422. Past 50 actions +`batch_facets()` raises a `ValueError` before the request goes out: split the list +yourself rather than have the SDK guess where to cut, since chunking would restore the +per-chunk reindex the endpoint exists to avoid. ### Building the taxonomy @@ -1041,6 +1043,68 @@ with LightOn(config=config) as client: ... ``` +## Errors + +Everything the SDK raises descends from `LightOnError`, so one `except` catches the lot: + +```python +from lighton.exceptions import LightOnError, NotFoundError, RateLimitError +``` + +| exception | when | +| --- | --- | +| `LightOnError` | base class; catch this to catch everything | +| `LightOnConnectionError` | the request never reached the API (DNS, timeout, reset) | +| `MalformedResponseError` | a 2xx body that wasn't valid JSON | +| `StreamError` | the server sent an `error` event partway through a streamed `ask` | +| `LightOnAPIError` | any non-2xx response; the parent of everything below | +| `AuthenticationError` | 401, the API key is missing or wrong | +| `PermissionDeniedError` | 403, the key is valid but not allowed here | +| `NotFoundError` | 404 | +| `RateLimitError` | 429; also carries `retry_after` in seconds, or `None` | +| `ServerError` | 5xx | +| `MaintenanceError` | a 503 from a planned window, not a crash; a `ServerError` subclass | + +Every `LightOnAPIError` carries four attributes: + +| attribute | what it holds | +| --- | --- | +| `status_code` | the HTTP status | +| `body` | the decoded error payload, exactly as the API sent it and never stripped | +| `fields` | per-field validation errors of a 422, keyed by field name; `{}` otherwise | +| `index` | the 0-based position of the failing action in a batch request; `None` otherwise | + +**`fields` is where a 422 actually explains itself.** Its top-level `detail` is a fixed +sentence, the same one for every validation failure, so the field name and its cause are +the part worth logging. They are already in the message, so a bare traceback names the +offender: + +``` +lighton.exceptions.LightOnAPIError: 422 Unprocessable Entity: One or more fields +failed validation. (choices: must be a non-empty list; title: may not be blank) +``` + +and the structured form is there when you want to branch on it rather than print it: + +```python +from lighton.exceptions import LightOnAPIError + +try: + client.extract(path="contract.pdf", schema=MySchema) +except LightOnAPIError as e: + for name, errors in e.fields.items(): + for err in errors: + print(name, err["error"], err["detail"]) + # choices invalid must be a non-empty list +``` + +Each entry keeps the API's machine-readable `error` code alongside the human `detail`. +Anything the attributes above don't name is still on `.body`. + +Retries are handled for you: connection failures and HTTP 429 are retried per +[Client configuration](#client-configuration) before any exception reaches you, so a +`RateLimitError` means the retries were already spent. 5xx is never retried. + ## Agent Frameworks LightOn drops into any agent framework as a **retrieval tool**: wrap a `client.search()` diff --git a/lighton/exceptions.py b/lighton/exceptions.py index 9c137fa..f23dc98 100644 --- a/lighton/exceptions.py +++ b/lighton/exceptions.py @@ -37,6 +37,18 @@ def __init__(self, message: str, *, body: Any = None) -> None: class LightOnAPIError(LightOnError): """The API returned a non-2xx response. + `status_code` is the HTTP status, and `body` is the decoded error payload + exactly as the API sent it (the parsed JSON, or the raw text when it wasn't + JSON, or None when there was no body). Nothing is stripped from it, so + anything this class doesn't name is still readable there. + + `fields` holds the per-field validation errors of a 422, keyed by field name: + `{"choices": [{"error": "invalid", "detail": "must be a non-empty list"}]}`. + The top-level `detail` of a validation error is a constant sentence, so this + is the part that says what actually went wrong; it is already folded into the + message, and this attribute is for callers that want to branch on it. Empty + for every error that isn't a field-level rejection. + `index` is set only on a **batch** endpoint failure: the 0-based position of the action that failed. Batch endpoints validate every action's fields up front (a field-level 422 executes nothing), then run in order and stop at the @@ -50,6 +62,7 @@ def __init__(self, message: str, *, status_code: int, body: Any = None) -> None: super().__init__(message) self.status_code = status_code self.body = body + self.fields: dict[str, list[dict[str, Any]]] = _fields(body) self.index: int | None = _index(body) @@ -143,6 +156,9 @@ def from_response(response: httpx.Response) -> LightOnAPIError: message = f"{response.status_code} {response.reason_phrase}" if detail: message = f"{message}: {detail}" + summary = _field_summary(_fields(body)) + if summary: + message = f"{message} ({summary})" index = _index(body) if index is not None: message = f"{message} (action {index})" @@ -185,6 +201,53 @@ def _index(body: Any) -> int | None: return value if isinstance(value, int) else None +def _fields(body: Any) -> dict[str, list[dict[str, Any]]]: + """Per-field validation errors keyed by field name, or {} when there are none. + + Passed through in the API's own shape, so the machine-readable `error` code + beside each `detail` survives. Defensive at every level because this runs + while an exception is being built: a server shape change has to degrade to an + empty mapping, never raise on top of the error it was meant to describe. The + untouched payload stays on `.body` either way. A key whose list yields no + entries is dropped, so a non-empty result always has something to say. + """ + raw = body.get("fields") if isinstance(body, dict) else None + if not isinstance(raw, dict): + return {} + fields: dict[str, list[dict[str, Any]]] = {} + for name, errors in raw.items(): + if not isinstance(errors, list): + continue + entries = [e for e in errors if isinstance(e, dict)] + if entries: + fields[str(name)] = entries + return fields + + +def _field_summary(fields: dict[str, list[dict[str, Any]]]) -> str: + """Render `fields` for the message: `name: why, why; name: why`. + + Falls back to the `error` code when an entry carries no readable `detail`, so + a named field is never left without a cause. ponytail: uncapped, `fields` is + keyed by request-body field and so bounded by the request schema; cap it here + if an endpoint ever reports per-item errors. + """ + parts = [] + for name, errors in fields.items(): + causes = [c for c in (_cause(e) for e in errors) if c] + if causes: + parts.append(f"{name}: {', '.join(causes)}") + return "; ".join(parts) + + +def _cause(entry: dict[str, Any]) -> str | None: + for key in ("detail", "error"): + value = entry.get(key) + if isinstance(value, str) and value: + return value + return None + + def _timestamp(raw: Any) -> datetime | None: """Parse an ISO-8601 timestamp, or None. The raw value stays on `.body`.""" if not isinstance(raw, str): diff --git a/tests/test_client.py b/tests/test_client.py index 9f71750..02a491f 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -313,3 +313,115 @@ def handler(req: httpx.Request) -> httpx.Response: with pytest.raises(exc.MaintenanceError): make_client(handler, rate_limit_retries=3).ask("q") assert calls["n"] == 1 + + +# --- field-level validation errors ------------------------------------------ +# The top-level `detail` of a 422 is a constant sentence, so every validation +# failure would otherwise stringify identically and a log of them names nothing. +# The cause lives in `fields`, which the message and a dedicated attribute both +# carry now. Parsing happens in the base __init__, so it reaches every subclass. + +_VALIDATION_BODY = { + "code": 422, + "error": "validation_error", + "detail": "One or more fields failed validation.", + "fields": {"choices": [{"error": "invalid", "detail": "must be a non-empty list"}]}, +} + + +def test_validation_422_names_the_failing_field_in_the_message(): + client = make_client(lambda req: httpx.Response(422, json=_VALIDATION_BODY)) + + with pytest.raises(exc.LightOnAPIError) as excinfo: + client.ask("q") + + e = excinfo.value + assert str(e) == ( + "422 Unprocessable Entity: One or more fields failed validation. " + "(choices: must be a non-empty list)" + ) + assert e.fields == _VALIDATION_BODY["fields"] # the code beside it survives + assert e.body == _VALIDATION_BODY # the untouched payload is still there + + +def test_field_errors_join_across_fields_and_causes(): + body = { + "detail": "One or more fields failed validation.", + "fields": { + "choices": [ + {"error": "invalid", "detail": "must be a non-empty list"}, + {"error": "type", "detail": "must contain strings"}, + ], + "title": [{"error": "blank", "detail": "may not be blank"}], + }, + } + client = make_client(lambda req: httpx.Response(422, json=body)) + with pytest.raises(exc.LightOnAPIError) as excinfo: + client.ask("q") + assert ( + "(choices: must be a non-empty list, must contain strings; " + "title: may not be blank)" in str(excinfo.value) + ) + + +def test_field_errors_and_the_batch_index_both_land_in_the_message(): + # Order matters: what went wrong, then which action it went wrong on. + body = {**_VALIDATION_BODY, "index": 3} + client = make_client(lambda req: httpx.Response(422, json=body)) + with pytest.raises(exc.LightOnAPIError) as excinfo: + client.ask("q") + assert str(excinfo.value).endswith("(choices: must be a non-empty list) (action 3)") + assert excinfo.value.index == 3 + + +def test_an_error_without_fields_is_unchanged(): + """The no-regression pin: errors that aren't field-level must read as before.""" + client = make_client(lambda req: httpx.Response(404, json={"detail": "nope"})) + with pytest.raises(exc.NotFoundError) as excinfo: + client.ask("q") + assert str(excinfo.value) == "404 Not Found: nope" + assert excinfo.value.fields == {} + + +@pytest.mark.parametrize( + "fields", + [ + None, + "choices must be a non-empty list", # a string where a mapping was promised + [{"error": "invalid"}], # a list + {"choices": "must be a non-empty list"}, # value is not a list + {"choices": ["must be a non-empty list"]}, # entries are not objects + {"choices": []}, # nothing to say -> not named at all + ], +) +def test_malformed_fields_degrade_to_empty_without_breaking_the_error(fields): + body = {"detail": "One or more fields failed validation.", "fields": fields} + client = make_client(lambda req: httpx.Response(422, json=body)) + with pytest.raises(exc.LightOnAPIError) as excinfo: + client.ask("q") + e = excinfo.value + assert e.fields == {} + assert str(e) == "422 Unprocessable Entity: One or more fields failed validation." + assert e.body == body # raw value preserved + + +def test_a_field_error_without_a_detail_falls_back_to_its_code(): + # Naming the field with no cause at all would be worse than naming the code. + body = {"detail": "invalid", "fields": {"choices": [{"error": "required"}]}} + client = make_client(lambda req: httpx.Response(422, json=body)) + with pytest.raises(exc.LightOnAPIError) as excinfo: + client.ask("q") + assert "(choices: required)" in str(excinfo.value) + + +def test_field_errors_reach_the_subclasses(): + # Parsed in the base __init__, so no subclass has to opt in. + body = { + "detail": "not found", + "fields": {"file_id": [{"detail": "does not exist"}]}, + } + client = make_client(lambda req: httpx.Response(404, json=body)) + with pytest.raises(exc.NotFoundError) as excinfo: + client.ask("q") + assert excinfo.value.fields == {"file_id": [{"detail": "does not exist"}]} + assert "(file_id: does not exist)" in str(excinfo.value)