mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-14 19:27:05 -04:00
Web: a job worker that fails to start fails its job (#216)
worker.js left a rejected start (its async init) unhandled, which a page's Worker error event doesn't see: the page waited for "ready" forever and the render stayed pending until cancelled. The failed start is now re-raised where the page's onerror sees it, like the other worker failures; the job fails with the error and the worker is terminated. Test: apps/effectcraft-web/tests/workers.mjs (worker.js with a stub module whose init rejects; host.js with a worker whose start fails). Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018gnTnH3Jmk5zr4tMq1Mc9f
This commit is contained in:
4 files changed
+57
-2
No files matched your search
@@ -348,7 +348,9 @@ export function workerRun(id, json, files, base, onMessage) {
|
||||
}
|
||||
w.worker.postMessage({ type: "job", json });
|
||||
}, (e) => {
|
||||
// The worker didn't start (#216): the job fails and the worker goes.
|
||||
running.delete(id);
|
||||
w.worker.terminate();
|
||||
onMessage("error", String(e.message || e), null, null);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,19 +1,24 @@
|
||||
// Job-worker plumbing of the web app (js/host.js) under Node with a fake Worker,
|
||||
// Job-worker plumbing of the web app (js/host.js, web/worker.js) under Node with a fake Worker,
|
||||
// no browser or Wasm build needed:
|
||||
//
|
||||
// node apps/effectcraft-web/tests/workers.mjs
|
||||
//
|
||||
// - A reused worker is sent a file again when it was replaced, even by one of the same size
|
||||
// (#218), and not when it is unchanged.
|
||||
// - A worker whose start fails fails its job and is terminated, instead of the job waiting for
|
||||
// "ready" forever (#216): host.js on the worker's error, worker.js re-raising the failed start.
|
||||
// Exits non-zero on failure.
|
||||
import assert from "node:assert/strict";
|
||||
import { copyFileSync, mkdtempSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
|
||||
const here = dirname(fileURLToPath(import.meta.url));
|
||||
const tick = () => new Promise((r) => setTimeout(r, 0));
|
||||
globalThis.self = globalThis;
|
||||
|
||||
// A Worker that starts on "init" and finishes each job at once.
|
||||
// A Worker that starts (or fails to) on "init" and finishes each job at once.
|
||||
let failStart = false;
|
||||
const workers = [];
|
||||
class FakeWorker {
|
||||
@@ -53,4 +58,44 @@ assert.equal((await run(3, [["/collision.png", blue, 2]])).kind, "reply");
|
||||
assert.equal(workers.length, 1, "the worker is reused");
|
||||
assert.deepEqual(sent(workers[0]), [1, 2], "the replacement is sent once, the unchanged file not again");
|
||||
|
||||
// #216: a worker that fails to start.
|
||||
failStart = true;
|
||||
const slow = run(4, []); // the idle worker takes this one
|
||||
const failed = await run(5, []);
|
||||
await slow;
|
||||
assert.equal(failed.kind, "error");
|
||||
assert.ok(workers[1].terminated, "the failed worker is terminated");
|
||||
assert.equal(host.workerStats().running, 0);
|
||||
|
||||
// #216, worker side: a rejected start is re-raised where the page's `onerror` sees it.
|
||||
const dir = mkdtempSync(join(tmpdir(), "ec-worker-"));
|
||||
copyFileSync(join(here, "../web/worker.js"), join(dir, "worker.js"));
|
||||
writeFileSync(
|
||||
join(dir, "effectcraft_web.js"),
|
||||
`export default async function init() {}
|
||||
export function workerInit() {}
|
||||
export async function workerJobInit() { throw new Error("no device"); }
|
||||
export async function workerFrameInit() {}`,
|
||||
);
|
||||
const posted = [];
|
||||
globalThis.postMessage = (m) => posted.push(m);
|
||||
// A browser's Worker `error` event only sees thrown errors, not unhandled rejections (Node would
|
||||
// turn one into an uncaught exception unless it is listened to).
|
||||
const unhandled = [];
|
||||
process.on("unhandledRejection", (e) => unhandled.push(e));
|
||||
const raised = new Promise((resolve) => {
|
||||
process.once("uncaughtException", resolve);
|
||||
setTimeout(() => resolve(new Error("nothing was raised")), 1000);
|
||||
});
|
||||
await import(pathToFileURL(join(dir, "worker.js")));
|
||||
self.onmessage({ data: { type: "init", base: "http://localhost/" } });
|
||||
const err = await raised;
|
||||
await tick();
|
||||
// (So a failed assertion below fails the run.)
|
||||
process.removeAllListeners("unhandledRejection");
|
||||
process.removeAllListeners("uncaughtException");
|
||||
assert.deepEqual(unhandled, [], "the failed start is handled, not left as an unhandled rejection");
|
||||
assert.equal(err.message, "no device");
|
||||
assert.ok(!posted.some((m) => m.type === "ready"), "no ready after a failed start");
|
||||
|
||||
console.log("workers: ok");
|
||||
@@ -30,6 +30,9 @@ self.onmessage = (e) => {
|
||||
else await ec.workerJobInit(!!m.gpu);
|
||||
self.postMessage({ type: "ready" });
|
||||
})();
|
||||
// A failed start is re-raised outside the promise so the page's `onerror` sees it: its job
|
||||
// fails instead of waiting for "ready" forever (#216).
|
||||
ready.catch((err) => setTimeout(() => { throw err; }));
|
||||
return;
|
||||
}
|
||||
// A failure (a panic) is re-raised outside the queue so the page's `onerror` sees it, as
|
||||
|
||||
@@ -413,6 +413,11 @@ cargo xtask web --serve 8765 &
|
||||
node apps/effectcraft-web/tests/smoke.mjs --url http://127.0.0.1:8765/ --out target/web/smoke
|
||||
```
|
||||
|
||||
`apps/effectcraft-web/tests/workers.mjs` checks the job-worker plumbing (`js/host.js`,
|
||||
`web/worker.js`) under Node with a fake `Worker`, without a build: a replaced file is sent to a
|
||||
reused worker again even at the same size, and a worker that fails to start fails its job and is
|
||||
terminated (`node apps/effectcraft-web/tests/workers.mjs`).
|
||||
|
||||
It also checks the M13.10 paths: viewer frames rendered in frame workers while scrubbing with the
|
||||
CPU renderer (project synced as diffs, event-loop gaps under 400 ms), a `wait: true` render that
|
||||
replies when done while the page keeps running, and the Media Browser over browser storage; and
|
||||
|
||||
Reference in new issue
Block a user