fix(runtime): route every domain catch through errorFromThrown (#3918 follow-up) - #3925
Merged
Merged
Conversation
…follow-up) #3867 taught `errorResponseBase` to read `status` (not just `statusCode`) and #3918 taught `errorFromThrown` the `VALIDATION_FAILED` shape. Both fixes were invisible to the tier underneath: the domain modules each caught their own errors and called `deps.error(e.message, e.statusCode || 500)` directly, bypassing `errorFromThrown` — 13 call sites, 9 of them in `/packages`. So on `/packages`, `/meta/_drafts`, `/ui`, `/security` and the `/mcp` transport, a deliberate 404 still rendered as a 500 (every protocol-layer domain error carries its status as `status`, the exact read #3867 fixed one tier up), and a `ValidationError` still lost its `fields[]` and its 400 — re-opening #3918 on the very routes it was filed against. Every such catch now calls `deps.errorFromThrown(e, …)`. Deliberate per-route fallbacks are preserved rather than flattened to 500: `/meta` save keeps 501, the `/meta` two-part lookup keeps 404 — but a validation failure on either now answers 400 with its fields instead of being swallowed by the fallback. `domains/keys.ts` is deliberately NOT converted: it discards the underlying error on purpose because the message could echo row contents, so its literal 'Failed to create API key' stays. Covered by `domains/error-passthrough.test.ts` — 13 cases driven through the real routes, 8 of which 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
|
github-actions
Bot
added
documentation
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 was referenced Jul 29, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 29, 2026
…ccess:false (#3913 follow-up) (#3951) #3937 settled that a failed action reports in the payload at HTTP 200 — "an action that fails is a normal outcome, not a transport error". That is a statement about the action REJECTING: a business rule saying no. The same exit was also covering a third case it never argued for. A TypeError in a handler, a driver blowing up, a sandbox timeout are not outcomes the action chose to report — they are the server failing to produce one. Serving them as 200 hid every handler crash from the layers that exist to catch server faults: gateway error rates, retry and circuit-breaker policy, APM auto-capture, alerting, fetch().ok. Those are 500 now, through the same errorFromThrown exit every other domain catch has used since #3925 — which also puts a driver dump behind the internal-error-leak sanitiser (#3867) instead of letting it reach the client verbatim in a 200 body. Nothing #3937 put in the payload moves. Rejection and crash are told apart by the error's NAME, the signal @objectstack/rest already uses on this exact distinction: a plain Error, a SandboxError carrying innerMessage, anything with code/fields, and a ValidationError by name are rejections; TypeError / ReferenceError / a driver's own class, and a SandboxError with no innerMessage (timeout, capability denial) are crashes. A throw with no name at all is not confidently a fault and keeps its 200 — deliberately the narrow direction. Also in the same exit: an error carrying its own status/statusCode (a plugin's FORBIDDEN with status 403) is served with it rather than buried in a 200 payload. Record ValidationErrors deliberately carry no .status, so #3937's cases never reach that branch. Documented in api/error-catalog.mdx (new Action Errors section with the full status table and the two-check pattern a raw fetch caller needs) and ui/actions.mdx.
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 free
to 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.
Follow-up to #3920 (which closed #3918). Same family of bug, one tier further down — found while doing the #3918 work, not fixed there because it was out of that issue's stated scope.
The bug
#3867 taught
dispatcher-plugin'serrorResponseBaseto read an error'sstatus(not juststatusCode). #3920 taughtHttpDispatcher.errorFromThrowntheVALIDATION_FAILEDshape. Both fixes were invisible to a whole tier of handlers underneath them: the domain modules each catch their own errors and calleddeps.error(e.message, e.statusCode || 500)directly, bypassingerrorFromThrownentirely — 13 call sites, 9 of them in/packagesalone.So on
/packages,/meta/_drafts,/ui,/securityand the/mcptransport:status, notstatusCode—OBJECT_NOT_FOUND,RECORD_NOT_FOUND,CLONE_DISABLED, plugin-sharing'sFORBIDDEN, … That is precisely the read analytics /query 未做 cube 存在性校验,未注册名直达驱动当表名;且错误路径原样回显驱动 SQL(#3770 同类,另一子系统) #3867 fixed one tier up; these call sites were still doing the old thing. A 404 the route meant to return arrived as a 500, and the message got dragged through the 5xx leak sanitiser on the way out.ValidationErrorstill lost itsfields[]and its 400 — re-opening [17.0.0-rc.0] Runtime dispatcher error paths drop ValidationError.fields[] and downgrade status to 500 (rest-server maps it correctly) #3918 on exactly the routes it was filed against.security.tsis the clearest illustration: its comment says "The service throws typed errors carrying their HTTP status: PermissionDeniedError → 403, SuggestionNotFoundError → 404, SuggestionStateError → 409" — and then readstatusCode, which none of them set.The fix
Every such catch now calls
deps.errorFromThrown(e, …), so both earlier fixes finally reach the routes that need them. This is a pure convergence — no new concepts, anderrorFromThrownalready understandsstatus,statusCodeand the validation shape.Deliberate per-route fallbacks are preserved, not flattened to 500:
/metasave (metadata-service branch)saveMetaItem, so "unsupported" is the honest default/metatwo-part lookup…but a validation failure on either now answers 400 with its fields instead of being swallowed by the fallback. A 501 "Not Implemented" for a bad field value was never right.
domains/keys.tsis deliberately NOT converted. It discards the underlying error on purpose — the comment says "Never surface the underlying error (could echo row contents)" — so converting it would turn a privacy guard into a leak. Its literal'Failed to create API key'is the correct answer and stays. Guarded by a note in the test file so nobody "finishes the job" later.Also added a note to the
handlePackagesRequestdocblock so new handlers route through the shared helper instead of re-deriving the status.Verification
domains/error-passthrough.test.ts, driven through the real routes: onestatus-survives case, one validation case and one 500-fallback case per converted module, plus astatusCode-still-works anti-regression and the two preserved-fallback cases.No behaviour change for errors that already carried
statusCode— that read is preserved, only widened.Generated by Claude Code