Skip to content

Commit db618be

Browse files
fix(runtime): deliver fire-and-forget notify() when the script throws
runScript drained un-awaited async verbs only after fn(scope) returned, so a script that threw skipped the drain entirely. The CLI catches that throw and calls process.exit(1), which kills the in-flight POST — dropping exactly the notification the operator most wants: notify("build failed"); nope(); printed the "notify() -> build failed" line but never delivered it. Drain in a finally block instead. Promise.allSettled never rejects, so the script's own error still propagates unchanged.
1 parent 8e183ad commit db618be

2 files changed

Lines changed: 134 additions & 3 deletions

File tree

src/runtime.mjs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,18 @@ export async function runScript(source, opts = {}) {
144144
const scope = makeScope(registry, ctx, control, pending);
145145
const body = `with (__scope__) {\n${stripShebang(source)}\n}`;
146146
const fn = new AsyncFunction("__scope__", body);
147-
await fn(scope);
148-
// Let any un-awaited async verbs (fire-and-forget notify) finish delivering.
149-
if (pending.length) await Promise.allSettled(pending);
147+
try {
148+
await fn(scope);
149+
} finally {
150+
// Let any un-awaited async verbs (fire-and-forget notify) finish delivering.
151+
//
152+
// Drained in `finally`, not after the call: a script that throws has already
153+
// queued its notify(), and the CLI's catch calls process.exit(1) — which kills
154+
// the in-flight POST. Draining only on the success path silently dropped the
155+
// failure ping, i.e. exactly the notification the operator most wants.
156+
// allSettled never rejects, so this cannot mask the script's own error.
157+
if (pending.length) await Promise.allSettled(pending);
158+
}
150159

151160
return { iterations: control.ticks, stopped: control.stopped };
152161
}
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import assert from "node:assert/strict";
2+
import test from "node:test";
3+
4+
import { runScript } from "../src/runtime.mjs";
5+
import { createRegistry } from "../src/registry.mjs";
6+
7+
// A vocabulary whose async verb only records once it has actually settled, the
8+
// way notify()'s POST only counts once it reaches the app. `delivered` therefore
9+
// distinguishes "queued" from "really delivered" — the whole point of the drain.
10+
//
11+
// The verb resolves on a macrotask (setTimeout), not a microtask: an un-drained
12+
// promise chained purely off microtasks can still finish by accident before the
13+
// caller observes anything, which would make these tests pass for the wrong
14+
// reason. A timer cannot.
15+
function slowVocab() {
16+
const delivered = [];
17+
const registry = createRegistry([
18+
{
19+
name: "notify",
20+
summary: "fire-and-forget async verb",
21+
run: (_ctx, msg) =>
22+
new Promise((resolve) => setTimeout(() => { delivered.push(msg); resolve({ ok: true }); }, 5)),
23+
},
24+
{
25+
name: "reject",
26+
summary: "fire-and-forget async verb that fails",
27+
run: () => new Promise((_res, rej) => setTimeout(() => rej(new Error("delivery refused")), 5)),
28+
},
29+
{ name: "sync", summary: "blocking verb", run: () => "done" },
30+
]);
31+
return { delivered, registry };
32+
}
33+
34+
test("a fire-and-forget notify still delivers when the script throws", async () => {
35+
const { delivered, registry } = slowVocab();
36+
await assert.rejects(
37+
async () => runScript(`notify("build failed"); nope();`, { commands: registry }),
38+
/nope is not defined/
39+
);
40+
// The failure ping is the one the operator most wants; it must not be dropped.
41+
assert.deepEqual(delivered, ["build failed"]);
42+
});
43+
44+
test("a fire-and-forget notify still delivers when a verb throws", async () => {
45+
const { delivered, registry } = slowVocab();
46+
registry.register({ name: "explode", run: () => { throw new Error("verb blew up"); } });
47+
await assert.rejects(
48+
async () => runScript(`notify("half way"); explode();`, { commands: registry }),
49+
/verb blew up/
50+
);
51+
assert.deepEqual(delivered, ["half way"]);
52+
});
53+
54+
test("every queued notify delivers when the script throws, not just the first", async () => {
55+
const { delivered, registry } = slowVocab();
56+
await assert.rejects(
57+
async () => runScript(`notify("one"); notify("two"); notify("three"); nope();`, { commands: registry }),
58+
/nope is not defined/
59+
);
60+
assert.deepEqual(delivered.sort(), ["one", "three", "two"]);
61+
});
62+
63+
test("notifies queued inside a while (alive) loop deliver when the script throws", async () => {
64+
const { delivered, registry } = slowVocab();
65+
await assert.rejects(
66+
async () => runScript(`while (alive) { notify("tick"); } nope();`, { commands: registry, max: 3 }),
67+
/nope is not defined/
68+
);
69+
assert.deepEqual(delivered, ["tick", "tick", "tick"]);
70+
});
71+
72+
test("the script's own error survives the drain unchanged", async () => {
73+
const { registry } = slowVocab();
74+
// allSettled never rejects, so draining cannot replace or mask the real cause.
75+
await assert.rejects(
76+
async () => runScript(`notify("x"); throw new Error("the original cause");`, { commands: registry }),
77+
/the original cause/
78+
);
79+
});
80+
81+
test("a REJECTING fire-and-forget verb does not mask the script's error", async () => {
82+
const { registry } = slowVocab();
83+
await assert.rejects(
84+
async () => runScript(`reject(); nope();`, { commands: registry }),
85+
/nope is not defined/
86+
);
87+
});
88+
89+
// ---- controls: the pre-existing contract must be unchanged both ways ----
90+
91+
test("control: notify still delivers on the success path", async () => {
92+
const { delivered, registry } = slowVocab();
93+
const r = await runScript(`notify("all good");`, { commands: registry });
94+
assert.deepEqual(delivered, ["all good"]);
95+
assert.equal(r.stopped, false);
96+
});
97+
98+
test("control: runScript still resolves { iterations, stopped } after draining", async () => {
99+
const { delivered, registry } = slowVocab();
100+
registry.register({ name: "halt", summary: "stop the loop", run: (ctx) => ctx.stop() });
101+
const r = await runScript(`while (alive) { notify("x"); halt(); }`, { commands: registry, max: 5 });
102+
assert.equal(r.iterations, 1);
103+
assert.equal(r.stopped, true);
104+
assert.deepEqual(delivered, ["x"]);
105+
});
106+
107+
test("control: a throwing script with nothing queued still rejects", async () => {
108+
const { delivered, registry } = slowVocab();
109+
await assert.rejects(
110+
async () => runScript(`sync(); nope();`, { commands: registry }),
111+
/nope is not defined/
112+
);
113+
assert.deepEqual(delivered, []);
114+
});
115+
116+
test("control: an awaited notify is unaffected and still returns its value", async () => {
117+
const { delivered, registry } = slowVocab();
118+
await runScript(`const r = await notify("awaited"); if (!r.ok) throw new Error("lost the return value");`, {
119+
commands: registry,
120+
});
121+
assert.deepEqual(delivered, ["awaited"]);
122+
});

0 commit comments

Comments
 (0)