Skip to content

Commit 7d3b1b7

Browse files
os-steveclaude
andauthored
fix(service-automation): a no-catch try_catch keeps the record of the writes its try region already made (#14224)
* test(service-automation): reproduce #14184 — a no-catch try_catch discards its try-region step record Measured on origin/main e621291: 11 of 16 pins RED. The run log keeps zero try-region steps and the #4354 summary reports acted: 0 over 2 writes that really landed. The 5 GREEN pins are the controls that must not move: failure text, NODE_FAILURE step code, $error contents, the honest-zero reverse control, and the contained (with-catch) path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs * fix(service-automation): a no-catch try_catch keeps its try-region step record (#14184) Attach childSteps to try_catch's no-catch failing return, and fold result.childSteps in the engine's if (!result.success) branch — the returned-failure half of #13803's throw-path fold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs * docs(service-automation): point the failing-catch boundary pin at #14222 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3e9c0d8 commit 7d3b1b7

4 files changed

Lines changed: 567 additions & 6 deletions

File tree

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
---
4+
5+
fix(service-automation): a `try_catch` with no `catch` region keeps the record of the writes its try region already made (#14184)
6+
7+
The returned-failure half of the engine's `childSteps` asymmetry. #13803 closed
8+
the **throw** half — a dying `loop` brands its thrown error with the body steps
9+
it completed and the engine's `catch` arm folds them into the run log — and
10+
deliberately left the `if (!result.success)` branch alone, because at that
11+
moment no executor returned `childSteps` on a failing result and a fold for
12+
zero producers is speculative.
13+
14+
`try_catch` is the producer that makes it real. It does not throw: it catches
15+
the try region's failure and RETURNS it, and on that return it withheld its
16+
`childSteps` on purpose — correct while the engine spliced them only after a
17+
successful result, and stale the moment the failing branch learned to fold. So
18+
for a `try_catch` with **no** `catch` region, the try region's completed steps
19+
were recorded nowhere: the run log kept no step for them, and the #4354 summary
20+
folded over that log reported `acted: 0` for a region that had genuinely
21+
written rows.
22+
23+
That is wrong in the one direction that causes harm. `acted: 0` on a failed run
24+
reads as "nothing happened, safe to re-run", and for a non-idempotent region
25+
(notifications, counters, external calls) that misread invites double-execution.
26+
27+
Two halves, mirroring #13803:
28+
29+
- `try_catch`'s no-`catch` failing return now carries `childSteps`.
30+
- The engine folds `result.childSteps` in its `if (!result.success)` branch, in
31+
the same position the throw arm and the success path use — right behind the
32+
container's own step, ahead of any `fault` handler's steps.
33+
34+
**A record fix only; accept/reject is untouched and measured so.** `try_catch`
35+
still returns failure with the same error text, still produces a `NODE_FAILURE`
36+
step with the same message, still writes the same `$error`, still routes down
37+
the same `fault` edge, and the run still ends `failed`. Those four were green
38+
before this change and are green after it. The contained (with-`catch`) path,
39+
the all-succeeding path and the failing-`catch` path are unchanged, and a try
40+
region that fails before writing anything still reports `acted: 0` — there 0 is
41+
the honest answer.
42+
43+
Every folded step carries a `parentNodeId`, so the ADR-0044 runaway guard, which
44+
counts only top-level visits, does not see them. Nesting was measured for
45+
double-folding: a container's sink already absorbs an inner container's steps,
46+
so each step object still reaches the run log exactly once.

packages/services/service-automation/src/builtin/try-catch-node.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,32 @@ export function registerTryCatchNode(engine: AutomationEngine, ctx: PluginContex
182182
}
183183

184184
// No catch handler — surface the failure to the flow's fault edge / error
185-
// handling. No `childSteps` here on purpose: the engine splices them only
186-
// on a SUCCESSFUL node result, so attaching them to a failing one would
187-
// be dead weight. That path is not the gap #7546 closes either — an
188-
// unhandled failure already terminates the run `failed` with both
189-
// run-level and step-level errors, which is loud by construction.
190-
return { success: false, error: `try_catch '${node.id}': try region failed — ${lastError}` };
185+
// handling.
186+
//
187+
// #14184 — and the try region's steps ride out WITH it. They used to be
188+
// withheld here on purpose, because the engine spliced `childSteps` only
189+
// on a SUCCESSFUL node result and attaching them to a failing one really
190+
// was dead weight. #13803 taught the engine's THROW arm to fold a dying
191+
// container's carried steps, and this card teaches its returned-failure
192+
// arm the same, so the sink now has a reader on both channels.
193+
//
194+
// Withholding them was the #13803 defect one construct over. The try
195+
// region's nodes may have written rows before one of them failed; the run
196+
// log kept no step for any of them, so the #4354 summary folded over that
197+
// log reported `acted: 0` for a region that had genuinely written. `acted:
198+
// 0` on a failed run reads as "nothing happened, safe to re-run", which
199+
// for a non-idempotent region invites double-execution.
200+
//
201+
// Purely additive to the RECORD: this return already reported failure,
202+
// already produced a `NODE_FAILURE` step, already set `$error` and was
203+
// already routable by a `fault` edge. Adding `childSteps` moves none of
204+
// that — unlike the rejected "make `loop` swallow its throw and return"
205+
// shape (see `partial-steps.ts`), which would have changed all four.
206+
return {
207+
success: false,
208+
error: `try_catch '${node.id}': try region failed — ${lastError}`,
209+
childSteps: failedAttemptSteps,
210+
};
191211
},
192212
});
193213

0 commit comments

Comments
 (0)