Skip to content

Commit 8ed9c54

Browse files
claude[bot]claude
andauthored
lint(flows): warn on an uncontained loop body and on a try_catch with no catch, and document loop { try_catch } (#14617)
* feat(lint): warn on an uncontained loop body and on a try_catch with no catch Two authoring-time rules in the flow anti-pattern family, both warnings: `flow-loop-body-uncontained` (a `loop` body running a fallible node with no `try_catch` between the loop and it) and `flow-try-catch-without-catch` (the near-miss shape that gives zero containment while looking like containment). Documents `loop { try_catch { … } }` in the flow docs as the per-iteration containment spelling, with the measured minimal `catch` — one bare `assignment` node, `edges` and `errorVariable` omitted — and the two empty-`catch` spellings the region schema refuses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV * fix(lint): keep tracker ids out of the containment rules' runtime hints `check:doc-authoring` refuses a NEW internal issue id in string prose a runtime surface shows an author: the ids move to the adjacent comments, where the reader who can resolve them already looks. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3d9aa3e commit 8ed9c54

5 files changed

Lines changed: 780 additions & 13 deletions

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
flows: warn on a `loop` body with a fallible node and no containment, and on a `try_catch` with no `catch` (#14394)
6+
7+
Two authoring-time rules in the flow anti-pattern family, both `warning`:
8+
9+
- **`flow-loop-body-uncontained`** — a `loop` whose `body` region runs a node
10+
that can end the run (a record read/write, `http`, `notify`,
11+
`connector_action`, `script`, `subflow`, `map`, `approval`) with no
12+
`try_catch` between the loop and that node. The `loop` executor iterates with
13+
a bare `await` and has no `try`/`catch` at all, so the first failing item ends
14+
the whole run: later items are never processed, and the work already done is
15+
not even reported. The finding names the loop, the node, and the prescribed
16+
spelling.
17+
- **`flow-try-catch-without-catch`** — the near-miss, and the first target
18+
rather than an extra: `catch` is optional in the schema, and omitting it makes
19+
the container fail through, so an author who wrapped the node and stopped
20+
there gets **zero** containment and previously got no diagnostic either.
21+
Measured, the no-`catch` run and the unwrapped control produce identical
22+
output; a `retry` policy only delays that.
23+
24+
Both stay warnings under the family's severity bar: a loop deliberately allowed
25+
to stop at the first failure, and a retry-then-fail `try_catch`, are legitimate
26+
readings the rule cannot disprove.
27+
28+
`content/docs/automation/flows.mdx` documents `loop { try_catch { … } }` as the
29+
per-iteration containment spelling, with the measured minimal handler — one bare
30+
`assignment` node, `edges` and `errorVariable` omitted — and the three `catch`
31+
spellings the schema refuses (`catch` omitted gives no containment; `catch: {}`
32+
and `catch: { nodes: [] }` are rejected, the region's `nodes` being `.min(1)`).
33+
34+
No spec, engine or runtime change: the containment capability already exists and
35+
was measured working (5 of 5 iterations, items 4-5 processed, run completes).

content/docs/automation/flows.mdx

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,15 @@ Runs its `body` region once per item of a collection, binding the current item
411411
maxIterations: 500, // hard cap (clamped to the engine ceiling)
412412
body: { // single-entry/single-exit region
413413
nodes: [
414-
{ id: 'send', type: 'script', label: 'Notify', config: { /**/ } },
414+
// Per-iteration containment — see the subsection below. A body node that
415+
// can fail, wrapped in nothing, ends the WHOLE run at the first failure.
416+
{
417+
id: 'guard', type: 'try_catch', label: 'Guarded iteration',
418+
config: {
419+
try: { nodes: [{ id: 'send', type: 'script', label: 'Notify', config: { /**/ } }], edges: [] },
420+
catch: { nodes: [{ id: 'handled', type: 'assignment', label: 'Handled' }] },
421+
},
422+
},
415423
],
416424
edges: [],
417425
},
@@ -422,6 +430,77 @@ Runs its `body` region once per item of a collection, binding the current item
422430
A `loop` node with **no `body`** keeps the legacy flat-graph behavior — the
423431
container is additive.
424432

433+
#### Per-iteration containment: `loop { try_catch { … } }`
434+
435+
A `loop` body has **no error handling of its own**. The container iterates with a
436+
bare `await`, so a body node that returns `success: false` (or throws) propagates
437+
straight out of the loop and ends the run: every later item is never processed,
438+
and the work already done is not even reported. Measured on the engine — a 5-item
439+
sweep whose 3rd item fails touched 3 items, reported `acted: 0`, and finished
440+
`status: failed`.
441+
442+
The containment spelling is a `try_catch` **inside the body**, one per iteration.
443+
Measured with the same 5-item sweep: all 5 iterations run, items 4 and 5 are
444+
processed, and the run completes.
445+
446+
```typescript
447+
{
448+
id: 'each_case',
449+
type: 'loop',
450+
label: 'For each breached case',
451+
config: {
452+
collection: '{cases}',
453+
iteratorVariable: 'currentCase',
454+
body: {
455+
nodes: [
456+
{
457+
id: 'guard',
458+
type: 'try_catch',
459+
label: 'Guarded iteration',
460+
config: {
461+
try: {
462+
nodes: [
463+
{
464+
id: 'notify_owner', type: 'notify', label: 'Notify owner',
465+
config: { title: 'SLA breach', recipients: ['{currentCase.owner}'] },
466+
},
467+
],
468+
edges: [],
469+
},
470+
// The shortest handler that works: ONE bare `assignment` node with no
471+
// `config` at all. `edges` omitted; `errorVariable` omitted (it
472+
// defaults to `$error`).
473+
catch: { nodes: [{ id: 'handled', type: 'assignment', label: 'Handled' }] },
474+
},
475+
},
476+
],
477+
edges: [],
478+
},
479+
},
480+
}
481+
```
482+
483+
**A `catch` region cannot be empty.** `FlowRegionSchema.nodes` is `.min(1)`, so
484+
only the last row below is usable:
485+
486+
| `catch` spelling | result |
487+
|:---|:---|
488+
| omitted entirely | parses — and contains **nothing**: the container fails through exactly like an unwrapped node |
489+
| `catch: {}` | rejected — `catch.nodes`: expected array, received undefined |
490+
| `catch: { nodes: [] }` | rejected — `catch.nodes`: too small, expected at least 1 item |
491+
| `catch: { nodes: [ …one node… ] }` | parses, and contains |
492+
493+
Two authoring-time lint rules cover this pair (both warnings, so neither fails a
494+
build): `flow-loop-body-uncontained` names a loop body running a node that can
495+
fail with no `try_catch` between the loop and it, and
496+
`flow-try-catch-without-catch` names the near-miss — a `try_catch` whose `catch`
497+
is absent, which gives **zero** containment while looking like containment. A
498+
`retry` policy does not substitute: it re-runs the `try` region and then fails
499+
anyway.
500+
501+
Deliberately letting the sweep stop at the first failure is a legitimate choice —
502+
that is why both rules warn rather than gate.
503+
425504
### Parallel block
426505

427506
Declares N branch regions that run **concurrently** and **join implicitly** when
@@ -465,6 +544,14 @@ events).
465544
}
466545
```
467546

547+
`catch` is optional in the schema, and omitting it is the trap: with no `catch`
548+
the container **fails** when the `try` region fails, so the failure propagates
549+
exactly as if nothing had been wrapped (measured — the no-`catch` run and the
550+
unwrapped control produce identical output). `retry` only delays that. The
551+
`flow-try-catch-without-catch` lint rule names the shape at authoring time; the
552+
minimal handler is one bare `assignment` node, as shown under "Per-iteration
553+
containment" above.
554+
468555
> BPMN `parallel_gateway` / `join_gateway` / `boundary_event` remain in the
469556
> protocol as the **interop** representation and map onto these constructs on
470557
> import/export — they are not the native authoring model.

packages/lint/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,8 @@ export {
734734
FLOW_MULTIPLE_DEFAULT_EDGES,
735735
FLOW_INERT_NODE_CONDITION,
736736
FLOW_MULTI_WRITE_UNFILTERED,
737+
FLOW_LOOP_BODY_UNCONTAINED,
738+
FLOW_TRY_CATCH_WITHOUT_CATCH,
737739
} from './lint-flow-patterns.js';
738740

739741
export { lintLivenessProperties } from './lint-liveness-properties.js';

0 commit comments

Comments
 (0)