fix(console): a flow or action that failed under HTTP 200 stops reporting success (#2958) - #2995
Merged
Merged
Conversation
…ting success (#2958) Business failures come back HTTP 200 with the failure on the INNER envelope (objectstack#3913), and a failed flow launch carries neither `status` nor `screen`. Three call sites read that response by hand and two read it incompletely, so a failed run was indistinguishable from a completed one: no dialog, a green "completed successfully" toast, a refresh, and the real error swallowed. - `utils/flowResponse.ts` (new) — the flow trigger/resume rule, once: transport failure / flow failure / screen pause / terminal success, with `error` guaranteed to be a STRING. Failure is classified before `paused`, which is safe because the engine always stamps `success: true` alongside `status: 'paused'` (service-automation engine.ts). - Both flow-launch handlers (`useConsoleActionRuntime`, `RecordDetailView`) now go through it, so a launch that failed returns `success: false` and no longer refreshes. RecordDetailView additionally stops passing `json.error` through raw — the nested `{code, message}` shape reaches `toast.error()` as a React child and crashes the page (React #31). - `FlowRunner`'s resume shares the same interpreter, keeping its retryable (transport) vs terminal (flow) close behaviour and gaining the coercion. - `apiHandler` classifies an HTTP-200 `success: false` body as a failure before refreshing on it. The `script` handler was already fixed via `interpretActionResponse`; this is the same lesson on the automation route, with a matching ratchet test so the rule cannot be hand-rolled back into three copies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
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
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 #2958.
Business failures come back HTTP 200 with the failure on the inner envelope (objectstack#3913), and a failed flow launch carries neither
statusnorscreen. Three call sites read that response by hand and two read it incompletely, so a failed run was indistinguishable from a completed one: no dialog, a green "completed successfully" toast, a refresh, and the real error swallowed.What changed
utils/flowResponse.ts(new) — the flow trigger/resume rule, once. Classifies transport failure / flow failure / screen pause / terminal success, and guaranteeserroris a string.Failure is classified before
paused, which is safe: the engine always stampssuccess: truealongsidestatus: 'paused'(verified in frameworkservice-automation/src/engine.ts— both paused returns). So a wizard can never be swallowed by the new check.useConsoleActionRuntime.flowHandlersuccess: false, no refreshRecordDetailView.flowHandlerjson.errorpassthroughFlowRunner.resumeWithuseConsoleActionRuntime.apiHandlerres.okonly → refreshed on a rejected callsuccess: falseclassified before the refreshThe
scripthandler needed no change — it was already fixed viainterpretActionResponse. This is that same lesson on the automation route.Defect 3 (the raw error value)
RecordDetailViewpassedjson.errorstraight through andFlowRunnerpasseddata.error. The nested{code, message}shape reachestoast.error()as a React child and crashes the page (React #31) — the trapactionErrorDetailexists for. Both now route through it.Tests
utils/__tests__/flowResponse.test.ts— 18 cases over every response shape, incl. the object-shaped-error coercion and "paused is not a failure".useConsoleActionRuntime.test.tsx— 6 handler-level cases pinning the reported repro (failed launch,errorMessagepreference, wizard still opens, nested-error coercion,apiHandler200-failure, and that a payload merely containing asuccesskey is left alone).flow-envelope.ratchet.test.ts(new) — sibling of the existing/actionsratchet, so the rule cannot be hand-rolled back into three copies. Scoped totrigger/resumecallers, since most automation callers just read the registry.Verified the tests actually catch the bug: neutralising the inner-failure branch fails 9 tests, including the pre-existing
FlowRunnerterminal-failure test — confirming all three sites now share one rule. Neutralising theapiHandlercheck fails its test.unit+dom+dom-heavyprojects run clean (830 app-shell dom tests, 217 dom-heavy). Typecheck error count is unchanged frommain(7 pre-existing@object-ui/typesstale-dist errors, none in touched files); 0 lint errors.🤖 Generated with Claude Code