From e514f6db75cdb15e78dbe3f004e25ad2406b1bce Mon Sep 17 00:00:00 2001 From: Eric J Date: Sun, 30 Aug 2026 13:40:53 -0700 Subject: [PATCH] Stop discarding the render failure's cause, and refuse startingLakePositions (#227) A render that failed inside the worker produced no usable message anywhere, because the cause was thrown away three separate times: | step | what it did | | --- | --- | | `elevationRender.worker.ts` | `serve()` had no `try`, so it threw instead of posting | | `useElevationPreview.ts` | `onerror` took no argument and substituted a constant | | `ElevationPreviewPanel.vue` | a bare `catch` substituted "Preview failed." | Issue #341 covers the third. Fixing any one alone still leaves a useless message, so all three go together. The first one is worse than a lost string. A throw out of `onmessage` fires the worker's `error` event, which the host reads as a crashed worker: it terminates the slot and rejects every tile the slot was holding. So one bad request failed every good one beside it, and reported six words that named none of them. `serve()` now catches, settles exactly the request that failed, and leaves its siblings alone. "Preview failed." was also the string `PreviewPanel.vue` produces from the unrelated Docker preview service, which #341 records having been confused by. ## The startingLakePositions refusal A caller-supplied `startingLakePositions` is now refused as the FIRST statement of `runRenderRequest`, before the planet split. The two checks it replaces sat inside leaves of the view/planet dispatch and missed three cases between them: the Vulcanus branch returns before the Nauvis gate is evaluated, the Fulgora branch likewise - and that one is live, since `findIslands` posts `planet: "fulgora", view: "landmask"` - and `"landmask"` on Nauvis is in the outer view test but absent from the Nauvis gate's allowlist. It refuses rather than ignores because the TYPE outlives every consumer: `eval/ctx.ts` and `expressions/elevationIsland.ts` survive #227 while every module that acted on the override does not. Accepting and ignoring it would render a different map than the caller asked for and say nothing. The `NAUVIS_MAX_STARTING_POINTS` clause deliberately stays. Its fallback is the TypeScript path, so it goes with the deletion, not here - and it is safe to remove then only because `serve()` is wrapped now. ## Planted, not predicted | plant | result | | --- | --- | | `serve()` throws again instead of posting | RED - both worker error tests | | `onerror` substitutes the constant again | RED - `carries the crash's own message` | | the panel discards the cause again | RED - `shows the render's OWN message` | | the guard moves into the elevation dispatch leaf | RED - `refuses on every planet and view` | | the guard gains a `length > 0` test | RED - `refuses an EMPTY list too` | One plant corrected a note rather than the code. The guard was written `!== undefined` on the stated grounds that a truthiness test would let `[]` through; planting truthiness came back GREEN, because `[]` is truthy in JavaScript and both forms refuse it. The form that does let `[]` through is a length test, so that is what the empty-list block now guards against, and the comment says so. `!== undefined` stays, because it states the type's own distinction rather than relying on that coincidence. Two tests were strengthened rather than added. The panel's error test asserted only that an error element EXISTED, which stayed true the whole time the cause was being discarded. Closes #341. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NgrxV4zcQLSsWu4XymFMBV --- src/components/ElevationPreviewPanel.vue | 9 +- src/components/useElevationPreview.ts | 23 +++++- src/noise/preview/elevationRender.worker.ts | 23 +++++- src/noise/preview/elevationRenderRequest.ts | 84 ++++++++++++++----- test/elevationPreviewPanel.spec.ts | 29 ++++++- test/renderWorkerEngine.spec.ts | 51 +++++++++++- test/wasmElevationRenderParity.spec.ts | 92 +++++++++++++++------ test/workerHost.spec.ts | 28 +++++++ 8 files changed, 283 insertions(+), 56 deletions(-) diff --git a/src/components/ElevationPreviewPanel.vue b/src/components/ElevationPreviewPanel.vue index 64ddec19..a8e9efdc 100644 --- a/src/components/ElevationPreviewPanel.vue +++ b/src/components/ElevationPreviewPanel.vue @@ -248,8 +248,13 @@ async function generate() { // blit) - that is the latency a user feels, and what tiling had to move. elapsedMs.value = Math.round(performance.now() - startedAt); } - } catch { - error.value = "Preview failed."; + } catch (e) { + // The real cause, not a constant. A bare `catch` here discarded every + // render failure's message - issue #341 - and produced the same + // "Preview failed." that `PreviewPanel.vue` produces from the unrelated + // Docker preview service, which is what made the two hard to tell apart. + // Same idiom as `IslandFinderPanel.vue`. + error.value = e instanceof Error ? e.message : "Preview failed."; } finally { loading.value = false; } diff --git a/src/components/useElevationPreview.ts b/src/components/useElevationPreview.ts index 7153fffa..18b5b593 100644 --- a/src/components/useElevationPreview.ts +++ b/src/components/useElevationPreview.ts @@ -128,6 +128,25 @@ export function createWorkerHost( } } + /** + * The cause out of a worker `error` event. + * + * This used to take no argument at all and substitute the constant below, so + * a worker that died of a bad import, a syntax error or an out-of-memory + * reported the same six words as one that died of anything else. `ErrorEvent` + * carries `message`; a worker whose script failed to LOAD reports an empty + * one, and a test's fake carries whatever it likes - both fall back to the + * bare constant rather than to an empty string, which would read as no error + * at all. + */ + function workerErrorMessage(e: unknown): string { + const raw = + typeof e === "object" && e !== null && "message" in e + ? String((e as { message: unknown }).message) + : ""; + return raw === "" ? "Elevation render worker error" : `Elevation render worker error: ${raw}`; + } + function dropWorker(slot: number) { const w = workers[slot]; workers[slot] = null; @@ -157,9 +176,9 @@ export function createWorkerHost( }; // A worker crash would otherwise leave its tiles' promises pending forever. // Every tile this slot holds fails, not just the newest. - w.onerror = () => { + w.onerror = (e: unknown) => { dropWorker(slot); - rejectSlot(slot, "Elevation render worker error"); + rejectSlot(slot, workerErrorMessage(e)); }; workers[slot] = w; return w; diff --git a/src/noise/preview/elevationRender.worker.ts b/src/noise/preview/elevationRender.worker.ts index 8df0be6c..4bfd88e5 100644 --- a/src/noise/preview/elevationRender.worker.ts +++ b/src/noise/preview/elevationRender.worker.ts @@ -82,8 +82,27 @@ function serve(req: ElevationRenderRequest): void { self.postMessage(message); return; } - const result = runRenderRequest(req, engine); - self.postMessage(result, [result.buffer]); + // A render that throws must be REPORTED, not allowed to escape. + // + // Letting it escape `onmessage` fires the worker's `error` event, and the + // host treats that as a crashed worker: it terminates the slot and rejects + // every tile the slot holds. So one bad request - a view the module refuses, + // a spawn list over the ABI cap, a `startingLakePositions` override - failed + // every OTHER render in flight beside it, and did so with a constant string + // that named none of those causes. + // + // Catching it here settles exactly the request that failed, with the reason, + // and leaves the worker and its siblings alone. + try { + const result = runRenderRequest(req, engine); + self.postMessage(result, [result.buffer]); + } catch (err) { + const message: RenderErrorMessage = { + id: req.id, + error: err instanceof Error ? err.message : String(err), + }; + self.postMessage(message); + } } self.onmessage = (e: MessageEvent) => { diff --git a/src/noise/preview/elevationRenderRequest.ts b/src/noise/preview/elevationRenderRequest.ts index f48978e6..bba076ea 100644 --- a/src/noise/preview/elevationRenderRequest.ts +++ b/src/noise/preview/elevationRenderRequest.ts @@ -55,7 +55,15 @@ export interface ElevationRenderRequest { * Vulcanus terrain colors rather than a Nauvis field composited on top. */ planet?: Planet; - /** Omitted => the game's real lake positions are computed inside the render. */ + /** + * **Refused.** Kept on the interface only because `eval/ctx.ts` and + * `expressions/elevationIsland.ts` still carry the field, so the type outlives + * every module that acted on it. Passing one - including `[]`, which used to + * mean "far-field only" - throws + * `STARTING_LAKE_POSITIONS_UNSUPPORTED`. The lake positions are derived inside + * the render from the seed and the starting positions, which is the game's own + * rule. + */ startingLakePositions?: Point[]; /** * Climate controls (Task 12b) - consumed only when `view: "terrain"`; the @@ -402,13 +410,14 @@ function renderFulgoraThroughWasm( * **`"elevation"` is ported as of #227**, as three `view` codes rather than * one, because the common prefix has no `mapType` field. See the gate at the * tail of `runRenderRequest`, which still keeps two cases on the TypeScript - * path: a caller-supplied `startingLakePositions`, and a non-Nauvis `planet`. + * path: a spawn list over the ABI cap, and a non-Nauvis `planet`. * - * **A caller-supplied `startingLakePositions` also stays on the TypeScript - * path**, for the same reason a moved spawn does. The module derives the lake - * list from the seed and the origin spawn - the game's own rule, and what the - * TypeScript does when the caller passes nothing - so an explicit list would be - * a wrong answer rather than a slow one. The app never sets it; only tests do. + * **A caller-supplied `startingLakePositions` is refused outright** rather + * than routed anywhere - see `STARTING_LAKE_POSITIONS_UNSUPPORTED` and the + * guard at the top of `runRenderRequest`. The module derives the lake list from + * the seed and the origin spawn, which is the game's own rule, so an explicit + * list was always a wrong answer rather than a slow one; once the TypeScript + * arm goes there is nothing left that could honour it. The app never set it. * * **`waterLevel` is sent and deliberately ignored by the module** - issue #326. * `renderTerrain.ts` resolves every tile at `waterLevel = 0` however the slider @@ -492,6 +501,16 @@ function renderNauvisThroughWasm( return { id: req.id, buffer: owned.buffer, width: req.width, height: req.height }; } +/** + * What a caller-supplied `startingLakePositions` is refused with. + * + * Exported so specs assert the exact string rather than a substring of whatever + * the `Error` happened to say. + */ +export const STARTING_LAKE_POSITIONS_UNSUPPORTED = + "startingLakePositions is not supported: the render derives the lake list from " + + "the seed and the starting positions, which is the game's own rule"; + /** * The view this request actually renders, which is not always the one it asks * for. @@ -551,6 +570,33 @@ export function runRenderRequest( req: ElevationRenderRequest, engine?: EngineExports, ): ElevationRenderResult { + // FIRST, before the planet split, and deliberately so. + // + // The two checks this replaces sat inside leaves of the view/planet dispatch + // and between them missed three cases: the Vulcanus branch returns before the + // Nauvis gate is ever evaluated, the Fulgora branch likewise - and that one is + // reachable, since `findIslands` posts `planet: "fulgora", view: "landmask"` - + // and `"landmask"` on Nauvis is in the outer view test but absent from the + // Nauvis gate's allowlist. A guard that runs before any of that has no leaves + // to miss. + // + // `!== undefined` rather than a truthiness test because it states the type's + // own distinction, `Point[] | undefined`, instead of relying on a coincidence. + // The coincidence is real and was measured: `[]` is TRUTHY in JavaScript, so + // a truthiness test refuses an empty list too and the two forms agree on + // every value this field can legally hold. The form to avoid is a length + // test - `!== undefined && length > 0` - which would wave `[]` through, and + // `[]` is a meaningful value rather than an absent one: `elevationLakes.ts` + // documented "Pass `[]` for the old far-field-only behavior". + // + // An error rather than a silent no-op because the TYPE outlives every + // consumer: `eval/ctx.ts` and `expressions/elevationIsland.ts` survive #227 + // while every module that acted on the override does not. Accepting and + // ignoring it would render a different planet than the caller asked for and + // say nothing. + if (req.startingLakePositions !== undefined) { + throw new Error(STARTING_LAKE_POSITIONS_UNSUPPORTED); + } const planet = req.planet ?? "nauvis"; const view = servedView(planet, req.view); let image: ImageData; @@ -727,10 +773,10 @@ export function runRenderRequest( // reaches the same distance terms. An over-long list is refused by the // writer rather than silently shortened, so it cannot arrive here wrong. // - // `startingLakePositions` still does force it: the module derives the lake - // list from the seed and the spawn, which is the game's own rule and what - // the TypeScript does when the caller passes nothing, so an explicit list - // would be a wrong answer rather than a slow one. The app never sets it. + // `startingLakePositions` no longer appears here: it is refused outright at + // the top of this function, so by the time the gate is evaluated there is + // nothing left to test. The spawn-list cap still forces the TypeScript path, + // and it is the last thing that does. if ( engine !== undefined && (view === "terrain" || @@ -740,7 +786,6 @@ export function runRenderRequest( view === "cliffs" || view === "resources" || view === "all") && - req.startingLakePositions === undefined && req.startingPositions.length <= NAUVIS_MAX_STARTING_POINTS ) { return renderNauvisThroughWasm(req, engine, view); @@ -867,13 +912,13 @@ export function runRenderRequest( // no layout change. `mapType` picks the code because the common prefix has // no `mapType` field; see `VIEW` in `src/noise/wasm/request.ts`. // - // **`startingLakePositions` still forces the TypeScript path.** That is a - // CORRECTNESS carve-out, not a speed one: the module derives the lake list - // from the seed and the spawn, which is the game's own rule and what the - // TypeScript does when the caller passes nothing, so an explicit list would - // be a wrong answer rather than a slow one. It is an ABI carve-out too - - // the request is a fixed-size struct with no room for a variable-length - // array. The app never sets it; only a test does, and not on this path. + // **`startingLakePositions` is gone from this gate**, because it is refused + // at the top of the function now. It was a CORRECTNESS carve-out rather + // than a speed one - the module derives the lake list from the seed and the + // spawn, which is the game's own rule - and an ABI one besides, the request + // being a fixed-size struct with no room for a variable-length array. With + // the TypeScript arm going there is no path that could honour it, so the + // honest answer is to refuse rather than to ignore. // // **A non-Nauvis `planet` also stays on TypeScript.** `mapType` spans the // Nauvis family only, and the branch below ignores `planet` outright, so @@ -882,7 +927,6 @@ export function runRenderRequest( if ( engine !== undefined && planet === "nauvis" && - req.startingLakePositions === undefined && req.startingPositions.length <= NAUVIS_MAX_STARTING_POINTS ) { return renderNauvisThroughWasm( diff --git a/test/elevationPreviewPanel.spec.ts b/test/elevationPreviewPanel.spec.ts index 7d293bb4..48e2a8d9 100644 --- a/test/elevationPreviewPanel.spec.ts +++ b/test/elevationPreviewPanel.spec.ts @@ -394,18 +394,41 @@ describe("ElevationPreviewPanel", () => { expect(putImageData).toHaveBeenCalledTimes(4); }); - it("shows an error when the render rejects", async () => { + it("shows the render's OWN message when it rejects, not a constant", async () => { + // Issue #341. This asserted only that the element existed, which stayed + // true while a bare `catch` replaced every cause with "Preview failed." - + // the same string `PreviewPanel.vue` produces from the unrelated Docker + // preview service, which is what made the two impossible to tell apart. stubCanvas(); const renderer: ElevationRenderer = { render: vi.fn(async () => { - throw new Error("boom"); + throw new Error("engine.wasm speaks ABI v3, this bundle writes v4"); + }), + dispose: vi.fn(), + }; + const w = setup("lakes", renderer); + await w.find('[data-test="generate"]').trigger("click"); + await flushPromises(); + const shown = w.find('[data-test="preview-error"]'); + expect(shown.exists()).toBe(true); + expect(shown.text()).toContain("engine.wasm speaks ABI v3, this bundle writes v4"); + }); + + it("falls back to a readable line when the render throws a non-Error", async () => { + // `e instanceof Error` is false for a thrown string or a rejected promise + // carrying one, and `String(e)` on some of those reads as "[object Object]". + // The constant is the floor rather than the default. + stubCanvas(); + const renderer: ElevationRenderer = { + render: vi.fn(async () => { + throw "not an Error"; }), dispose: vi.fn(), }; const w = setup("lakes", renderer); await w.find('[data-test="generate"]').trigger("click"); await flushPromises(); - expect(w.find('[data-test="preview-error"]').exists()).toBe(true); + expect(w.find('[data-test="preview-error"]').text()).toContain("Preview failed."); }); it("hides the view toggles when dev mode is off", () => { diff --git a/test/renderWorkerEngine.spec.ts b/test/renderWorkerEngine.spec.ts index 031e8b12..99a6fd07 100644 --- a/test/renderWorkerEngine.spec.ts +++ b/test/renderWorkerEngine.spec.ts @@ -2,7 +2,10 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; -import type { ElevationRenderRequest } from "../src/noise/preview/elevationRenderRequest"; +import { + STARTING_LAKE_POSITIONS_UNSUPPORTED, + type ElevationRenderRequest, +} from "../src/noise/preview/elevationRenderRequest"; import { surfaceSeedForPlanet } from "../src/model/planetSurfaceSeed"; /** @@ -190,4 +193,50 @@ describe("the render worker's engine handshake", () => { expect(message.id).toBe(7); expect(message.error).toContain("render engine failed to instantiate"); }, 120000); + + it("posts a render failure rather than throwing out of onmessage", async () => { + // The case that used to take the whole slot down. `serve()` had no `try`, so + // a throw escaped `onmessage`, fired the worker's `error` event, and the + // host terminated the worker and rejected EVERY tile it was holding - one + // bad request failing every good one beside it, under a constant string + // that named none of them. + // + // A `startingLakePositions` override is simply the cheapest way to make + // `runRenderRequest` throw. The claim here is about the worker, not about + // that particular refusal. + const w = await loadWorker(); + const module = await WebAssembly.compile(readFileSync(wasmPath)); + w.onmessage?.({ data: { kind: "engine", module } }); + + expect(() => { + w.onmessage?.({ data: { ...request(11), startingLakePositions: [{ x: 300, y: 300 }] } }); + }, "a failing render must not escape onmessage").not.toThrow(); + + expect(posted).toHaveLength(1); + const message = replyAt(0); + expect(message.id, "the error must carry the id, or the host strands it").toBe(11); + expect(message.error, "the REAL cause, not a constant").toBe( + STARTING_LAKE_POSITIONS_UNSUPPORTED, + ); + expect(posted[0]?.transfer, "no buffer, so nothing to transfer").toBeUndefined(); + }, 120000); + + it("keeps serving its other requests after one of them fails", async () => { + // The whole point of catching rather than crashing: the sibling tiles in + // the same slot are unaffected. Before the wrap the second request never + // got an answer at all, because the worker was gone by the time it arrived. + const w = await loadWorker(); + const module = await WebAssembly.compile(readFileSync(wasmPath)); + w.onmessage?.({ data: { kind: "engine", module } }); + + w.onmessage?.({ data: { ...request(12), startingLakePositions: [] } }); + w.onmessage?.({ data: request(13) }); + + expect(posted).toHaveLength(2); + expect(replyAt(0).id).toBe(12); + expect(replyAt(0).error).toBe(STARTING_LAKE_POSITIONS_UNSUPPORTED); + expect(replyAt(1).id).toBe(13); + expect(replyAt(1).error, "the good request must be a result, not an error").toBeUndefined(); + expect(pixelsOf(1).length).toBe(12 * 9 * 4); + }, 120000); }); diff --git a/test/wasmElevationRenderParity.spec.ts b/test/wasmElevationRenderParity.spec.ts index edd024f2..0307bf1c 100644 --- a/test/wasmElevationRenderParity.spec.ts +++ b/test/wasmElevationRenderParity.spec.ts @@ -15,6 +15,7 @@ import { import { compileEngine, instantiateEngine, renderThroughWasm } from "../src/noise/wasm/engine"; import { runRenderRequest, + STARTING_LAKE_POSITIONS_UNSUPPORTED, type ElevationRenderRequest, } from "../src/noise/preview/elevationRenderRequest"; import { LAND_RGBA, WATER_RGBA } from "../src/noise/preview/palette"; @@ -356,43 +357,82 @@ describe("the elevation levers move both paths together", () => { }, 300000); }); -describe("a caller-supplied startingLakePositions stays on the TypeScript path", () => { +describe("a caller-supplied startingLakePositions is refused", () => { /** - * The one carve-out in the gate, and it is a CORRECTNESS one rather than a + * This block asserted the opposite until #227: that an explicit lake list + * forced the TypeScript path, which was a CORRECTNESS carve-out rather than a * speed one. The module derives the lake list from the seed and the spawn - - * the game's own rule, and what the TypeScript does when the caller passes - * nothing - so an explicit list is a different answer, not a slower one. The - * request is also a fixed-size struct with no room for a variable-length - * array, so there is nowhere to put one without an ABI change. + * the game's own rule - so an explicit list was a different answer, not a + * slower one, and the request is a fixed-size struct with nowhere to put a + * variable-length array besides. * - * Planted rather than read: the gate is asserted by giving the engine a - * request it would answer DIFFERENTLY, and requiring the TypeScript answer. + * With the TypeScript arm going there is no path left that could honour it, + * so the choice is between refusing and silently ignoring. Ignoring would + * render a different map than the caller asked for and say nothing, so it + * refuses. + * + * **Both arms are asserted.** With an engine present the request would + * otherwise route through WASM ignoring the list, which is precisely the + * wrong-answer case the carve-out existed to prevent; without one it would + * have taken the TypeScript path. Neither may quietly succeed. */ const w = WINDOWS[0]; const explicit = [{ x: 300, y: 300 }]; - // **Deliberately NOT frozen**, for the reason the Nauvis spec's ABI-cap test - // is not: both arms here are the TypeScript renderer - that is the whole - // claim - so a frozen row would capture an image the engine can never - // reproduce, and would fail the moment the carve-out goes. - // - // Unlike the spawn cap, this carve-out is documented as a CORRECTNESS one - // rather than a speed one, so #227 has to decide what a caller-supplied - // `startingLakePositions` means once there is no TypeScript path to fall back - // to. The app never sets it; only a test does. - - it("renders the TypeScript answer even with a live engine", async () => { + it("throws with the engine present and with it absent", async () => { const e = await engine(); const req = request(w, "lakes", { startingLakePositions: explicit }); - expect(Array.from(pixels(req, e)), "engine path taken").toEqual(Array.from(pixels(req))); + expect(() => pixels(req, e), "with an engine").toThrow(STARTING_LAKE_POSITIONS_UNSUPPORTED); + expect(() => pixels(req), "without an engine").toThrow(STARTING_LAKE_POSITIONS_UNSUPPORTED); }, 300000); - it("and that list actually changes the render, so the check above is not vacuous", async () => { - const withList = request(w, "lakes", { startingLakePositions: explicit }); - const derived = request(w, "lakes"); - expect(Array.from(pixels(withList)), "explicit lakes changed nothing").not.toEqual( - Array.from(pixels(derived)), - ); + it("refuses an EMPTY list too, which used to mean far-field only", () => { + // `elevationLakes.ts` documented "Pass `[]` for the old far-field-only + // behavior", so `[]` is a meaningful value rather than an absent one - + // rendering the derived lakes for a caller who asked for none is a wrong + // answer, not a default. + // + // Planted: this does NOT discriminate `!== undefined` from a truthiness + // test, because `[]` is truthy and both forms refuse it. What it catches is + // a LENGTH test, `!== undefined && length > 0`, which is the plausible + // mistake - it reads like a tidy-up and silently restores the old + // behaviour for the one value that used to select it. + const req = request(w, "lakes", { startingLakePositions: [] }); + expect(() => pixels(req)).toThrow(STARTING_LAKE_POSITIONS_UNSUPPORTED); + }); + + it("refuses on every planet and view, not only the one the old checks reached", async () => { + // The two checks this guard replaced sat inside leaves of the view/planet + // dispatch, and between them missed three cases: the Vulcanus branch + // returns before the Nauvis gate is ever evaluated, the Fulgora branch + // likewise - and that one is live, since `findIslands` posts + // `planet: "fulgora", view: "landmask"` - and `"landmask"` on Nauvis is in + // the outer view test but absent from the Nauvis gate's allowlist. + // + // Every other assertion in this describe is Nauvis `lakes`, so a guard put + // back into a dispatch leaf would satisfy all of them and still let these + // three through. This is the block that grades the guard's PLACEMENT. + const e = await engine(); + const holes = [ + { label: "vulcanus terrain", planet: "vulcanus", view: "terrain" }, + { label: "fulgora landmask", planet: "fulgora", view: "landmask" }, + { label: "nauvis landmask", planet: "nauvis", view: "landmask" }, + ] as const; + for (const h of holes) { + const req = request(w, "lakes", { + planet: h.planet, + view: h.view, + startingLakePositions: explicit, + }); + expect(() => pixels(req, e), h.label).toThrow(STARTING_LAKE_POSITIONS_UNSUPPORTED); + } + }, 300000); + + it("still renders when the field is simply absent", async () => { + // The guard runs before everything, so a bug in it would take out every + // render rather than only the overriding ones. + const e = await engine(); + expect(pixels(request(w, "lakes"), e).length).toBe(w.width * w.height * 4); }, 300000); }); diff --git a/test/workerHost.spec.ts b/test/workerHost.spec.ts index 7cdcdf61..ec42c2ca 100644 --- a/test/workerHost.spec.ts +++ b/test/workerHost.spec.ts @@ -89,4 +89,32 @@ describe("createWorkerHost", () => { host.dispose(); expect(made[0]!.terminate).toHaveBeenCalled(); }); + + it("carries the crash's own message into every tile the slot held", async () => { + // `onerror` was declared with NO argument and substituted a constant, so a + // worker that died of a bad import and one that died of anything else + // reported the same six words. Failing every tile in the slot was already + // right - a crashed worker cannot answer any of them - but each one now + // says what happened. + const w = fakeWorker(); + const host = createWorkerHost(() => w, 1); + const first = host.execute(req(1), 0); + const second = host.execute(req(2), 0); + // Synchronously, before the fake's queued replies run. + w.onerror?.({ message: "Cannot find module './engine.wasm'" }); + await expect(first).rejects.toThrow("Cannot find module './engine.wasm'"); + await expect(second).rejects.toThrow("Cannot find module './engine.wasm'"); + host.dispose(); + }); + + it("keeps the bare label when the crash carries no message", async () => { + // A worker whose SCRIPT failed to load reports an empty `message`. An empty + // string would read as no error at all, so the constant stays as the floor. + const w = fakeWorker(); + const host = createWorkerHost(() => w, 1); + const only = host.execute(req(1), 0); + w.onerror?.({ message: "" }); + await expect(only).rejects.toThrow("Elevation render worker error"); + host.dispose(); + }); });