fix(aw-server): return JSON errors with a reason for 400/404/422 under /api

[TimeToBuildBob]

Request-guard failures (malformed event JSON, bad body, unknown /api route) returned Rocket's default HTML page; the real reason only reached the server log ("No 422 catcher registered"). Clients can't tell what failed (see #174). aw-server (Python) already returns JSON {"message": ...}.

• Registers 400/404/422 catchers on /api returning the same {"message": ...} shape as HttpErrorJson.
• Adds ApiJson<T>, a drop-in for Json<T> on the event/bucket/heartbeat/query/settings/import-JSON handlers that stashes the serde error so the catcher can include it, e.g. Unprocessable Entity: missing field \timestamp` at line 1 column 15`.
• Test: test_api_errors_are_json_with_reason. cargo test -p aw-server, clippy and fmt clean.

Not included: ?limit=abc being silently ignored (separate change).

[greptile-apps]

Confidence Score: 5/5

[Medium risk] API error responses now return structured JSON with details.

No outstanding finding or confirmed new issue prevents merging.

Summary

The PR adds JSON 400, 404, and 422 catchers under /api and preserves JSON body-parsing errors for client responses. The latest change adds a 400-response assertion to the API error test.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Request[API request] --> Guard[JSON body guard]
  Guard -->|Valid| Handler[Endpoint handler]
  Guard -->|Invalid| Cache[Request-local error]
  Cache --> Catcher[API error catcher]
  Request -->|Route or request error| Catcher
  Catcher --> Response[JSON message response]

Loading

_{Reviews (2) · Last reviewed commit: "test(aw-server): cover the 400 catcher w..."}

[codecov]

Codecov Report

❌ Patch coverage is 96.96970% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.70%. Comparing base (656f3c9) to head (118ca08).
⚠️ Report is 182 commits behind head on master.

Files with missing lines	Patch %	Lines
aw-server/src/endpoints/util.rs	88.88%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #772       +/-   ##
===========================================
+ Coverage   70.81%   83.70%   +12.88%     
===========================================
  Files          51       82       +31     
  Lines        2916    10664     +7748     
===========================================
+ Hits         2065     8926     +6861     
- Misses        851     1738      +887     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

CI status note: both red checks are unrelated to this diff.

• clippy — the repo-wide block_scrutinee break from the Rust 1.99 toolchain (aw-query/src/parser.rs). Tracked and fixed by fix(query): allow clippy::block_scrutinee in generated parser #771 (green, needs maintainer merge). Clippy here goes green once fix(query): allow clippy::block_scrutinee in generated parser #771 lands on master and CI re-runs.
• windows-latest — timing flake in aw-client-rust test_wait_for_start_retries_without_blocking_executor, untouched by this PR.

The Greptile P2 (400 catcher untested) is addressed in c2ed8b8.

[TimeToBuildBob]

CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

[TimeToBuildBob]

🤖 AI code review

Adds 400/404/422 catchers scoped to /api that return JSON error bodies, plus an ApiJson<T> data guard that records the serde parse error in a request-local cache so the 422 catcher can include a reason, and replaces Json<T> on the JSON POST handlers for buckets, events, heartbeat, query, settings and import. Also trims two clippy allow attributes in aw-query and adds an API-error integration test.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No thread-worthy findings. Advisory notes follow; they are retained without opening review threads.

2 advisory findings (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 medium — aw-server/src/endpoints/import.rs:152

The new reason mechanism only works for handlers switched to ApiJson<T>. bucket_import_form still parses its payload through Form<ImportForm> with an embedded Json<BucketsExport> (import.rs:152), so a malformed JSON body inside the multipart upload fails the Form data guard before any ApiJson::from_data runs and BodyError is never populated. The matching /api catcher (400 or 422) therefore returns {"message":"Bad Request"} or {"message":"Unprocessable Entity"} with no serde detail, while the sibling POST /api/0/import JSON route now returns the full reason. A client whose multipart import has a typo gets exactly the unhelpful message this PR was written to remove, and diagnosis still falls back to server logs. The Form guard's error needs the same stash, or the form route's parse error text surfaced.

_{How this was verified: BodyError is only written in ApiJson::from_data (util.rs:77); bucket_import_form at import.rs:158 bypasses that path entirely, so no reason text is available to api_error_json.}

⚠️ P2 medium — aw-server/src/endpoints/util.rs:89

api_error_json makes Rocket's reason phrase plus the raw serde_json rendering of the parse failure the client-facing message, e.g. "Unprocessable Entity: invalid type: map, expected a sequence at line 1 column 16". The tail is Rocket's JsonError Display text, not a stable public contract; serde_json/Rocket have rephrased these strings between releases. Because this message is now the documented replacement for the default HTML page, any API client parsing body["message"] (and the new test, which asserts msg.contains("line 1")) depends on internal wording and on serde's byte-column arithmetic. The position info is also noise for most callers, who want the missing field name, not offsets. Keeping the human phrase stable and returning the serde text as a separate structured detail (e.g. an object with message/reason) would make the wire contract robust to dependency upgrades.

_{How this was verified: The only 422 catcher output is api_error_json, which forwards serde's Display text unchanged; api.rs:961 pins the 'line 1' rendering, confirming the test depends on that unstable format.}

Files changed (8) — the diff as I read it

• aw-query/src/lib.rs — Removes clippy::block_scrutinee and unknown_lints from the parser module's allow list, keeping match_single_binding, redundant_closure_call, and unused_braces.
• aw-server/src/endpoints/bucket.rs — Switches bucket_new, bucket_events_create, and bucket_events_heartbeat from Json<T> to ApiJson<T> data guards.
• aw-server/src/endpoints/import.rs — Imports ApiJson and changes bucket_import_json's data guard to ApiJson<BucketsExport>.
• aw-server/src/endpoints/mod.rs — Registers catchers for 400/404/422 under /api that return HttpErrorJson with a reason.
• aw-server/src/endpoints/query.rs — Changes query's data guard from Json<Query> to ApiJson<Query>.
• aw-server/src/endpoints/settings.rs — Changes setting_set's data guard from Json<serde_json::Value> to ApiJson<serde_json::Value>.
• aw-server/src/endpoints/util.rs — Adds ApiJson<T>, its FromData impl that stashes the serde error in a request-local BodyError, and api_error_json for the catchers.
• aw-server/tests/api.rs — Adds test_api_errors_are_json_with_reason covering malformed bodies, unknown routes, and invalid URIs under /api.

Previous review passes

commit	score	findings	engine	when
4bc6f41233c2	5/5	0	llm	2026-10-02 03:59 UTC

_{Reviewed 8343b0c1e39d · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 919s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

[TimeToBuildBob]

CI update: the new red clippy is unrelated to this diff. It's a clippy::duplicated_attributes error in aw-query/src/{lib,parser}.rs. #771 fixed the earlier block_scrutinee break, but it also added duplicate #[allow]s, and master's Lint has been red since it merged.

The fix is #779 (all checks green, mergeable). Once #779 lands on master, re-running CI here should turn green. No code change needed on this branch.

[TimeToBuildBob]

Rebased onto master (conflict in aw-query/src/lib.rs: removed duplicate unknown_lints that #771 re-added to the outer #[allow] on mod parser). CI running on 60c96cc.

[TimeToBuildBob]

Clippy on 60c96cc was this PR, not master: the rebase conflict resolution dropped clippy::block_scrutinee from the outer #[allow] on mod parser. parser.rs inner attrs do not cover that lint (9 errors).

Restored the allow list to match master in a3c139c. Master already landed the duplicate-attr cleanup in #785 and is clippy-green. Local cargo clippy -p aw-query -- -D warnings is clean.

[ErikBjare]

@TimeToBuildBob Conflicts