From ca881be3e6cb4c1f23742cdac6e7dac23ecf6bcd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 03:39:15 +0000 Subject: [PATCH] fix(service-automation): a try_catch whose catch region fails carries the refusal it met The catch-region failure return re-words the handler's failure into a new returned failure, and the engine's carried-refusal brand on the handler's throw did not cross it, so a refusal met inside a catch region reached the flow's caller as text only. Spread readCarriedRefusal(catchErr) onto that return, the same carriage the no-catch return already uses. The carried refusal is the catch region's own, never the try region's it handled; a handler that met a fault carries neither member. The error text is unchanged. Pinned in flow-inner-refusal.test.ts through engine.execute; the inner-refusal.ts header's carrier list now names this path. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016WV4d1Np3hU7RQnA13YZcL --- ...76-try-catch-catch-region-carry-refusal.md | 12 +++ .../src/builtin/try-catch-node.ts | 8 ++ .../src/flow-inner-refusal.test.ts | 78 ++++++++++++++++++- .../service-automation/src/inner-refusal.ts | 12 ++- 4 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 .changeset/27876-try-catch-catch-region-carry-refusal.md diff --git a/.changeset/27876-try-catch-catch-region-carry-refusal.md b/.changeset/27876-try-catch-catch-region-carry-refusal.md new file mode 100644 index 00000000000..091c19e5fbc --- /dev/null +++ b/.changeset/27876-try-catch-catch-region-carry-refusal.md @@ -0,0 +1,12 @@ +--- +'@objectstack/service-automation': patch +--- + +fix(service-automation): a refusal met inside a `try_catch` catch region reaches the flow's caller as the structured refusal + +Clause-②: no +Metadata-protocol: none + +A `try_catch` whose catch region itself fails now carries the refusal that region met (a permission, a validation rule, a duplicate, an app hook's declared refusal) on the run's result as `innerRefusal` (`{ code, message }`) and the producer-marked `userMessage`, the same way a `try_catch` with no catch region already does. Before, the caller got only the node chain as text (`try_catch 'ID': catch region failed — …`), so a runner had no business message to show. + +The refusal carried is the catch region's own, never the try region's refusal that it handled. A catch region that failed with no refusal carries neither member, even when the try region was refused, and the `error` text is unchanged. diff --git a/packages/services/service-automation/src/builtin/try-catch-node.ts b/packages/services/service-automation/src/builtin/try-catch-node.ts index 5186063a00b..09436a8a70c 100644 --- a/packages/services/service-automation/src/builtin/try-catch-node.ts +++ b/packages/services/service-automation/src/builtin/try-catch-node.ts @@ -343,9 +343,17 @@ export function registerTryCatchNode(engine: AutomationEngine, ctx: PluginContex // with this error text, already produced a `NODE_FAILURE` step and // was already routable by a `fault` edge. No engine change — the // #14184 fold on `if (!result.success)` is already the reader. + // + // #27876 — and the CATCH region's refusal rides out the way the + // no-catch return's does (#27743, below): this return re-words the + // handler's failure, the brand on the handler's throw does not cross + // that, so it is spread here (`inner-refusal.ts`). It is the refusal + // the HANDLER met, never the try region's it handled; `{}` when the + // handler met none. The `error` text does not move. return { success: false, error: `try_catch '${node.id}': catch region failed — ${catchMsg}`, + ...readCarriedRefusal(catchErr), childSteps: [...failedAttemptSteps, ...catchAttemptSteps], }; } diff --git a/packages/services/service-automation/src/flow-inner-refusal.test.ts b/packages/services/service-automation/src/flow-inner-refusal.test.ts index 5a261cbd0c0..6ac6a701568 100644 --- a/packages/services/service-automation/src/flow-inner-refusal.test.ts +++ b/packages/services/service-automation/src/flow-inner-refusal.test.ts @@ -28,7 +28,9 @@ * refusal carries none; (#27743) a `try_catch` with no catch region * carries its last failed attempt's refusal, and a `subflow` child that * paused and fails on resume carries it to a caller holding either the - * parent's run id or the child's (the leg a `map` item takes); + * parent's run id or the child's (the leg a `map` item takes); (#27876) a + * `try_catch` whose catch region fails carries the refusal the catch + * region met, never the try region's it handled; * 4. ⛔ `error` is byte-identical to the composition it always had — the * diagnostic channel does not move. * @@ -590,6 +592,80 @@ describe('#27743 — a `try_catch` with no catch region carries the guarded refu }); }); +describe('#27876 — a `try_catch` whose CATCH region fails carries the refusal the catch region met', () => { + /** start → guard (try_catch, try = [a], catch = [b], both `create_record`) → end. */ + function caughtFlow(): any { + const create = (id: string) => ({ + id, type: 'create_record', label: 'Create', config: { objectName: 'crm_account', fields: { name: 'Acme' } }, + }); + return { + name: 'caught', label: 'Caught', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'guard', type: 'try_catch', label: 'Guard', + config: { try: { nodes: [create('a')], edges: [] }, catch: { nodes: [create('b')], edges: [] } }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'guard' }, + { id: 'e2', source: 'guard', target: 'end' }, + ], + }; + } + + /** Each insert throws the next error in `errs` — the try region's `a` first, then the catch region's `b`. */ + function caughtEngine(...errs: unknown[]): { engine: AutomationEngine; calls: () => number } { + let calls = 0; + const data = refusingData(undefined); + data.insert = async () => { throw errs[Math.min(calls++, errs.length - 1)]; }; + const engine = new AutomationEngine(makeLogger()); + const ctx = ctxWith(data); + registerCrudNodes(engine, ctx); + registerTryCatchNode(engine, ctx); + engine.registerFlow('caught', caughtFlow()); + return { engine, calls: () => calls }; + } + + it('both regions refused by a validation rule (the measured case): the refusal reaches the run, the text keeps its chain', async () => { + const { engine, calls } = caughtEngine(validation(), validation()); + + const result = await engine.execute('caught', { userId: 'u1' }); + + expect(calls()).toBe(2); + expect(result.status).toBe('failed'); + expect(result.innerRefusal).toEqual({ code: 'VALIDATION_FAILED', message: 'Amount must be positive' }); + expect('userMessage' in result).toBe(false); + expect(result.error).toBe( + `Node 'guard' failed: try_catch 'guard': catch region failed — ` + + `Node 'b' failed: create_record(crm_account) failed: Amount must be positive`, + ); + }); + + it('the CATCH region\'s refusal is the one carried — code and userMessage — never the try region\'s it handled', async () => { + const hook = markedHook(); + const { engine } = caughtEngine(permission(), hook); + + const result = await engine.execute('caught', { userId: 'u1' }); + + expect(result.status).toBe('failed'); + expect(result.innerRefusal).toEqual({ code: 'RESOURCE_CONFLICT', message: hook.message }); + expect(result.userMessage).toBe(HOOK_USER_TEXT); + }); + + it('control: a catch region that meets a FAULT carries neither member, even though the try region met a refusal', async () => { + const { engine, calls } = caughtEngine(markedHook(), new Error('ECONNREFUSED: could not reach the store')); + + const result = await engine.execute('caught', { userId: 'u1' }); + + expect(calls()).toBe(2); + expect(result.status).toBe('failed'); + expect('innerRefusal' in result).toBe(false); + expect('userMessage' in result).toBe(false); + }); +}); + describe('#27743 — a `map` item or `subflow` child that pauses and fails on resume', () => { /** start → hold (pause) → mk (create_record) → end — the child that fails after its pause. */ const PAUSING_WRITER = 'hold_then_write'; diff --git a/packages/services/service-automation/src/inner-refusal.ts b/packages/services/service-automation/src/inner-refusal.ts index 3a3603fbd5a..6f65f80145d 100644 --- a/packages/services/service-automation/src/inner-refusal.ts +++ b/packages/services/service-automation/src/inner-refusal.ts @@ -132,6 +132,11 @@ export function refusalCarriedBy(err: unknown): CarriedRefusal { * a new returned failure too, and carries the members its LAST failed * attempt's throw carried (`readCarriedRefusal`), so a retry ladder reports * the last attempt's refusal and never an earlier one's. + * - A `try_catch` whose CATCH region itself fails (#27876) re-words the + * handler's failure the same way, and carries the members the CATCH + * region's throw carried (`readCarriedRefusal`): the refusal the handler + * met, never the try region's refusal it handled. A handler that met a + * fault carries none, even after a refused try region. * - A `subflow` child that PAUSED and then fails on resume, resumed at the * parent's run id (the delegated leg in `run-resume.ts`): the parent's * re-worded failure carries the child's result's members @@ -141,10 +146,9 @@ export function refusalCarriedBy(err: unknown): CarriedRefusal { * terminal result, which carries the members; the ancestors that leg fails * record no member, and no caller reads their result. * - * Not a carrier: a `try_catch` whose CATCH region itself fails builds its - * failure from the handler's error and carries no member; the run's result - * then carries none, which the spec's "present only when a refusal was met" - * allows and which never invents one. + * On every carrier, a failure that met no refusal carries no member, which the + * spec's "present only when a refusal was met" allows and which never invents + * one. */ export const CARRIED_REFUSAL: unique symbol = Symbol.for( 'objectstack.automation.carriedRefusal',