Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions packages/runtime/src/sandbox/body-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ export function hookBodyRunnerFactory(
name: hook.name,
object: typeof (hook as any).object === 'string' ? (hook as any).object : undefined,
},
timeoutMs: (body as any).timeoutMs ?? 250,
// When the body declares no timeout, leave opts.timeoutMs unset so the
// runner's own configurable hook default applies (QuickJSScriptRunner
// defaults to 250ms). Hardcoding the fallback here would make that
// constructor option dead for hooks.
timeoutMs: (body as any).timeoutMs,
});
applyMutationsToInput(engineCtx, result);
} catch (err: any) {
Expand Down Expand Up @@ -123,7 +127,9 @@ export function actionBodyRunnerFactory(
});
const result = await runner.run(body, sandboxCtx, {
origin: { kind: 'action', name: action.name, object: action.object },
timeoutMs: (body as any).timeoutMs ?? action.timeoutMs ?? 5000,
// As with hooks above: no declared timeout → let the runner's
// configurable action default (5000ms) apply.
timeoutMs: (body as any).timeoutMs ?? action.timeoutMs,
});
return result.value;
} catch (err: any) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ describe('#1867 nested cross-object write — REAL SqlDriver (better-sqlite3, on
engine.registerDriver(driver, true);
await engine.init();
for (const o of [EXPENSE_REPORT, EXPENSE_LINE]) engine.registry.registerObject(o as any);
engine.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner(), { ql: engine, appId: 'expense' }));
// Generous hook budget — the subject is the nested-write path on a real
// driver, not the 250ms default (see nested-write.integration.test.ts).
engine.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }), { ql: engine, appId: 'expense' }));
bindHooksToEngine(engine, [ROLLUP_HOOK as any], { packageId: 'expense' });
return engine;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,13 @@ describe('#1867 nested cross-object write from a hook (real engine + sandbox)',
engine.registerDriver(driver, true);
await engine.init();
for (const o of [parent, child]) engine.registry.registerObject(o as any);
// Generous hook budget: the subject here is nested-write correctness, not
// the 250ms default. Each hook invocation compiles a fresh WASM module and
// the nested parent hook compiles another inside the child's budget — on a
// loaded CI machine that overhead alone blew 250ms ("hook
// 'rollup_parent_total' exceeded timeout of 250ms").
engine.setDefaultBodyRunner(
hookBodyRunnerFactory(new QuickJSScriptRunner(), { ql: engine, appId: 'test' }),
hookBodyRunnerFactory(new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }), { ql: engine, appId: 'test' }),
);
});

Expand Down
27 changes: 18 additions & 9 deletions packages/runtime/src/sandbox/quickjs-runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@ import { describe, it, expect } from 'vitest';
import { QuickJSScriptRunner, SandboxError } from './quickjs-runner.js';
import type { ScriptContext, ScriptRunOptions } from './script-runner.js';

const runner = new QuickJSScriptRunner();
// Generous hook budget: these tests exercise sandbox *behaviour*, not the stock
// 250ms hook budget. Every invocation compiles a fresh WASM module, and nested
// hooks compile another one inside the parent's budget — on a loaded CI machine
// that fixed cost alone can blow 250ms and flake (e.g. "hook 'lvl4' exceeded
// timeout of 250ms"). Tests that ARE about the default budget use
// `defaultRunner` below.
const runner = new QuickJSScriptRunner({ hookTimeoutMs: 10_000 });
const hookOpts: ScriptRunOptions = { origin: { kind: 'hook', name: 't' } };
const actionOpts: ScriptRunOptions = { origin: { kind: 'action', name: 't' } };

Expand Down Expand Up @@ -364,6 +370,10 @@ describe('QuickJSScriptRunner — nested sandbox re-entrancy (#1867)', () => {
// the 250ms hook default and killed mid-flight.
// ---------------------------------------------------------------------------
describe('QuickJSScriptRunner — timeout resolution honors body.timeoutMs (#1867)', () => {
// Stock engine defaults (250ms hooks) — these tests assert the default budget
// itself, so they must NOT use the generous shared `runner` above.
const defaultRunner = new QuickJSScriptRunner();

it('honors a hook body timeoutMs above the 250ms hook default', async () => {
// Host call settles at ~600ms — comfortably past the old 250ms hook cap but
// within the body's declared 5000ms budget. Must resolve, not time out.
Expand All @@ -375,7 +385,7 @@ describe('QuickJSScriptRunner — timeout resolution honors body.timeoutMs (#186
},
}),
};
const r = await runner.runScript(
const r = await defaultRunner.runScript(
{
language: 'js',
source: "return await ctx.api.object('x').update({});",
Expand All @@ -390,27 +400,26 @@ describe('QuickJSScriptRunner — timeout resolution honors body.timeoutMs (#186

it('still applies the 250ms hook default when the body declares no timeoutMs', async () => {
const api = { object: () => ({ update: () => new Promise<never>(() => {}) }) };
const start = Date.now();
await expect(
runner.runScript(
defaultRunner.runScript(
{ language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'] },
ctx({ api }),
hookOpts,
),
).rejects.toThrow(/timeout/i);
// Fired near the 250ms default, not some larger value.
expect(Date.now() - start).toBeLessThan(2000);
// The error message embeds the effective budget — asserting on it proves
// the 250ms default applied without a flaky wall-clock measurement.
).rejects.toThrow(/timeout of 250ms/);
}, 10000);

it('lets a hook body LOWER its timeout below the default', async () => {
const api = { object: () => ({ update: () => new Promise<never>(() => {}) }) };
await expect(
runner.runScript(
defaultRunner.runScript(
{ language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'], timeoutMs: 50 },
ctx({ api }),
hookOpts,
),
).rejects.toThrow(/timeout/i);
).rejects.toThrow(/timeout of 50ms/);
}, 10000);
});

Expand Down