diff --git a/apps/effectcraft-web/js/host.js b/apps/effectcraft-web/js/host.js index 486865ae..702c5d33 100644 --- a/apps/effectcraft-web/js/host.js +++ b/apps/effectcraft-web/js/host.js @@ -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); }); } diff --git a/apps/effectcraft-web/tests/workers.mjs b/apps/effectcraft-web/tests/workers.mjs index 96e3f1f0..15a77f1d 100644 --- a/apps/effectcraft-web/tests/workers.mjs +++ b/apps/effectcraft-web/tests/workers.mjs @@ -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"); diff --git a/apps/effectcraft-web/web/worker.js b/apps/effectcraft-web/web/worker.js index 7c6752e5..2d2c154b 100644 --- a/apps/effectcraft-web/web/worker.js +++ b/apps/effectcraft-web/web/worker.js @@ -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 diff --git a/docs/web.md b/docs/web.md index 104cbc2c..12d2b24d 100644 --- a/docs/web.md +++ b/docs/web.md @@ -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