Skip to content

Standardize API input validation and error handling #134

Description

@lvwerra

Goal and agreed scope

Make HTTP API failures predictable for the browser and supported agent/script clients: validate inputs before acting, reliably catch asynchronous request failures, and preserve a safe, useful error message and machine-readable reason through to the caller.

The operator approved this as one focused PR. Prioritize the existing Express 4 request/error boundary and frontend response decoding, not a framework migration or a rewrite of domain services. Safe trace re-imports and stable scheduled-target identities were explicitly deferred; their transaction/targeting behavior is not part of this issue.

Starting points

Planning inspected main at ef08e843d1c67fcc4e1c463415819ea351734d6d; recheck before implementing. The server declares Express 4. web/src/api.ts has a shared helper that reduces failures to their HTTP status, another that preserves the server message, and specialized handling for uploads, trace availability and sharing. Preserve those useful domain distinctions while consolidating the transport/error contract.

Express 4's router invokes handlers inside a synchronous try/catch but does not itself consume their returned promises; use an explicit request-scoped rejection-forwarding pattern on the current stack. Do not assume the automatic promise behavior described for Express 5 applies here. See the Express 4.19.2 router source. A global unhandled-rejection logger is not a substitute for answering the affected request.

This is a desired correctness/compatibility specification, not a public vulnerability reproduction. Follow SECURITY.md for any sensitive findings encountered during implementation; do not publish operator payloads, credentials or private review evidence.

Requirements

Area Required behavior
Inventory the boundary Inventory HTTP handlers/middleware, existing validation and browser/native consumers before migration. Identify JSON, text/plain, raw upload, download, stream/long-poll and deliberately empty responses. Record existing domain statuses, codes, supplemental fields and meaningful success responses containing ok: false; do not classify every such result as a transport failure. Keep streaming/WebSocket protocol handling explicit rather than forcing every endpoint through a JSON wrapper.
Reliable async handling Add a small shared async-handler/error boundary and cover applicable existing handlers and async middleware, including synchronous throws and rejected promises. Preserve local catches that intentionally isolate optional subsystems. Ensure each request receives one appropriate response or controlled stream termination, never hangs merely because a promise rejected, and never calls the error path twice. Event-emitter/callback failures need explicit forwarding/cleanup; a promise wrapper alone does not catch them.
Compatible error envelope Retain a top-level error string for existing clients, with a stable machine-readable code and narrowly defined optional safe fields such as validation details, conflict/revision data or retry guidance. Document the shape/status mapping and preserve established fields such as trace reason, lock reason, redaction hits and conflict tags during migration. Prefer an additive contract; do not replace the error string with an object or require all copied agent scripts to update at once.
Safe, truthful errors Expected domain failures get actionable messages and appropriate status codes. Unexpected internal errors get a generic safe message/code, not a stack trace, credential, file contents, raw upstream response or arbitrary exception text. Keep diagnostic correlation metadata bounded and safe. Do not downgrade a failed request to HTTP 200 or convert a legitimate successful empty response into a JSON parse failure. A completed partial operation must retain its documented partial-result semantics rather than falsely implying nothing happened.
Server-side input validation Validate types, required fields, enums, numeric bounds and semantic constraints at the request boundary before domain side effects. Cover mutation payloads and applicable path/query parameters using small reusable validators or schemas; TypeScript annotations alone are not validation. Inventory supported forms and aliases, including text prompts and uploads. Distinguish omitted/null/empty/false values; avoid blanket coercions that change meaning. Define unknown-field behavior per existing endpoint compatibility rather than silently dropping meaningful fields or globally rejecting formerly supported extension data.
Preserve mutation semantics Document whether an endpoint replaces an object, patches selected fields, executes a command or starts asynchronous work. Keep existing valid client behavior and supported PUT/PATCH routes; this is not permission to rename endpoints or change partial-update semantics wholesale. Coordinate settings validation with #123. Persisted-store schemas, durable commit guarantees, corrupt-state recovery and import transaction changes remain outside scope; if one is necessary, identify the separate blocker rather than expanding this PR.
Browser decoding Consolidate JSON/status/error decoding into one tested transport contract with a typed error carrying HTTP status, stable code and safe optional domain data. Preserve compatibility with legacy { error: string } responses. Handle empty, malformed and non-JSON failure responses from proxies or interrupted connections without hiding the status behind a JSON syntax error or rendering arbitrary HTML. Update applicable call sites so expected validation/conflict errors are useful rather than bare numbers. Keep uploads' progress/cancel behavior and their XMLHttpRequest path.
Preserve domain and cancellation behavior Keep equivalent behavior for TraceUnavailable/no-trace, sharing redaction refusal, file revision conflicts, rate limits and privacy locks. A new session with no trace must still show its usable Reader composer. Distinguish deliberate abort/cancellation from an HTTP rejection, timeout, offline/network failure or unreadable successful response. Do not replace existing Reader deadlines or remote long-poll timing with a blanket short timeout. Error metadata is not authority to retry: do not automatically replay state-changing requests or imply uncertain delivery did not happen.
Streams and committed responses Before headers are sent, return the documented structured failure where applicable. After streaming/headers have begun, do not append JSON into a file/terminal stream, send another status or trigger headers-already-sent errors. Close/cancel or use the protocol's existing failure mechanism, releasing timers/listeners/resources. Client disconnect and late async completion must not create a second response, retry an operation or affect unrelated sessions. Do not redesign WebSocket framing or remote protocols.
Routing, guards and diagnostics Add deliberate API not-found/body-parser error handling so ordinary API mistakes do not fall through to an HTML app page. Preserve static asset and SPA routing. Keep origin/privacy/attribution guards and their ordering/semantics; coordinate #130/#131 rather than bypassing them to simplify error handling. Preserve one accurate audit outcome and the filtering requirements of #132; do not broaden rejected-body capture or add secret-bearing diagnostics. A request wrapper must not introduce a process restart/agent revival policy.

Required tests

Use real local HTTP/middleware integration plus focused decoder/validator and browser tests with isolated state, fake agents/dispatchers and mocked external services. Test actual responses and side effects, not only construction of an Error object.

Area Required cases and assertions
Async failures Inject a synchronous throw, immediate and delayed rejection, non-Error rejection, callback/stream error and late failure after a response. Requests settle exactly once with the documented outcome, without escaping to an unhandled rejection, hanging or sending a second response. A subsequent unrelated request still succeeds.
Validation before action Missing/wrong-type/null/empty values, arrays where scalar values are expected, duplicate/structured query values, invalid enum/numeric bounds, malformed JSON and unsupported content types under the documented policy. Assert zero domain calls, file writes, notifications or agent input for rejected inputs. Include valid boundary values, supported aliases/text payloads and existing replacement/patch semantics as positive cases.
Contract and client compatibility Existing error string plus added code/details; old-style server errors consumed by the new browser; supported native/internal/remote clients; 400/403/404/409/413/429/5xx as applicable. Preserve existing domain fields and success payloads, including deliberately empty success and meaningful ok: false results.
Domain UI Useful session/group validation messages, file conflict details, rate-limit feedback, public-share redaction refusal and trace-unavailable/no-conversation states. Test the actual affected controls, retaining user input/drafts and existing failure actions. No empty-state substitution for a failed fetch, blanket popup system or per-poll toast spam.
Network and parsing Non-JSON HTML/text failure, empty failure body, malformed JSON failure, malformed expected-JSON success, abort, timeout and connection loss. Messages remain safe and useful, the HTTP status is retained where present, and no automatic mutation replay occurs. Error responses should be read once, with bounded diagnostic content.
Uploads and streams Real XMLHttpRequest success/progress/cancel/error decoding; file/download and remote long-poll failures before/after headers; client disconnect and late callbacks. No JSON appended to stream bytes, duplicate work, leaked listeners/timers or changed legitimate long-poll behavior.
Routing and protection API not-found and parser errors versus ordinary SPA/static requests; privacy/origin/attribution denial retains its code/reason and zero forbidden effects. Test with guards enabled and safe synthetic credentials. Audit recording reflects the actual final response once, without raw sensitive failure content.
Error safety and regressions Inject credential-like strings, private paths, payload text and markup into internal/upstream exceptions. They must not leak through public error bodies or unsafe diagnostics. Verify existing normal API fixtures, Reader/terminal separation, save controllers and unaffected optional-subsystem isolation.
  • Add normally discovered tests. Run focused API/validator/decoder suites, relevant files/settings/attachments/trace/remote/operations integrations and browser cases, normal server/web suites, and frontend typecheck/production build. Report exact commands and unrelated baseline failures separately; still run the new focused suites if an aggregate runner stops early. Do not implement the deferred CI project to satisfy this requirement.
  • Include a route/response compatibility inventory, error-code/status table, documented validator conventions and concise examples of adding a new handler/client. Keep helpers small; no backend TypeScript conversion, schema-generation platform or giant route extraction is required.

Integration and exclusions

Start with server/src/index.js, applicable route-level validators, web/src/api.ts, shared TypeScript API types and error-consuming components/tests. Preserve the approved boundaries in #121, #122, #123, #124, #129, #130, #131 and #132. Share applicable helpers; do not duplicate or take over their feature work.

No safe-import replacement implementation, scheduled-target identity changes, core-store durability/recovery rewrite, automatic retries/delivery receipts, process-supervision/restart policy, new authentication, general dialog/UI redesign, Express major-version migration, mass dependency upgrades, build/CI changes or deployment. Any incompatible client/API behavior needs a separately agreed migration, not an undocumented breaking change. No real agent prompts/stops, notifications, live credential/log reads, hosting changes or paid jobs in tests. Open one focused PR; merging and deployment require separate authorization.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions