Uh oh!
There was an error while loading. Please reload this page.
fix(runtime): serve VALIDATION_FAILED as 400 with fields[] from both dispatcher exits (#3918) - #3920
Merged
Conversation
…dispatcher exits (#3918) `ValidationError` carries `.code = 'VALIDATION_FAILED'` and `.fields[]`, and deliberately carries no `.status`, no `.statusCode` and no `.issues` — deciding it means "400" is the HTTP boundary's job. `@objectstack/rest` has always done that (`mapDataError`). The runtime dispatcher's two error exits did not, because each read exactly the properties this error lacks: - `HttpDispatcher.errorFromThrown` fell back to the caller's `fallbackStatus` for want of a `.status`, and built `details` from `.issues` alone — so `fields[]` was dropped. - `dispatcher-plugin`'s `errorResponseBase` took the same 500 fallback, and its body was only `{message, code}`. Landing on 5xx then dragged the message through the #3867 leak sanitiser, so a bad email address came back as a 500 "Internal server error" with nothing to attach to the offending input. Both exits now answer the way rest-server does: status 400, with `fields[]` passed through verbatim in `details` alongside `code: 'VALIDATION_FAILED'`, so every surface the dispatcher serves can do per-field error display. The predicate lives in the new `validation-failure.ts` — one definition shared by both exits, duck-typed on `code`/`name` exactly like `mapDataError`, so the runtime takes no dependency on objectql and hand-thrown errors of the same shape are served identically. An explicit `.status`/`.statusCode` still wins; 400 is supplied only as the fallback that used to be 500. Non-validation errors keep their status, message sanitising and `details` untouched, and `errorResponseBase` still emits its exact two-key body for them. Covered by `dispatcher-validation-error.test.ts`, which drives both exits through real routes (`/packages/:id/publish-drafts` for the returned path, `POST /analytics/query` for the thrown one); 11 of its 15 cases fail on the pre-fix source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LeZ7DmSKjiKmQzYh8L5CJS
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 18 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes#3918.
The bug
ValidationError(objectql's record/rule validators) carries.code = 'VALIDATION_FAILED'and.fields[]— one entry per offending field. It deliberately carries no.status, no.statusCode, and no.issues: it's a plain domain error, and deciding it means "400" is the HTTP boundary's job.@objectstack/resthas always done that (mapDataError→ 400 withfields[]). The runtime dispatcher's two error exits did not, because each read exactly the properties this error lacks:http-dispatcher.ts—errorFromThrown(the returned-error path:/metasave,/packagespublish, …).status→ fell back to the caller'sfallbackStatus(500 on/packages);detailsbuilt from.issuesalone →fields[]droppeddispatcher-plugin.ts—errorResponseBase(the thrown-error path: every route the plugin mounts —/analytics,/packages,/i18n,/storage,/automation,/auth,/notifications,/mcp, …){message, code}. Landing on 5xx then dragged the message through the #3867 leak sanitiser — so a user typing a bad email address got back a 500 "Internal server error": no status a client could act on, no message worth showing, nothing to attach to the inputNet effect: per-field error display was foreclosed on every surface the runtime dispatcher serves.
The fix
New
packages/runtime/src/validation-failure.tsholds one predicate shared by both exits, duck-typed oncode === 'VALIDATION_FAILED' || name === 'ValidationError'— the same both-ways testmapDataErroruses. So the runtime takes no dependency on objectql, and a hook that throws{ code: 'VALIDATION_FAILED', fields }by hand is served identically.Both exits now answer the way rest-server does: status 400, with the error's
fields[]passed through verbatim indetailsalongsidecode: 'VALIDATION_FAILED'.Two deliberate calls worth a reviewer's attention:
.status/.statusCodestill wins. 400 is supplied only as the fallback that used to be 500. rest-server hardcodes 400; keeping the dispatcher's existing precedence made this a strictly smaller change, and it's equivalent for a realValidationError(which has neither).fields[]goes inerror.details, not at the body's top level. The dispatcher's envelope is{success, error: {message, code, details}}throughout, wherecodeis the HTTP status and structured detail lives indetails; rest-server's flat{error, code, fields}shape doesn't exist here. This matches rest-server's semantics (400 +fields[]present) inside the dispatcher's own envelope rather than reproducing its literal layout. Happy to mirror the list aterror.fieldstoo if you'd rather — it's a one-liner.Non-validation errors are untouched: same status, same message sanitising, same
details, anderrorResponseBasestill emits its exact two-key body for them.Verification
dispatcher-validation-error.test.ts, driving both exits through real routes rather than calling the private helpers —/packages/:id/publish-draftsfor the returned path (the 500-fallback caller, i.e. where the downgrade actually bit),POST /analytics/queryfor the thrown one.tsc --noEmiton the package reports 11 pre-existingTS6133unused-variable errors inaction-execution.ts,domains/mcp.tsanddomains/meta.ts— files this change doesn't touch. None in the changed files.Changeset
Added a patch changeset. AGENTS.md says pure bug fixes don't require one, but the wire behaviour visibly changes for consumers (500 → 400, new
details), so it seemed worth landing in the CHANGELOG. Easy to drop if you disagree.Generated by Claude Code