From fbc1a9a0ec55c67e71c1f7fedd51308158ea6e2d Mon Sep 17 00:00:00 2001 From: Denis Groshev Date: Fri, 13 Nov 2026 09:22:55 +0100 Subject: [PATCH] fix(cockpit): a provider member that throws when read (a getter) leaves that provider out with one warning and keeps the rest; a worded member that starts throwing keeps its last good words; a candidate, frame or HUD view whose getter throws reads as null, so the view ends or keeps its last HUD --- docs/APPLICATION.md | 10 +- docs/CURRENT-STATE.md | 3 +- src/cockpitSubjectProviders.js | 224 +++++++++++++++++---------- src/cockpitSubjectProviders.test.mjs | 89 +++++++++++ src/ui/cockpitInstruments.js | 9 +- src/ui/cockpitSubjectView.test.mjs | 21 +++ 6 files changed, 264 insertions(+), 92 deletions(-) diff --git a/docs/APPLICATION.md b/docs/APPLICATION.md index 5f9d74109..d0f8ad579 100644 --- a/docs/APPLICATION.md +++ b/docs/APPLICATION.md @@ -237,15 +237,19 @@ Core's Cockpit rides a tracked aircraft, the tracked satellite and the selected AIS vessel. An application adds kinds of its own with `services.cockpitSubjects` on `createApplicationControls`: a list of providers, checked once when the controls are built -(`src/cockpitSubjectProviders.js`). A malformed provider is left out with a -warning. A provider's subject takes entry when it was selected after every +(`src/cockpitSubjectProviders.js`). A malformed provider, or one with a member +that throws when read (a getter), is left out with a warning and the rest are +kept. A provider's subject takes entry when it was selected after every built-in subject present (a tracked aircraft or satellite, a selected vessel), whose own fixed order is unchanged, and needs the provider's `layerId` on; the provider then holds its subject, hands the camera a frame and the generic HUD a view model on every update, and releases it on exit. Provider methods are synchronous and are called on the provider, so a class instance's methods keep their `this`; one that throws or returns a promise is treated as having -failed. +failed. What a method returns may carry getters: one that throws when read +makes a candidate none, a frame malformed (the view ends) and a HUD view keep +the last; a worded member (labels, entry face, look) that throws keeps its +last good words. ```js import { getLocale, onLocaleChange } from '@gods-eye-view/core/i18n'; diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 3ad5c6c26..2b98ef275 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -2213,7 +2213,8 @@ covers entry, camera, zoom, look, HUD and exit. An application can add its own first-person subjects beside the aircraft, vessel and satellite: `services.cockpitSubjects` to `createApplicationControls`, a list of subject providers (`src/cockpitSubjectProviders.js`, checked once by -the coordinator; a malformed one is left out with a warning). A provider names +the coordinator; a malformed one, or one with a member that throws when read, +is left out with a warning and the rest are kept). A provider names its layer, an entry face and labels in the page's language, and supplies four operations: `candidate()` (what entry would ride now), `hold(id)`, `read(nowMs, ctx)` (a camera frame each update) and `hud(frame, ctx)` (a HUD diff --git a/src/cockpitSubjectProviders.js b/src/cockpitSubjectProviders.js index aca759662..e7a8153b3 100644 --- a/src/cockpitSubjectProviders.js +++ b/src/cockpitSubjectProviders.js @@ -104,20 +104,48 @@ function labelsOf(raw) { : null; } +/** A provider's look, normalized, or null without an aheadLabel. */ +function lookOf(raw) { + if (!isObject(raw)) return null; + const aheadLabel = cockpitSubjectText(raw.aheadLabel, 64); + if (!aheadLabel) return null; + return Object.freeze({ + restPitchDeg: finite(raw.restPitchDeg) + ? clamp(raw.restPitchDeg, -90, 90) + : 0, + aheadLabel, + }); +} + /** - * A provider's worded member as it reads now: the value, or what the - * function it is returns when called on `provider` (so it can follow the - * page's language); undefined when that throws. + * A provider's worded member `key` as it reads now, normalized: the value, + * or what the function it is returns when called on `provider` (so it can + * follow the page's language); null when it is malformed or a read throws. + * Reading the member, and reading what it gives, runs provider code too (a + * getter), so both are guarded as the call is. */ -function wordedNow(value, provider) { - if (typeof value !== 'function') return value; +function wordedNow(provider, key, normalize) { try { - return value.call(provider); + const value = provider[key]; + return ( + normalize(typeof value === 'function' ? value.call(provider) : value) ?? + null + ); } catch { - return undefined; + return null; } } +/** A provider's name in a warning: its kind, cut, or its place in the list. */ +function providerName(provider, order) { + try { + if (typeof provider?.kind === 'string') return provider.kind.slice(0, 32); + } catch { + // A kind getter that throws gives no name. + } + return `#${order}`; +} + /** Why `provider` cannot be kept, or null. */ function refusal(provider, kinds) { if (!isObject(provider)) return 'not an object'; @@ -137,15 +165,10 @@ function refusal(provider, kinds) { typeof provider.isEntryAllowed !== 'function' ) return 'isEntryAllowed is not a function'; - if (!faceOf(wordedNow(provider.entryFace, provider))) - return 'malformed entry face'; - if (!labelsOf(wordedNow(provider.labels, provider))) + if (!wordedNow(provider, 'entryFace', faceOf)) return 'malformed entry face'; + if (!wordedNow(provider, 'labels', labelsOf)) return 'labels need view, exit and exitText'; - const look = wordedNow(provider.look, provider); - if ( - provider.look != null && - (!isObject(look) || !cockpitSubjectText(look.aheadLabel, 64)) - ) + if (provider.look != null && !wordedNow(provider, 'look', lookOf)) return 'look needs an aheadLabel'; return null; } @@ -184,7 +207,8 @@ function refusal(provider, kinds) { * once that promise settles; `release` must therefore be harmless for a * subject it does not hold or already gave back. * - * A provider that is not so is left out with one warning naming it. + * A provider that is not so, or whose members throw when read (a getter), is + * left out with one warning naming it; the providers after it are kept. * @param {unknown} list * @param {{warn?: (message: string) => void}} [options] * @returns {ReadonlyArray} @@ -196,88 +220,99 @@ export function cockpitSubjectProviders( const kept = []; const kinds = new Set(); (Array.isArray(list) ? list : []).forEach((provider, order) => { - const reason = refusal(provider, kinds); + let reason; + let copy = null; + try { + reason = refusal(provider, kinds); + if (!reason) copy = keptProvider(provider, order); + } catch { + reason = 'a member threw when read'; + } if (reason) { - const name = - typeof provider?.kind === 'string' - ? provider.kind.slice(0, 32) - : `#${order}`; - warn(`[Cockpit] subject provider ${name} left out: ${reason}`); + warn( + `[Cockpit] subject provider ${providerName(provider, order)} left out: ${reason}`, + ); return; } - kinds.add(provider.kind); - // Worded members are read again on each use (see above); the last good - // read stands in for a bad one. - const worded = (value) => wordedNow(value, provider); - let face = faceOf(worded(provider.entryFace)); - let labels = labelsOf(worded(provider.labels)); - const firstLook = worded(provider.look); - const lookOf = (raw) => - Object.freeze({ - restPitchDeg: finite(firstLook.restPitchDeg) - ? clamp(firstLook.restPitchDeg, -90, 90) - : 0, - aheadLabel: cockpitSubjectText(raw.aheadLabel, 64), - }); - let look = isObject(firstLook) ? lookOf(firstLook) : null; - kept.push( - Object.freeze({ - kind: provider.kind, - layerId: provider.layerId, - priority: finite(provider.priority) ? provider.priority : 0, - order, - get entryFace() { - face = faceOf(worded(provider.entryFace)) ?? face; - return face; - }, - get labels() { - labels = labelsOf(worded(provider.labels)) ?? labels; - return labels; - }, - get look() { - if (!look) return null; - const raw = worded(provider.look); - if (isObject(raw) && cockpitSubjectText(raw.aheadLabel, 64)) - look = lookOf(raw); - return look; - }, - cameraIntervalMs: - finite(provider.cameraIntervalMs) && - provider.cameraIntervalMs >= 0 && - provider.cameraIntervalMs <= 1000 - ? provider.cameraIntervalMs - : null, - headingSlewDps: - provider.headingSlewDps === Infinity || - (finite(provider.headingSlewDps) && provider.headingSlewDps > 0) - ? provider.headingSlewDps - : null, - // Bound, so a method that uses `this` (a class instance's, say) - // still reads its own provider when called on this copy. - candidate: provider.candidate.bind(provider), - isEntryAllowed: provider.isEntryAllowed - ? provider.isEntryAllowed.bind(provider) - : () => true, - hold: provider.hold.bind(provider), - read: provider.read.bind(provider), - hud: provider.hud.bind(provider), - release: provider.release.bind(provider), - }), - ); + kinds.add(copy.kind); + kept.push(copy); }); kept.sort((a, b) => a.priority - b.priority || a.order - b.order); return Object.freeze(kept); } +/** The frozen copy the Cockpit keeps of a provider `refusal` passed. */ +function keptProvider(provider, order) { + // Worded members are read again on each use (see above); the last good + // read stands in for one that is malformed or throws. + let face = wordedNow(provider, 'entryFace', faceOf); + let labels = wordedNow(provider, 'labels', labelsOf); + // On or off, and its rest pitch, as first read. + let look = wordedNow(provider, 'look', lookOf); + const restPitchDeg = look?.restPitchDeg ?? 0; + return Object.freeze({ + kind: provider.kind, + layerId: provider.layerId, + priority: finite(provider.priority) ? provider.priority : 0, + order, + get entryFace() { + face = wordedNow(provider, 'entryFace', faceOf) ?? face; + return face; + }, + get labels() { + labels = wordedNow(provider, 'labels', labelsOf) ?? labels; + return labels; + }, + get look() { + if (!look) return null; + const now = wordedNow(provider, 'look', lookOf); + if (now) + look = Object.freeze({ restPitchDeg, aheadLabel: now.aheadLabel }); + return look; + }, + cameraIntervalMs: + finite(provider.cameraIntervalMs) && + provider.cameraIntervalMs >= 0 && + provider.cameraIntervalMs <= 1000 + ? provider.cameraIntervalMs + : null, + headingSlewDps: + provider.headingSlewDps === Infinity || + (finite(provider.headingSlewDps) && provider.headingSlewDps > 0) + ? provider.headingSlewDps + : null, + // Bound, so a method that uses `this` (a class instance's, say) still + // reads its own provider when called on this copy. + candidate: provider.candidate.bind(provider), + isEntryAllowed: provider.isEntryAllowed + ? provider.isEntryAllowed.bind(provider) + : () => true, + hold: provider.hold.bind(provider), + read: provider.read.bind(provider), + hud: provider.hud.bind(provider), + release: provider.release.bind(provider), + }); +} + /** * A provider's candidate: `{id, name, face, selectedAt}`, or null without a * usable id. `name` falls back to the id; `face` is null unless well formed; * `selectedAt` (a number or a short string, else null) is a token the * provider changes each time its subject is selected, even again: a * subject selected after the built-in ones takes entry (entryCandidates). + * Null too when reading `raw` throws (a getter runs provider code). * @param {unknown} raw What `candidate()` returned. */ export function cockpitSubjectCandidate(raw) { + try { + return candidateOf(raw); + } catch { + // A getter on what candidate() returned threw: no candidate. + return null; + } +} + +function candidateOf(raw) { if (!isObject(raw) || typeof raw.id !== 'string') return null; const id = raw.id.trim(); if (!id || id.length > 128) return null; @@ -297,7 +332,8 @@ export function cockpitSubjectCandidate(raw) { /** * One camera frame from a provider, normalized, or null when it cannot place * the camera (the view then ends): not an object, an id other than `heldId`, - * a position that is not finite, or an eye with no valid `upM` (0..1000). + * a position that is not finite, an eye with no valid `upM` (0..1000), or a + * read of it that throws (a getter runs provider code). * A `heightM` that is not finite or lies outside * COCKPIT_SUBJECT_MIN_HEIGHT_M..COCKPIT_SUBJECT_MAX_HEIGHT_M reads as null: * the eye then stands on the shared floor, or where it last stood while that @@ -309,6 +345,15 @@ export function cockpitSubjectCandidate(raw) { * pitchDeg, minClearanceM}}`. */ export function cockpitSubjectFrame(raw, heldId) { + try { + return frameOf(raw, heldId); + } catch { + // A getter on what read() returned threw: as a malformed frame. + return null; + } +} + +function frameOf(raw, heldId) { if (!isObject(raw) || raw.id !== heldId) return null; const latitude = raw.latitude; const longitude = raw.longitude; @@ -412,10 +457,12 @@ function progressOf(raw) { * A provider's HUD view model, normalized; never throws. Texts are coerced * and cut to their caps, lists to their lengths, tones to * COCKPIT_SUBJECT_TONES, numbers clamped; a malformed member reads as absent. + * Null when reading `raw` throws (a getter runs provider code), so the caller + * keeps the view it has, as when `hud()` itself throws. * All text is shown as given: the provider words it in the page's language. * @param {unknown} raw What `hud()` returned. - * @returns {object} `{kicker, title, meta: {text, tone}, speed: {value, unit, - * tag, tone}|null, readouts: [{id, label, value, detail, tone}], + * @returns {object|null} `{kicker, title, meta: {text, tone}, speed: {value, + * unit, tag, tone}|null, readouts: [{id, label, value, detail, tone}], * sources: [{id, label, text, tone}], * route: {title, from, to, status}|null, * progress: {ticks: [{at, tone, label: {text, detail, priority, @@ -424,6 +471,15 @@ function progressOf(raw) { * lookReadout, signals: [{id, tone, title, detail}]}`. */ export function cockpitSubjectHudView(raw) { + try { + return hudViewOf(raw); + } catch { + // A getter on what hud() returned threw: the caller keeps its last view. + return null; + } +} + +function hudViewOf(raw) { const view = isObject(raw) ? raw : {}; const meta = isObject(view.meta) ? view.meta : {}; const speed = diff --git a/src/cockpitSubjectProviders.test.mjs b/src/cockpitSubjectProviders.test.mjs index 54424d5bf..fb97b93b6 100644 --- a/src/cockpitSubjectProviders.test.mjs +++ b/src/cockpitSubjectProviders.test.mjs @@ -596,3 +596,92 @@ test('labels, entry face and look given as functions are read again on each use' assert.deepEqual(bad.kept, []); assert.equal(bad.warnings.length, 2); }); + +/** `target` with a getter for `key` that throws (provider code runs on a read). */ +const throwingGetter = (target, key) => + Object.defineProperty(target, key, { + enumerable: true, + get() { + throw new Error(`${key} unavailable`); + }, + }); + +test('a provider whose getter throws is left out with one warning; the next is kept', () => { + const { kept, warnings } = keep([ + throwingGetter(provider({ kind: 'thrown-face' }), 'entryFace'), + throwingGetter(provider({ kind: 'thrown-labels' }), 'labels'), + throwingGetter(provider({ kind: 'thrown-look' }), 'look'), + throwingGetter(provider(), 'kind'), + throwingGetter(provider({ kind: 'thrown-method' }), 'read'), + // Read only once the provider is accepted, so its kind must stay free. + throwingGetter(provider({ kind: 'example-kind' }), 'priority'), + provider({ kind: 'example-kind' }), + ]); + assert.deepEqual( + kept.map(({ kind }) => kind), + ['example-kind'], + ); + assert.deepEqual(warnings, [ + '[Cockpit] subject provider thrown-face left out: malformed entry face', + '[Cockpit] subject provider thrown-labels left out: labels need view, exit and exitText', + '[Cockpit] subject provider thrown-look left out: a member threw when read', + '[Cockpit] subject provider #3 left out: a member threw when read', + '[Cockpit] subject provider thrown-method left out: a member threw when read', + '[Cockpit] subject provider example-kind left out: a member threw when read', + ]); +}); + +test('a worded getter that starts throwing keeps the last good words', () => { + let failing = false; + const words = (value) => () => { + if (failing) throw new Error('translations unavailable'); + return value; + }; + const definition = provider(); + for (const [key, value] of Object.entries({ + labels: { view: 'Ride view', exit: 'Exit ride view', exitText: 'EXIT' }, + entryFace: { icon: 'commute', label: 'RIDE' }, + look: { restPitchDeg: 10, aheadLabel: 'Look ahead' }, + })) + Object.defineProperty(definition, key, { get: words(value) }); + const { kept, warnings } = keep([definition]); + assert.deepEqual(warnings, []); + const [ride] = kept; + failing = true; + assert.equal(ride.labels.view, 'Ride view'); + assert.equal(ride.entryFace.label, 'RIDE'); + assert.deepEqual(ride.look, { restPitchDeg: 10, aheadLabel: 'Look ahead' }); + // So does a worded object whose own member throws when read. + failing = false; + Object.defineProperty(definition, 'labels', { + get: () => throwingGetter({ view: 'v', exit: 'e', exitText: 'x' }, 'view'), + }); + assert.equal(ride.labels.view, 'Ride view'); +}); + +test('a candidate, frame or HUD view whose getter throws reads as null', () => { + assert.equal( + cockpitSubjectCandidate(throwingGetter({ name: 'Example' }, 'id')), + null, + ); + const frame = { id: 'a1', latitude: 1, longitude: 2, eye: { upM: 3 } }; + assert.ok(cockpitSubjectFrame(frame, 'a1'), 'well formed'); + assert.equal( + cockpitSubjectFrame(throwingGetter({ ...frame }, 'latitude'), 'a1'), + null, + ); + assert.equal( + cockpitSubjectFrame({ ...frame, eye: throwingGetter({}, 'upM') }, 'a1'), + null, + ); + assert.equal( + cockpitSubjectHudView(throwingGetter({ title: 'T' }, 'meta')), + null, + ); + assert.equal( + cockpitSubjectHudView({ title: 'T', readouts: [throwingGetter({}, 'id')] }), + null, + ); + // Nothing to read is still the empty view, never null. + assert.equal(cockpitSubjectHudView(null).title, ''); +}); diff --git a/src/ui/cockpitInstruments.js b/src/ui/cockpitInstruments.js index 57d0a418e..62cb0e6ec 100644 --- a/src/ui/cockpitInstruments.js +++ b/src/ui/cockpitInstruments.js @@ -154,9 +154,9 @@ const SUBJECT_SIGNAL_TONES = Object.freeze({ /** * Present a provider subject's HUD: its view model (cockpitSubjectHudView) * in the generic readouts, the compass at the camera's heading, the route - * card, and its signals in Live Signals. A HUD view that is null, or a - * `hud()` that throws, keeps the last one. Contact navigation stays with - * aircraft, so the Contact card is withdrawn. + * card, and its signals in Live Signals. A HUD view that is null, a `hud()` + * that throws, or a view whose reading throws keeps the last one. Contact + * navigation stays with aircraft, so the Contact card is withdrawn. */ function updateSubjectHud(info, nowMs, forceContext) { const provider = this.subjectProvider; @@ -176,7 +176,8 @@ function updateSubjectHud(info, nowMs, forceContext) { null, ) : null; - if (raw != null) this.subjectHud = cockpitSubjectHudView(raw); + if (raw != null) + this.subjectHud = cockpitSubjectHudView(raw) ?? this.subjectHud; const hud = this.subjectHud ?? cockpitSubjectHudView(null); const view = this.instruments; view.kind = 'subject'; diff --git a/src/ui/cockpitSubjectView.test.mjs b/src/ui/cockpitSubjectView.test.mjs index e988fd80e..fd0290c4f 100644 --- a/src/ui/cockpitSubjectView.test.mjs +++ b/src/ui/cockpitSubjectView.test.mjs @@ -604,6 +604,16 @@ test('the view ends when the frame stops coming, and gives the subject back', as for (const ending of [ null, { latitude: LAT, longitude: LON, id: 'other', eye: { upM: 2 } }, + // A frame whose getter throws runs provider code: as a malformed frame. + Object.defineProperty( + { id: 'a1', longitude: LON, eye: { upM: 2 } }, + 'latitude', + { + get() { + throw new Error('latitude unavailable'); + }, + }, + ), ]) { const ride = subjectProvider(); const { cockpit } = fixture(t, { providers: [ride] }); @@ -937,6 +947,17 @@ test('a read that throws ends the view once; a hud that throws keeps the last', await settle(); assert.equal(cockpit.active, true); assert.equal($('#cockpit-callsign').textContent, HUD.title, 'last view kept'); + // So does a view whose getter throws when read. + ride.subject.hud = Object.defineProperty({ ...HUD }, 'title', { + get() { + throw new Error('title unavailable'); + }, + }); + clock.advance(200); + cockpit.update(); + await settle(); + assert.equal(cockpit.active, true); + assert.equal($('#cockpit-callsign').textContent, HUD.title, 'last view kept'); ride.subject.frame = new Error('read failed'); clock.advance(16); cockpit.update();