Uh oh!
There was an error while loading. Please reload this page.
fix(runtime): answer 404, not 500, when toggling an unknown automation flow (#7535) - #7558
Merged
Merged
Conversation
…n flow (#7535) `POST /api/v1/automation/:name/toggle` against a flow name the registry does not hold answered **500 `INTERNAL_ERROR`**. It now answers **404 `RESOURCE_NOT_FOUND`**, naming the flow it could not find. The class was the defect, not the wording. Clients and retry layers branch on it: 5xx means "the server broke, try again", 4xx means "your request was wrong, don't". A typo'd flow name presented as a transient server fault, so any retry-on-5xx caller re-sent — repeatedly — a request that can never succeed. Cause: `toggleFlow` on an unregistered name throws a plain `Error("Flow '<name>' not found")` (service-automation's engine). It carries no `.status`, so both dispatcher error exits — `errorFromThrown` and the plugin's `errorResponseBase` — fell back to their 500 default. Fixed at the domain handler rather than in a generic catch, on purpose. Which HTTP status a plain domain error means is the serving boundary's decision (the rule `validation-failure.ts` already states for `ValidationError` → 400), and teaching a shared catch to recognise one engine's message string would make every domain's not-found depend on that prose. The handler instead runs the **same existence probe `GET /automation/:name` already uses**, so the two routes cannot disagree about which flows exist. This brings the missing-flow arm up to the standard the endpoint's *body* arm already met (#3899), where `{"enable": false}` — one letter off — is a located 400 naming the offending key rather than a silent enable. The refusals compose in that order: a malformed body is still rejected without the registry being consulted at all. Unchanged: toggling a real flow in either direction, the documented bodyless enable, the strict `{ enabled?: boolean }` validation, and any `IAutomationService` implementation that omits the optional `getFlow` — it cannot be asked whether a flow exists, so its toggle proceeds exactly as before rather than this inventing a 404. Six tests in `automation-toggle-unknown-flow.test.ts`, each proved to fail against a mutated source (status 404→500; guard deleted; message stops naming the flow; `!existing` inverted; optional-method guard dropped; probe moved ahead of the body checks). Full `@objectstack/runtime` suite green: 123 files, 1982 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015heAKHUUrVM5GGFgvhf317
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 20 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also reference the affected code. These are read-only:
|
os-help
marked this pull request as ready for review
August 11, 2026 07:47
Uh oh!
There was an error while loading. Please reload this page.
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.
Fixes#7535
POST /api/v1/automation/:name/toggleagainst a flow name the registry does not hold answered 500INTERNAL_ERROR. It now answers 404RESOURCE_NOT_FOUND, naming the flow it could not find.The class was the defect, not the wording. Clients and retry layers branch on it: 5xx means "the server broke, try again", 4xx means "your request was wrong, don't". A typo'd flow name presented as a transient server fault, so any retry-on-5xx caller re-sent — repeatedly — a request that can never succeed.
Cause
toggleFlowon an unregistered name throws a plainError("Flow '<name>' not found")(packages/services/service-automation/src/engine.ts:2411). It carries no.status, so both dispatcher error exits —HttpDispatcher.errorFromThrownanddispatcher-plugin'serrorResponseBase— fell back to their 500 default. Nothing was catching the not-found case and calling it a not-found.Fix
packages/runtime/src/domains/automation.ts— the toggle branch runs an existence probe before touching the service, and returns404in the house envelope naming the flow.Three deliberate choices worth reviewing:
validation-failure.tsalready states forValidationError→ 400. Teaching a shared catch to recognise one engine's message string would make every domain's not-found depend on that prose. (This also keeps the PR clear ofhttp-dispatcher.ts, which The runtime dispatcher serialises a PermissionDeniedError'sdetailsto the client, sopositions/permissionSetsreach the browser on the/datatransport #7450 is editing.)GET /automation/:namealready uses, rather than a second notion of existence, so the two routes cannot disagree about which flows exist.The endpoint's body arm already met this bar —
{"enable": false}, one letter off, is a located 400 naming the offending key rather than a silent enable (#3899). The missing-flow arm now answers in kind: a 404 that names the flow, not a bare status change.Unchanged: toggling a real flow in either direction, the documented bodyless enable, the strict
{ enabled?: boolean }validation, and anyIAutomationServiceimplementation that omits the optionalgetFlow?— it cannot be asked whether a flow exists, so its toggle proceeds exactly as before rather than this inventing a 404.On
error.codeLeft to derive from the status (
RESOURCE_NOT_FOUND), matching the siblingGET /:nameon this domain. Two alternatives were considered and rejected:StandardErrorCode's 404 bucket has no flow-specific member (RESOURCE_/OBJECT_/RECORD_/FIELD_/ENDPOINT_NOT_FOUND), and adding one is not this PR's business.packages/specdoes declareAutomationApiErrorCode.flow_not_found— but a repo-wide search finds no producer and no consumer for that enum outside its own shape test and the type-alias pin, so nothing serves it on the wire today. Adopting it here would (a) start wiring up a declared-but-unenforced vocabulary as a rider on a P3 bug fix, and (b) maketoggleandGET /:nameanswer different codes for the identical condition — the exact drift the shared existence probe above was chosen to avoid. Worth its own enforce-or-remove card; flagged, not actioned.Tests
New:
packages/runtime/src/domains/automation-toggle-unknown-flow.test.ts(6 tests). The fake models the real engine —getFlowresolvesnullfor an unknown name andtoggleFlowthrows — so a handler that never checks cannot pass.Every test proved to fail against a mutated source:
deps.error(…, 404)→500if (!existing)→if (existing)typeof getFlow === 'function'guard droppedgetFlowBaseline restored green after each.
Verification
pnpm --filter @objectstack/runtime typecheck— cleaneslint --no-inline-configon both changed files — clean@objectstack/runtimesuite — 123 files, 1982 tests, all passing, including the neighbouringautomation-body-validation.test.ts(请求体从不与声明它的 schema 对照(#3877 的请求侧对偶):7 个 schema 定义了从未启用,而 API 目录已宣称生效 #3899) unchangedChangeset included (
@objectstack/runtimepatch). Nocontent/docs/releases/**edits.Docs drift check: the 20 flagged pages are package-wide
@objectstack/runtimereferences, none of which document this endpoint. The only page that mentions/toggleat all iscontent/docs/references/api/automation-api.mdx, which is auto-generated and lists no error statuses. No doc asserts the old 500 behaviour. Conversely,docs/qa/platform-checklist/areas/automation.json:982already specified the negative probe as "expect a not-found error, no state change" — the checklist was right and the implementation was out of step; this change makes reality match it.