diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4932d7f..fed5b14 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,14 +37,19 @@ jobs: # different dependency tree than the one committed. - run: npm ci - # Four TypeScript projects, checked by four different things. These two have no build of - # their own, so nothing else would ever compile them. + # Five TypeScript projects, checked by four different things. These three have no build of + # their own that checks them, so nothing else would ever compile them. The worker is bundled by + # the build, but esbuild only strips its types, and `tsconfig.app.json` leaves it out, since + # its lib is `webworker` rather than `dom`. - name: Typecheck the ETL run: npm run etl:typecheck - name: Typecheck the end-to-end tests run: npm run e2e:typecheck + - name: Typecheck the routing worker + run: npm run worker:typecheck + # `tsconfig.spec.json` is compiled here, `tsconfig.app.json` by the build below. - name: Unit tests run: npm test -- --no-watch diff --git a/angular.json b/angular.json index 6cf8740..a7f9ddc 100644 --- a/angular.json +++ b/angular.json @@ -37,8 +37,7 @@ ], "styles": [ "src/styles.css" - ], - "webWorkerTsConfig": "tsconfig.worker.json" + ] }, "configurations": { "production": { diff --git a/package.json b/package.json index c138c64..6f6cc1d 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,8 @@ "etl": "tsx tools/etl/build.ts", "etl:typecheck": "tsc -p tools/etl/tsconfig.json --noEmit", "e2e": "playwright test", - "e2e:typecheck": "tsc -p e2e/tsconfig.json --noEmit" + "e2e:typecheck": "tsc -p e2e/tsconfig.json --noEmit", + "worker:typecheck": "tsc -p tsconfig.worker.json --noEmit" }, "private": true, "packageManager": "npm@11.12.1", diff --git a/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts b/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts index b0a7048..e3e7b66 100644 --- a/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts +++ b/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts @@ -294,6 +294,23 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => { expect(component.routePending()).toBe(false); }); + it('releases the routes panel when a route cannot be worked out, so it can be tried again', async () => { + const component = fixture.componentInstance as unknown as { + routing: { route(): Promise; links(): Promise; dispose(): void }; + routePending(): boolean; + onRouteRequested(request: { fromId: number; toId: number; rangePc: number }): void; + }; + const logged = vi.spyOn(console, 'error').mockImplementation(() => undefined); + component.routing = { route: () => Promise.reject(new Error('worker gone')), links: () => Promise.resolve(new Float32Array(0)), dispose: () => undefined }; + + component.onRouteRequested({ fromId: SUN.id, toId: PROXIMA.id, rangePc: 2 }); + await flushAsync(); + + expect(component.routePending()).toBe(false); + expect(logged).toHaveBeenCalled(); + logged.mockRestore(); + }); + it('asks for no more label candidates once the last label it will show is placed', () => { // Near the Sun a label candidate past the fifteenth can sit at the far end of the catalogue's // brightness order, so asking for one more than is used can cost a walk of the whole order. diff --git a/src/app/features/galaxy-system/galaxy-system-scene.component.ts b/src/app/features/galaxy-system/galaxy-system-scene.component.ts index b04513c..6eadbb9 100644 --- a/src/app/features/galaxy-system/galaxy-system-scene.component.ts +++ b/src/app/features/galaxy-system/galaxy-system-scene.component.ts @@ -1424,18 +1424,29 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { } const request = ++this.routeRequest; this.routePending.set(true); - void this.routing.route(fromId, toId, rangePc, ROUTE_RANGE_CEILING_PC).then(({ route, neededRangePc }) => { - if (request !== this.routeRequest) { - return; + void this.routing.route(fromId, toId, rangePc, ROUTE_RANGE_CEILING_PC).then( + ({ route, neededRangePc }) => { + if (request !== this.routeRequest) { + return; + } + this.routePending.set(false); + this.routeResult.set({ + stars: route ? route.stars.map((id) => ({ id, name: this.starsById.get(id)?.name ?? `Star ${id}` })) : [], + totalPc: route?.totalPc ?? 0, + neededRangePc + }); + this.jumpLinks?.setRoute(route?.stars ?? [], (id) => this.starsById.get(id)); + }, + (error: unknown) => { + // A request replaced by a newer one is settled this way too; only the latest matters. + if (request !== this.routeRequest) { + return; + } + // Released rather than left saying "Plotting…" with the button held, so it can be tried again. + this.routePending.set(false); + console.error('Route could not be plotted.', error); } - this.routePending.set(false); - this.routeResult.set({ - stars: route ? route.stars.map((id) => ({ id, name: this.starsById.get(id)?.name ?? `Star ${id}` })) : [], - totalPc: route?.totalPc ?? 0, - neededRangePc - }); - this.jumpLinks?.setRoute(route?.stars ?? [], (id) => this.starsById.get(id)); - }); + ); } /** @@ -1460,11 +1471,20 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { return; } this.drawnJumpRangePc = rangePc; - void this.routing.links(rangePc).then((segments) => { - if (this.drawnJumpRangePc === rangePc) { - this.jumpLinks?.setSegments(segments); + void this.routing.links(rangePc).then( + (segments) => { + if (this.drawnJumpRangePc === rangePc) { + this.jumpLinks?.setSegments(segments); + } + }, + () => { + // Replaced by a newer range, or failed. Either way this range is not drawn, and must not be + // remembered as if it were, or asking for it again would be skipped. + if (this.drawnJumpRangePc === rangePc) { + this.drawnJumpRangePc = null; + } } - }); + ); } /** A pinned body wins over a hovered one, so the card does not change under the pointer. */ diff --git a/src/app/features/galaxy-system/routing-client.spec.ts b/src/app/features/galaxy-system/routing-client.spec.ts index 1154298..3e5ea14 100644 --- a/src/app/features/galaxy-system/routing-client.spec.ts +++ b/src/app/features/galaxy-system/routing-client.spec.ts @@ -1,9 +1,10 @@ import { describe, expect, it } from 'vitest'; import { jumpLinkSegments, routeBetween } from '../../shared/astro/jump-links'; +import { RoutingRequest, RoutingResponse } from '../../shared/astro/routing'; import { StarNeighbourhood } from '../../shared/astro/star-neighbourhood'; import { StarRecord } from '../../shared/models/star.model'; -import { RoutingClient } from './routing-client'; +import { RoutingClient, SupersededRequest } from './routing-client'; const STARS: StarRecord[] = Array.from({ length: 6 }, (_, i) => ({ id: 100 + i, @@ -16,11 +17,51 @@ const STARS: StarRecord[] = Array.from({ length: 6 }, (_, i) => ({ colorIndex: 0.6 })); const POSITIONS = Float32Array.from(STARS.flatMap((star) => [star.x, star.y, star.z])); +const index = new StarNeighbourhood(STARS); + +/** Flushes settled promises and their handlers. */ +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)); + +/** A worker that records what it is sent and answers only when told to. */ +class FakeWorker { + readonly sent: Array = []; + private readonly listeners: Record void>> = {}; + terminated = false; + + postMessage(message: RoutingRequest | { kind: 'catalogue' }): void { + this.sent.push(message); + } + + addEventListener(type: string, listener: (event: { data?: unknown }) => void): void { + (this.listeners[type] ??= []).push(listener); + } + + terminate(): void { + this.terminated = true; + } + + /** The requests sent so far, catalogue aside. */ + get requests(): RoutingRequest[] { + return this.sent.filter((message): message is RoutingRequest => message.kind !== 'catalogue'); + } + + answer(response: RoutingResponse): void { + for (const listener of this.listeners['message'] ?? []) listener({ data: response }); + } + + fail(): void { + for (const listener of this.listeners['error'] ?? []) listener({}); + } +} + +function clientWithFake(): { client: RoutingClient; worker: FakeWorker } { + const worker = new FakeWorker(); + const client = new RoutingClient(STARS, POSITIONS, index, () => worker as unknown as Worker); + return { client, worker }; +} // The unit tests' DOM has no Worker, which is exactly the case the client answers in place. describe('RoutingClient without a worker', () => { - const index = new StarNeighbourhood(STARS); - it('has no Worker to use here, so the in-place answers are what is being tested', () => { expect(typeof Worker).toBe('undefined'); }); @@ -49,3 +90,87 @@ describe('RoutingClient without a worker', () => { client.dispose(); }); }); + +describe('RoutingClient with a worker', () => { + it('sends the catalogue first, then one request at a time', () => { + const { client, worker } = clientWithFake(); + + void client.links(8); + void client.links(3); + + expect(worker.sent[0].kind).toBe('catalogue'); + expect(worker.requests).toHaveLength(1); + client.dispose(); + }); + + // A graph at 8 pc is seconds of work the worker cannot drop once started. Every pause on the + // range slider used to queue another, and a route asked for after them waited behind them all. + it('replaces a waiting graph with the newer one before it is ever built, and sends a route ahead of it', async () => { + const { client, worker } = clientWithFake(); + const first = client.links(5); + const superseded = client.links(6).catch((error: unknown) => error); + const latest = client.links(8); + const route = client.route(100, 104, 1.5, 8); + + const building = worker.requests[0]; + worker.answer({ kind: 'links', requestId: building.requestId, segments: new Float32Array(6) }); + await flush(); + + expect(await superseded).toBeInstanceOf(SupersededRequest); + expect(worker.requests.map((request) => request.kind)).toEqual(['links', 'route']); + await expect(first).resolves.toHaveLength(6); + + const routeRequest = worker.requests[1]; + worker.answer({ kind: 'route', requestId: routeRequest.requestId, route: null, neededRangePc: 4 }); + await expect(route).resolves.toEqual({ route: null, neededRangePc: 4 }); + await flush(); + + expect(worker.requests.map((request) => (request.kind === 'links' ? request.rangePc : request.kind))).toEqual([5, 'route', 8]); + const lastGraph = worker.requests[2]; + worker.answer({ kind: 'links', requestId: lastGraph.requestId, segments: new Float32Array(12) }); + await expect(latest).resolves.toHaveLength(12); + client.dispose(); + }); + + it('shares the answer to a question already on its way rather than asking it twice', async () => { + const { client, worker } = clientWithFake(); + const once = client.links(7.5); + const again = client.links(7.5); + + expect(worker.requests).toHaveLength(1); + worker.answer({ kind: 'links', requestId: worker.requests[0].requestId, segments: new Float32Array(6) }); + + expect(await again).toBe(await once); + expect(worker.requests).toHaveLength(1); + client.dispose(); + }); + + it('rejects a request the worker failed on, and goes on to the next', async () => { + const { client, worker } = clientWithFake(); + const failing = client.route(100, 104, 1.5, 8).catch((error: unknown) => error); + const next = client.links(3); + + worker.answer({ kind: 'failed', requestId: worker.requests[0].requestId, message: 'out of memory' }); + + expect(((await failing) as Error).message).toBe('out of memory'); + await flush(); + expect(worker.requests.map((request) => request.kind)).toEqual(['route', 'links']); + worker.answer({ kind: 'links', requestId: worker.requests[1].requestId, segments: new Float32Array(0) }); + await expect(next).resolves.toHaveLength(0); + client.dispose(); + }); + + it('answers in place what a worker that failed to load left outstanding, and everything after', async () => { + const { client, worker } = clientWithFake(); + const route = client.route(100, 104, 1.5, 8); + const graph = client.links(1.5); + + worker.fail(); + + await expect(route).resolves.toEqual({ route: routeBetween(index, 100, 104, 1.5), neededRangePc: null }); + expect(Array.from(await graph)).toEqual(Array.from(jumpLinkSegments(index, 1.5))); + await expect(client.route(100, 105, 1.5, 8)).resolves.toMatchObject({ route: null }); + expect(worker.terminated).toBe(true); + client.dispose(); + }); +}); diff --git a/src/app/features/galaxy-system/routing-client.ts b/src/app/features/galaxy-system/routing-client.ts index e1cfaba..2e1b398 100644 --- a/src/app/features/galaxy-system/routing-client.ts +++ b/src/app/features/galaxy-system/routing-client.ts @@ -8,27 +8,75 @@ export interface RouteAnswer { readonly neededRangePc: number | null; } -type Pending = (response: RoutingResponse) => void; +/** A request dropped before it was sent, because a newer one of the same kind replaced it. */ +export class SupersededRequest extends Error { + constructor() { + super('Superseded by a newer request'); + } +} + +/** A request made and not yet answered: what was asked, and the promise whoever asked is holding. */ +interface Outstanding { + readonly request: RoutingRequest; + /** The question without its id, so the same question asked twice can be recognised. */ + readonly question: string; + readonly promise: Promise; + readonly resolve: (response: RoutingResponse) => void; + readonly reject: (error: Error) => void; +} + +function outstanding(request: RoutingRequest): Outstanding { + let resolve!: (response: RoutingResponse) => void; + let reject!: (error: Error) => void; + const promise = new Promise((onResolve, onReject) => { + resolve = onResolve; + reject = onReject; + }); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { requestId, ...question } = request; + return { request, question: JSON.stringify(question), promise, resolve, reject }; +} + +/** The routing worker, where this environment has one. */ +function startRoutingWorker(): Worker | undefined { + return typeof Worker === 'undefined' ? undefined : new Worker(new URL('../../shared/astro/routing.worker', import.meta.url), { type: 'module' }); +} /** * Asks the route questions of a worker holding its own copy of the catalogue, and hands back - * promises. Where there is no `Worker` — the unit tests' DOM has none — the same answers are - * worked out in place, from the index the scene already holds. + * promises. + * + * The worker answers one request at a time and cannot drop one it has started: a jump-link graph + * at 8 pc is seconds of work. So requests are held here and sent one by one, and while one is out, + * only the latest of each kind waits behind it — a newer graph replaces an older one before it is + * ever built, and the older promise is rejected with {@link SupersededRequest}. Routes go ahead of + * graphs, being quick and asked for by a click. The same question asked again while it is still + * outstanding shares the answer rather than being worked out twice. + * + * Where there is no worker — the unit tests' DOM has none, and a worker can fail to load or crash — + * the same answers are worked out in place, from the index the scene already holds. */ export class RoutingClient { - private readonly worker?: Worker; - private readonly pending = new Map(); + private worker?: Worker; + private inFlight?: Outstanding; + private readonly waiting: Partial> = {}; private nextRequestId = 0; - constructor(stars: readonly StarRecord[], positions: Float32Array, private readonly localIndex: StarNeighbourhood) { - if (typeof Worker === 'undefined') { + constructor( + stars: readonly StarRecord[], + positions: Float32Array, + private readonly localIndex: StarNeighbourhood, + startWorker: () => Worker | undefined = startRoutingWorker + ) { + this.worker = startWorker(); + if (!this.worker) { return; } - this.worker = new Worker(new URL('../../shared/astro/routing.worker', import.meta.url), { type: 'module' }); - this.worker.addEventListener('message', ({ data }: MessageEvent) => { - this.pending.get(data.requestId)?.(data); - this.pending.delete(data.requestId); - }); + this.worker.addEventListener('message', ({ data }: MessageEvent) => this.settle(data)); + // A worker that fails to load, or dies, answers nothing further: everything outstanding, and + // everything asked from here on, is worked out in place instead of waiting for good. + this.worker.addEventListener('error', () => this.abandonWorker()); + this.worker.addEventListener('messageerror', () => this.abandonWorker()); // Copies, since the scene goes on using its own; transferred, so the copy is sent and not cloned again. const ids = Int32Array.from(stars, (star) => star.id); const copy = positions.slice(); @@ -50,16 +98,70 @@ export class RoutingClient { dispose(): void { this.worker?.terminate(); - this.pending.clear(); + this.worker = undefined; + this.inFlight = undefined; + delete this.waiting.route; + delete this.waiting.links; } private ask(request: RoutingRequest): Promise { if (!this.worker) { - return Promise.resolve(answerRouting(this.localIndex, request)); + return new Promise((resolve) => resolve(answerRouting(this.localIndex, request))); + } + const asked = outstanding(request); + const same = [this.inFlight, this.waiting[request.kind]].find((other) => other?.question === asked.question); + if (same) { + return same.promise; + } + this.waiting[request.kind]?.reject(new SupersededRequest()); + this.waiting[request.kind] = asked; + this.sendNext(); + return asked.promise; + } + + private sendNext(): void { + if (this.inFlight || !this.worker) { + return; + } + const next = this.waiting.route ?? this.waiting.links; + if (!next) { + return; + } + delete this.waiting[next.request.kind]; + this.inFlight = next; + this.worker.postMessage(next.request); + } + + private settle(response: RoutingResponse): void { + const answered = this.inFlight; + if (!answered || answered.request.requestId !== response.requestId) { + return; + } + this.inFlight = undefined; + if (response.kind === 'failed') { + answered.reject(new Error(response.message)); + } else { + answered.resolve(response); + } + this.sendNext(); + } + + private abandonWorker(): void { + this.worker?.terminate(); + this.worker = undefined; + const stranded = [this.inFlight, this.waiting.route, this.waiting.links]; + this.inFlight = undefined; + delete this.waiting.route; + delete this.waiting.links; + for (const request of stranded) { + if (!request) { + continue; + } + try { + request.resolve(answerRouting(this.localIndex, request.request)); + } catch (error) { + request.reject(error instanceof Error ? error : new Error(String(error))); + } } - return new Promise((resolve) => { - this.pending.set(request.requestId, resolve); - this.worker!.postMessage(request); - }); } } diff --git a/src/app/shared/astro/routing.ts b/src/app/shared/astro/routing.ts index eb8aaa3..0b8b615 100644 --- a/src/app/shared/astro/routing.ts +++ b/src/app/shared/astro/routing.ts @@ -23,7 +23,9 @@ export type RoutingRequest = export type RoutingResponse = | { readonly kind: 'route'; readonly requestId: number; readonly route: Route | null; readonly neededRangePc: number | null } - | { readonly kind: 'links'; readonly requestId: number; readonly segments: Float32Array }; + | { readonly kind: 'links'; readonly requestId: number; readonly segments: Float32Array } + /** The question threw in the worker. Sent back so the request settles instead of waiting for good. */ + | { readonly kind: 'failed'; readonly requestId: number; readonly message: string }; /** A spatial index over a catalogue sent as a {@link RoutingCatalogue}. */ export function indexCatalogue({ ids, positions }: RoutingCatalogue): StarNeighbourhood { diff --git a/src/app/shared/astro/routing.worker.ts b/src/app/shared/astro/routing.worker.ts index e40b2dd..403d224 100644 --- a/src/app/shared/astro/routing.worker.ts +++ b/src/app/shared/astro/routing.worker.ts @@ -16,6 +16,10 @@ addEventListener('message', ({ data }: MessageEvent