From 74eeb901dbc8c2fa7a01043c3830ee6429144988 Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Sun, 5 Jul 2026 01:22:05 +0300 Subject: [PATCH 1/4] fix(data): reset all source registrations on path timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit timeoutPathObservable matched a single registration with _pathRegister.find(item => item.path == path) — loose == and first-match by path only. When the same path was registered under multiple $sources, only the first-registered subject received the timeout reset; other-source registrations kept their stale value. Switch to _pathRegisterByPath.get() (strict keyed lookup) and reset every registration for the path. Closes #155 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01VDu7zsKECA9SvBR5DLdUex --- src/app/core/services/data.service.spec.ts | 28 ++++++++++++++++++++++ src/app/core/services/data.service.ts | 8 ++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/app/core/services/data.service.spec.ts b/src/app/core/services/data.service.spec.ts index 977d528f..e0a3195a 100644 --- a/src/app/core/services/data.service.spec.ts +++ b/src/app/core/services/data.service.spec.ts @@ -315,6 +315,34 @@ describe('DataService', () => { expect(latest!.state).toBe(States.Normal); }); + it('resets every source registration for a path on timeout, not just the first', () => { + let latestDefault: IPathUpdate | undefined; + let latestSource: IPathUpdate | undefined; + service + .subscribePath('self.electrical.batteries.10.voltage', 'default') + .subscribe(update => (latestDefault = update)); + service + .subscribePath('self.electrical.batteries.10.voltage', 'test-source') + .subscribe(update => (latestSource = update)); + + dataPathUpdates$.next({ + context: 'self', + path: 'electrical.batteries.10.voltage', + source: 'test-source', + timestamp: '2026-01-01T00:00:01.000Z', + value: 12.5, + }); + expect(latestDefault!.data.value).toBe(12.5); + expect(latestSource!.data.value).toBe(12.5); + + service.timeoutPathObservable('self.electrical.batteries.10.voltage', 'number'); + + // Both the default bucket and the per-source registration must clear on timeout. + expect(latestDefault!.data.value).toBeNull(); + expect(latestSource!.data.value).toBeNull(); + expect(latestSource!.state).toBe(States.Normal); + }); + it('does not emit on timeout for an unrecognised pathType', () => { const updates: IPathUpdate[] = []; service diff --git a/src/app/core/services/data.service.ts b/src/app/core/services/data.service.ts index a5755f48..2af029dc 100644 --- a/src/app/core/services/data.service.ts +++ b/src/app/core/services/data.service.ts @@ -716,9 +716,11 @@ export class DataService implements OnDestroy { * @memberof SignalKDataService */ public timeoutPathObservable(path: string, pathType: string): void { - const pathRegister = this._pathRegister.find(item => item.path == path); - if (pathRegister && ['string', 'Date', 'number', 'multiple'].includes(pathType)) { - pathRegister.pathDataUpdate$.next({ + if (!['string', 'Date', 'number', 'multiple'].includes(pathType)) return; + const registrations = this._pathRegisterByPath.get(path); + if (!registrations) return; + for (const registration of registrations) { + registration.pathDataUpdate$.next({ data: { value: null, timestamp: null From 59742c9d53a18e3df44106d26209c3078761a779 Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Sun, 5 Jul 2026 01:22:05 +0300 Subject: [PATCH 2/4] fix(requests): handle 500/502/504 as known server errors; fix 200 wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deltaStatusCodes carried descriptions for 500/502/504, but handledStatusCodes omitted them, so those responses fell to the "Unknown Request Status Code" error toast — which then printed the very description it claimed not to know. Add them to the handled set and surface them as an error toast carrying their description plus any server message. Also fix the statusCode 200 wording: "The request was successfully." → "The request was successful." Closes #156 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01VDu7zsKECA9SvBR5DLdUex --- .../services/signalk-requests.service.spec.ts | 37 +++++++++++++++++-- .../core/services/signalk-requests.service.ts | 13 ++++++- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/app/core/services/signalk-requests.service.spec.ts b/src/app/core/services/signalk-requests.service.spec.ts index 6a8ab630..3de2f928 100644 --- a/src/app/core/services/signalk-requests.service.spec.ts +++ b/src/app/core/services/signalk-requests.service.spec.ts @@ -87,7 +87,7 @@ describe('SignalkRequestsService', () => { }); it.each<[number, string]>([ - [200, 'The request was successfully.'], + [200, 'The request was successful.'], [401, 'Login failed. Your User ID or Password is incorrect.'], [403, 'DENIED: Authorization with R/W or Admin permission level is required to send commands. Configure Sign In credential.'], [405, 'The server does not support the request.'], @@ -131,16 +131,45 @@ describe('SignalkRequestsService', () => { expect(received[0].statusCode).toBe(200); }); - it('reports a described but unhandled status code (500) as unknown yet still dispatches it', () => { + it.each<[number, string]>([ + [500, 'The request failed.'], + [502, 'Something went wrong carrying out the request on the server.'], + [504, 'Timeout on the server side trying to carry out the request.'], + ])('surfaces a %i server error as an error toast with its description and dispatches it', (statusCode, description) => { + const received: skRequest[] = []; + service.subscribeRequest().subscribe(r => received.push(r)); + + const requestId = service.putRequest('navigation.lights', true, 'widget-1')!; + requestUpdates$.next({ requestId, state: 'COMPLETED', statusCode, message: 'disk full' }); + + expect(toastShow).toHaveBeenCalledWith(`${description} - disk full`, 0, false, 'error'); + expect(received).toHaveLength(1); + expect(received[0].statusCode).toBe(statusCode); + expect(received[0].statusCodeDescription).toBe(description); + }); + + it('shows the bare description for a server error that carries no message', () => { + const received: skRequest[] = []; + service.subscribeRequest().subscribe(r => received.push(r)); + + const requestId = service.putRequest('navigation.lights', true, 'widget-1')!; + requestUpdates$.next({ requestId, state: 'COMPLETED', statusCode: 502 }); + + expect(toastShow).toHaveBeenCalledWith('Something went wrong carrying out the request on the server.', 0, false, 'error'); + expect(received).toHaveLength(1); + expect(received[0].statusCodeDescription).toBe('Something went wrong carrying out the request on the server.'); + }); + + it('reports a truly unknown status code as unknown yet still dispatches it', () => { const received: skRequest[] = []; service.subscribeRequest().subscribe(r => received.push(r)); const requestId = service.putRequest('navigation.lights', true, 'widget-1')!; - requestUpdates$.next({ requestId, state: 'COMPLETED', statusCode: 500 }); + requestUpdates$.next({ requestId, state: 'COMPLETED', statusCode: 418 }); expect(toastShow).toHaveBeenCalledWith(expect.stringContaining('Unknown Request Status Code'), 0, false, 'error'); expect(received).toHaveLength(1); - expect(received[0].statusCode).toBe(500); + expect(received[0].statusCode).toBe(418); expect(received[0].statusCodeDescription).toBeUndefined(); }); diff --git a/src/app/core/services/signalk-requests.service.ts b/src/app/core/services/signalk-requests.service.ts index b53cc0e3..5d04d9e2 100644 --- a/src/app/core/services/signalk-requests.service.ts +++ b/src/app/core/services/signalk-requests.service.ts @@ -8,7 +8,7 @@ import { UUID } from '../utils/uuid.util' import { ToastService } from './toast.service'; const deltaStatusCodes: Record = { - 200: "The request was successfully.", + 200: "The request was successful.", 202: "Request accepted and pending completion.", 400: "Something is wrong with the client's request.", 401: "Login failed. Your User ID or Password is incorrect.", @@ -19,7 +19,9 @@ const deltaStatusCodes: Record = { 504: "Timeout on the server side trying to carry out the request." } // Codes dispatched as recognized responses; anything else raises the unknown-status error toast. -const handledStatusCodes = new Set([200, 202, 400, 401, 403, 405]); +const handledStatusCodes = new Set([200, 202, 400, 401, 403, 405, 500, 502, 504]); +// Server-side failures surfaced to the user as an error toast carrying their description. +const serverErrorStatusCodes = new Set([500, 502, 504]); export interface skRequest { requestId: string; state: string; @@ -132,6 +134,13 @@ export class SignalkRequestsService { console.error("[Request Service] Status Code: " + this.requests[index].statusCode + " - " + this.requests[index].message); } + if (serverErrorStatusCodes.has(this.requests[index].statusCode)) { + const description = this.requests[index].statusCodeDescription; + const detail = this.requests[index].message ? description + " - " + this.requests[index].message : description; + this.toast.show(detail, 0, false, 'error'); + console.error("[Request Service] Status Code: " + this.requests[index].statusCode + " - " + description); + } + } else { this.toast.show("Unknown Request Status Code received: " + this.requests[index].statusCode + " - " + deltaStatusCodes[this.requests[index].statusCode] + " - " + this.requests[index].message, 0, false, 'error'); console.error("[Request Service] Unknown Request Status Code received: " + this.requests[index].statusCode + " - " + deltaStatusCodes[this.requests[index].statusCode] + " - " + this.requests[index].message); From 675eef4fff9c3a3d91c3fcf60d7e003224dc0f8f Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Sun, 5 Jul 2026 01:22:47 +0300 Subject: [PATCH 3/4] fix(dashboard): deep-clone the blank-seed default to stop array aliasing Blank-dashboard seeding spread DefaultDashboard entries shallowly, so a new dashboard's configuration array (and its widget objects) were the same objects as the module-level DefaultDashboard constant; any in-place widget mutation corrupted the shared default for every later blank creation. Deep-clone the default (same defect class as the already-fixed #35/#100). Closes #150 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01VDu7zsKECA9SvBR5DLdUex --- src/app/core/services/dashboard.service.spec.ts | 6 ++++-- src/app/core/services/dashboard.service.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/app/core/services/dashboard.service.spec.ts b/src/app/core/services/dashboard.service.spec.ts index 92b10035..35216fd1 100644 --- a/src/app/core/services/dashboard.service.spec.ts +++ b/src/app/core/services/dashboard.service.spec.ts @@ -92,8 +92,10 @@ describe('DashboardService', () => { expect(dashboards[0].name).toBe('Page 1'); expect(dashboards[0].id).toMatch(UUID_PATTERN); expect(widgetsOf(dashboards[0])[0].input.widgetProperties.type).toBe('widget-tutorial'); - // Pins today's aliasing: the blank dashboard shares its configuration array with the DefaultDashboard constant. - expect(dashboards[0].configuration).toBe(DefaultDashboard[0].configuration); + // The blank dashboard deep-clones the DefaultDashboard constant so later in-place edits cannot corrupt the shared default. + expect(dashboards[0].configuration).not.toBe(DefaultDashboard[0].configuration); + expect(widgetsOf(dashboards[0])[0]).not.toBe(DefaultDashboard[0].configuration![0]); + expect(dashboards[0].configuration).toEqual(DefaultDashboard[0].configuration); expect(consoleWarn).toHaveBeenCalled(); }); diff --git a/src/app/core/services/dashboard.service.ts b/src/app/core/services/dashboard.service.ts index 58d80025..ed412e3e 100644 --- a/src/app/core/services/dashboard.service.ts +++ b/src/app/core/services/dashboard.service.ts @@ -44,7 +44,7 @@ export class DashboardService { if (!dashboardsConfig || dashboardsConfig.length === 0) { console.warn('[Dashboard Service] No dashboards found in settings, creating blank dashboard'); const newBlankDashboard = DefaultDashboard.map(dashboard => ({ - ...dashboard, + ...cloneDeep(dashboard), id: UUID.create() })); this.dashboards.set([...newBlankDashboard]); From 504bbfeb41ad740deea40b66b7c952af470398fb Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Sun, 5 Jul 2026 01:22:47 +0300 Subject: [PATCH 4/4] fix(dashboard): remap active index on delete; guard bounds; reject fractional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit delete() only clamped an active index that overflowed the end; when a lower-indexed dashboard was removed, activeDashboard silently pointed at a different dashboard. Follow the active dashboard to its new index when a lower one is deleted, then clamp. Guard delete() against an out-of-range itemIndex (matching duplicate()/ navigateTo()/setActiveDashboardIndex()): without it the new remap shifts — or, for a negative index with active 0, sets a negative — active index for a delete that removes nothing. Fold in two siblings from the same characterization pass: - setActiveDashboardIndex accepted fractional indexes (Number('1.5') passed the bounds check), so a URL id like '1.5' set a non-integer active index; require an integer. - Two console.error prefixes in duplicate() read "Dashboard Service]" with no opening bracket. Closes #149 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01VDu7zsKECA9SvBR5DLdUex --- .../core/services/dashboard.service.spec.ts | 39 ++++++++++++++++++- src/app/core/services/dashboard.service.ts | 22 ++++++++--- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/app/core/services/dashboard.service.spec.ts b/src/app/core/services/dashboard.service.spec.ts index 35216fd1..6f29ddd2 100644 --- a/src/app/core/services/dashboard.service.spec.ts +++ b/src/app/core/services/dashboard.service.spec.ts @@ -167,6 +167,13 @@ describe('DashboardService', () => { expect(service.activeDashboard()).toBe(1); expect(consoleError).toHaveBeenCalledTimes(2); }); + + it('rejects a fractional index instead of activating a non-integer dashboard', () => { + service.setActiveDashboardIndex(1); + service.setActiveDashboardIndex(1.5); + expect(service.activeDashboard()).toBe(1); + expect(consoleError).toHaveBeenCalledTimes(1); + }); }); describe('add and update', () => { @@ -196,13 +203,41 @@ describe('DashboardService', () => { }); describe('delete', () => { - it('removes the dashboard without remapping the active index', () => { + it('shifts the active index down so it follows the same dashboard when a lower one is removed', () => { setup(); service.setActiveDashboardIndex(1); service.delete(0); expect(service.dashboards().map(d => d.id)).toEqual(['d-1', 'd-2']); - // Pins today's bookkeeping: the index is not shifted down, so it now points at the former third dashboard. + // The active dashboard (d-1) shifts from index 1 to index 0; the active index follows it. + expect(service.activeDashboard()).toBe(0); + expect(service.dashboards()[service.activeDashboard()!].id).toBe('d-1'); + }); + + it('leaves the active index unchanged when a higher-indexed dashboard is removed', () => { + setup(); + service.setActiveDashboardIndex(1); + service.delete(2); expect(service.activeDashboard()).toBe(1); + expect(service.dashboards()[service.activeDashboard()!].id).toBe('d-1'); + }); + + it('keeps the active index on the dashboard that slides up when the active, non-last one is deleted', () => { + setup(); + service.setActiveDashboardIndex(1); + service.delete(1); + expect(service.dashboards().map(d => d.id)).toEqual(['d-0', 'd-2']); + expect(service.activeDashboard()).toBe(1); + expect(service.dashboards()[service.activeDashboard()!].id).toBe('d-2'); + }); + + it('ignores an out-of-range index without touching the active dashboard', () => { + setup(); + service.setActiveDashboardIndex(0); + service.delete(-1); + service.delete(5); + expect(service.dashboards().map(d => d.id)).toEqual(['d-0', 'd-1', 'd-2']); + expect(service.activeDashboard()).toBe(0); + expect(consoleError).toHaveBeenCalledTimes(2); }); it('clamps the active index when it falls past the end', () => { diff --git a/src/app/core/services/dashboard.service.ts b/src/app/core/services/dashboard.service.ts index ed412e3e..fb108542 100644 --- a/src/app/core/services/dashboard.service.ts +++ b/src/app/core/services/dashboard.service.ts @@ -122,7 +122,7 @@ export class DashboardService { if (itemIndex === this.activeDashboard()) return; // No change if the same dashboard is selected to prevent unnecessary cascading updates - if (itemIndex >= 0 && itemIndex < this.dashboards().length) { + if (Number.isInteger(itemIndex) && itemIndex >= 0 && itemIndex < this.dashboards().length) { this.activeDashboard.set(itemIndex); } else { console.error(`[Dashboard Service] Invalid dashboard ID: ${itemIndex}`); @@ -180,14 +180,26 @@ export class DashboardService { * @param itemIndex The index of the dashboard to delete. */ public delete(itemIndex: number): void { + if (itemIndex < 0 || itemIndex >= this.dashboards().length) { + console.error(`[Dashboard Service] Invalid dashboard ID: ${itemIndex}`); + return; + } + + const active = this.activeDashboard(); this.dashboards.update(dashboards => dashboards.filter((_, i) => i !== itemIndex)); if (this.dashboards().length === 0) { this.add('Page ' + (this.dashboards().length + 1), []); this.activeDashboard.set(0); - } else if (this.activeDashboard() > this.dashboards().length - 1) { - this.activeDashboard.set(this.dashboards().length - 1); + return; } + + if (active === null) return; + + // Deleting a dashboard below the active one shifts everything down; follow the + // active dashboard to its new index, then clamp if the active one was the last. + const shifted = itemIndex < active ? active - 1 : active; + this.activeDashboard.set(Math.min(shifted, this.dashboards().length - 1)); } /** @@ -229,11 +241,11 @@ export class DashboardService { widget.id = UUID.create(); widget.input.widgetProperties.uuid = widget.id; } else { - console.error("Dashboard Service] Widget configuration is missing required properties:", widget); + console.error("[Dashboard Service] Widget configuration is missing required properties:", widget); } }); } else { - console.error("Dashboard Service] Dashboard configuration is not an array:", newDashboard.configuration); + console.error("[Dashboard Service] Dashboard configuration is not an array:", newDashboard.configuration); newDashboard.configuration = []; }