fix(actions): an action that CRASHED is a 500, not a 200 reporting success:false (#3913 follow-up) - #3951
Merged
Conversation
…ccess:false (#3913 follow-up) #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 — those 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. For a platform whose main extension surface is customer-authored script bodies, "customer action bodies are throwing" had no signal short of body-parsing at every hop. 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. A rejection and a 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. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011AvZj6cLX7APd7roh2eK4F
|
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 30, 2026
… single wrap (#3962) (#3969) Fixes #3962 — the platform decision classifying the 200-on-failure wire as a bug, not a contract. The 200-with-inner-envelope shape was never designed: no ADR or doc specified it, it originated as the catch block reusing deps.success(), and /actions was the only route of 12 that double-wrapped. Five defects traced back to that one extra layer. Contract now, identical to /data: - ran, returned → 200 {success:true, data: <handler return value>} (single wrap) - ran, rejected → 400, semantic code on error.code (VALIDATION_FAILED with fields[] in details; FLOW_FAILED for a rejected flow) - never dispatched → 404 / 403 / 400 / 503 (unchanged, #3930/#3951) - crashed → 500 (unchanged, #3951; name-based discriminator now selects 400 vs 500) actions-validation-envelope.test.ts pinned the 200 "so flipping it later is a conscious, documented break"; #3962 is that decision and the flipped test cites it. Integrates ADR-0110 (#3958/#3987 — name identity, undeclared refusal with no opt-out) and #3971 (semantic error.code, details.code promotion) from main. client.actions.invoke/invokeGlobal still never throw: every failure status folds into {success:false, error}, success reads the single wrap, and a narrow legacy heuristic (boolean success, no foreign keys) keeps a current SDK correct against pre-#3962 servers. Migration (raw-HTTP callers): branch on the status; on 200, data is the handler's return value directly. SDK callers need no change.
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 #3930 / #3913. Implements the one part of the 200-vs-status question I think is uncontested — see the analysis in #3913 for the wider argument, which this PR deliberately does not settle.
What #3937 argued, and what it also covered
#3937 settled that a failed action reports in the payload at HTTP 200:
That is a statement about the action rejecting — a business rule saying no. Correct, and nothing here touches it. But the same exit was also covering a third case it never argued for: a
TypeErrorin a handler, a driver blowing up, a sandbox timeout.Those 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. For a platform whose main extension surface is customer-authored script bodies, "customer action bodies are throwing" had no signal short of body-parsing at every hop.They are 500 now, through the same
errorFromThrownexit every other domain catch has used since #3925. A side benefit: a driver dump finally goes through the internal-error-leak sanitiser (#3867) instead of reaching the client verbatim inside a 200 body.Nothing #3937 put in the payload moves
Rejection and crash are told apart by the error's name — the signal
@objectstack/restalready uses on this exact distinction:new Error(msg)— a registered handler rejectingSandboxErrorwithinnerMessage— a body's deliberate throwcode/fields, or aValidationErrorby namenameat allTypeError/ReferenceError/SqliteError/ a driver's classSandboxErrorwith noinnerMessage— timeout, capability denialDeliberately the narrow direction: only what is certainly a fault moves; everything uncertain keeps the 200 it has today. In particular a plain
throw new Error('Lead already converted')from aregisterActionhandler stays a 200 payload — a naive "no sandbox marker ⇒ fault" rule would have broken it, and there is a test pinning that.One related fix in the same exit
An error carrying its own
status/statusCode— a plugin'sFORBIDDENwithstatus: 403— is now served with it rather than buried in a 200 payload. That status was the one thing the thrower was unambiguous about. RecordValidationErrors deliberately carry no.status(seevalidation-failure.ts), so #3937's cases never reach that branch.Testing
pnpm buildgreen (71 tasks).actions-validation-envelope.test.tsuntouched.domains/actions-fault-vs-rejection.test.ts(12 cases) pins both sides, because the whole risk here is over-reach — every shape fix(runtime): carry code/fields across the sandbox boundary so form actions can anchor validation errors (#3918 follow-up) #3937 put in the payload has a case asserting it stays there.Two of my own first-draft assertions were wrong and the code was right; one of them exposed the
.statusgap above, which is why that fix is in this PR.Docs
New Action Errors section in
api/error-catalog.mdxwith the full status table and the two-check pattern a rawfetchcaller needs:That contract previously lived only in test comments — four in-repo callers got it wrong, so it belongs where callers actually look.
ui/actions.mdxupdated to match.🤖 Generated with Claude Code
https://claude.ai/code/session_011AvZj6cLX7APd7roh2eK4F
Generated by Claude Code