From: Jérôme Benoit Date: Wed, 8 Jul 2026 12:46:10 +0000 (+0200) Subject: fix(ocpp): reset stationInfo.firmwareStatus on abort/exception in simulateFirmwareUpd... X-Git-Tag: cli@v4.11.0~48 X-Git-Url: https://git.piment-noir.org/?a=commitdiff_plain;h=4f959e9f8a939b309b0d74650a07e96abfba2860;p=e-mobility-charging-stations-simulator.git fix(ocpp): reset stationInfo.firmwareStatus on abort/exception in simulateFirmwareUpdateLifecycle (#1976) simulateFirmwareUpdateLifecycle wrote stationInfo.firmwareStatus at every status transition via sendFirmwareStatusNotification, but its finally block only cleared the per-station WeakMap state (activeFirmwareUpdateAbortController, activeFirmwareUpdateRequestId). On exception or abort (supersession, stop()), stationInfo.firmwareStatus stayed at the last-emitted non-terminal value (e.g. 'Downloading'), leaving a latent data-model split any future consumer that does not cross-check activeFirmwareUpdateRequestId would observe. Track the current lifecycle stage inside the lifecycle scope and, in the existing finally, reset stationInfo.firmwareStatus in-place when the lifecycle did not reach Installed. Per OCPP 2.0.1 FirmwareStatusEnumType, Idle SHALL only be used in TriggerMessage-triggered notifications, so as a persistent local state Idle is only valid before install starts: - clean download-phase abort -> Idle - clean install-phase abort -> InstallationFailed - exception during download stage -> DownloadFailed - exception during install stage -> InstallationFailed Per OCPP 2.0.1 L01.FR.24 Note the terminal *Failed FirmwareStatusNotification on supersession is optional; L02.FR.15 Note omits any such clause. Emitting no FirmwareStatusNotification from finally is safe under both profiles. The field is reset in-place; no FirmwareStatusNotification is emitted from finally, so the simulator does not race the caller driving the supersession. Stage transitions are placed only after successful advancement (after the Installing / Installed notification awaits resolve), never on error/abort branches, so the terminal-value selection in finally is unambiguous. The finally guards mirror clearActiveFirmwareUpdate's requestId-supersession pattern to avoid clobbering a superseder's live status, and preserve any explicit terminal already emitted from inside try (DownloadFailed / InvalidSignature / InstallationFailed). Stage->terminal mapping uses `Record, OCPP20FirmwareStatusEnumType>` with `as const satisfies` for compile-time exhaustiveness over the failure-eligible stages. Naming convention (SCREAMING_SNAKE_CASE) matches CoherentMeterValueBuilder's PHASE_FAMILY/PHASE_RANK pattern for private module-scope Record lookup tables. Add OCPP20VariableManager.getInstance().resetRuntimeOverrides() to the shared standardCleanup test helper so per-station variable overrides do not leak between tests (all mock stations share a hardcoded hashId). Add 8 tests covering: supersession mid-download -> Idle, supersession mid-install -> InstallationFailed, throw during download -> DownloadFailed, throw during install -> InstallationFailed, happy path preserved -> Installed, supersession-with-emit race (T2 launched via constructor listener) -> Downloading preserved, InvalidSignature preserved, retries-exhausted DownloadFailed preserved. Closes #1969 --- diff --git a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts index 27778cb1..0695d560 100644 --- a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts +++ b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts @@ -255,6 +255,13 @@ const buildRejectedResponse = ( ...(transactionId != null && { transactionId }), }) +type FirmwareStage = 'download' | 'install' | 'installed' + +const FIRMWARE_STAGE_FAILURE_STATUS = { + download: OCPP20FirmwareStatusEnumType.DownloadFailed, + install: OCPP20FirmwareStatusEnumType.InstallationFailed, +} as const satisfies Record, OCPP20FirmwareStatusEnumType> + interface OCPP20StationState { activeFirmwareUpdateAbortController?: AbortController activeFirmwareUpdateRequestId?: number @@ -3781,6 +3788,14 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { const checkAborted = (): boolean => abortController.signal.aborted + // Lifecycle-stage tracker for the finally-block terminal-status reset + // (issue #1969): advances only on actual lifecycle advancement, never on + // error/abort branches. + // 'download' → still in the download/verify stage (initial) + // 'install' → Installing notification succeeded, install stage entered + // 'installed' → Installed notification succeeded, lifecycle reached success + let stage: FirmwareStage = 'download' + try { // Delay the download until retrieveDateTime; inform the CSMS via DownloadScheduled first const now = Date.now() @@ -3938,6 +3953,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { OCPP20FirmwareStatusEnumType.Installing, requestId ) + stage = 'install' await interruptibleSleep(OCPP20Constants.RESET_DELAY_MS, abortController.signal) if (checkAborted()) return @@ -3946,6 +3962,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { OCPP20FirmwareStatusEnumType.Installed, requestId ) + stage = 'installed' // Send SecurityEventNotification after a successful firmware update this.sendSecurityEventNotification( @@ -3958,6 +3975,37 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { `${chargingStation.logPrefix()} ${moduleName}.simulateFirmwareUpdateLifecycle: Firmware update simulation completed for requestId ${requestId.toString()}` ) } finally { + // Reset stationInfo.firmwareStatus to a coherent terminal value when the + // lifecycle exits before reaching Installed (issue #1969): otherwise any + // consumer that does not cross-check activeFirmwareUpdateRequestId would + // observe a stale in-progress value. FirmwareStatusEnumType.Idle is + // SHALL-restricted to TriggerMessage-triggered notifications, so as a + // persistent local state Idle only fits the pre-install baseline: clean + // download-phase abort → Idle; clean install-phase abort → InstallationFailed + // (spec-recognized for cancellation per L01.FR.24 Note). No + // FirmwareStatusNotification is emitted from finally (L01.FR.24 Note MAY + // makes the terminal notification optional; L02.FR.15 omits the clause). + // Guards (`successorClaimed`, `isExplicitTerminal`) mirror + // `clearActiveFirmwareUpdate`'s requestId-supersession pattern and + // preserve any explicit terminal already emitted from inside try. + const activeRequestId = this.stationsState.get(chargingStation)?.activeFirmwareUpdateRequestId + const successorClaimed = activeRequestId != null && activeRequestId !== requestId + const currentStatus = chargingStation.stationInfo?.firmwareStatus + const isExplicitTerminal = + currentStatus === OCPP20FirmwareStatusEnumType.DownloadFailed || + currentStatus === OCPP20FirmwareStatusEnumType.InstallationFailed || + currentStatus === OCPP20FirmwareStatusEnumType.InvalidSignature + if ( + stage !== 'installed' && + !successorClaimed && + !isExplicitTerminal && + chargingStation.stationInfo != null + ) { + chargingStation.stationInfo.firmwareStatus = + abortController.signal.aborted && stage === 'download' + ? OCPP20FirmwareStatusEnumType.Idle + : FIRMWARE_STAGE_FAILURE_STATUS[stage] + } // Guarantee cleanup on every exit path (happy, abort, throw): without // this, a thrown await would leave activeFirmwareUpdateAbortController // set and any subsequent UpdateFirmware.req would abort a non-existent diff --git a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts index 76858b40..d7b9c818 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts @@ -885,5 +885,437 @@ await describe('L01/L02 - UpdateFirmware', async () => { }) }) }) + + await describe('stationInfo.firmwareStatus reset on exit (issue #1969)', async () => { + await it('should reset stationInfo.firmwareStatus to Idle on clean abort mid-download (L01.FR.24)', async t => { + const { sentRequests, station: trackingStation } = createMockStationWithRequestTracking() + const service = new OCPP20IncomingRequestService() + const testable = createTestableIncomingRequestService(service) + + const firstRequest: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/v1.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 200, + } + const firstResponse: OCPP20UpdateFirmwareResponse = { + status: UpdateFirmwareStatusEnumType.Accepted, + } + + await withMockTimers(t, ['setTimeout'], async () => { + service.emit( + OCPP20IncomingRequestCommand.UPDATE_FIRMWARE, + trackingStation, + firstRequest, + firstResponse + ) + await flushMicrotasks() + assert.strictEqual(sentRequests.length, 1) + assert.strictEqual( + sentRequests[0].payload.status, + OCPP20FirmwareStatusEnumType.Downloading + ) + assert.strictEqual( + trackingStation.stationInfo?.firmwareStatus, + OCPP20FirmwareStatusEnumType.Downloading + ) + + const secondRequest: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/v2.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 201, + } + const secondResponse = testable.handleRequestUpdateFirmware( + trackingStation, + secondRequest + ) + assert.strictEqual(secondResponse.status, UpdateFirmwareStatusEnumType.AcceptedCanceled) + await flushMicrotasks() + + assert.strictEqual( + trackingStation.stationInfo.firmwareStatus, + OCPP20FirmwareStatusEnumType.Idle + ) + }) + }) + + await it('should reset stationInfo.firmwareStatus to InstallationFailed on clean abort mid-install (Idle TriggerMessage-only; L01.FR.24 MAY)', async t => { + const { sentRequests, station: trackingStation } = createMockStationWithRequestTracking() + const service = new OCPP20IncomingRequestService() + const testable = createTestableIncomingRequestService(service) + + const firstRequest: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/v1.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 210, + } + const firstResponse: OCPP20UpdateFirmwareResponse = { + status: UpdateFirmwareStatusEnumType.Accepted, + } + + await withMockTimers(t, ['setTimeout'], async () => { + service.emit( + OCPP20IncomingRequestCommand.UPDATE_FIRMWARE, + trackingStation, + firstRequest, + firstResponse + ) + await flushMicrotasks() + // Downloading emitted; advance past FIRMWARE_STATUS_DELAY_MS to reach Installing + t.mock.timers.tick(2000) + await flushMicrotasks() + assert.strictEqual(sentRequests.length, 3) + assert.strictEqual( + sentRequests[2].payload.status, + OCPP20FirmwareStatusEnumType.Installing + ) + assert.strictEqual( + trackingStation.stationInfo?.firmwareStatus, + OCPP20FirmwareStatusEnumType.Installing + ) + + const secondRequest: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/v2.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 211, + } + const secondResponse = testable.handleRequestUpdateFirmware( + trackingStation, + secondRequest + ) + assert.strictEqual(secondResponse.status, UpdateFirmwareStatusEnumType.AcceptedCanceled) + await flushMicrotasks() + + assert.strictEqual( + trackingStation.stationInfo.firmwareStatus, + OCPP20FirmwareStatusEnumType.InstallationFailed + ) + }) + }) + + await it('should reset stationInfo.firmwareStatus to DownloadFailed on exception during download', async t => { + const { + requestHandlerMock, + sentRequests, + station: trackingStation, + } = createMockStationWithRequestTracking() + const service = new OCPP20IncomingRequestService() + + // Reject on the 2nd FirmwareStatusNotification (Downloaded) to force a throw in the download stage + requestHandlerMock.mock.mockImplementation((...args: unknown[]) => { + const payload = args[2] as { requestId?: number; status?: OCPP20FirmwareStatusEnumType } + sentRequests.push({ + command: args[1] as OCPP20RequestCommand, + payload, + }) + if (payload.status === OCPP20FirmwareStatusEnumType.Downloaded) { + return Promise.reject(new Error('simulated download failure')) + } + return Promise.resolve({}) + }) + + const request: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/update.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 220, + } + const response: OCPP20UpdateFirmwareResponse = { + status: UpdateFirmwareStatusEnumType.Accepted, + } + + await withMockTimers(t, ['setTimeout'], async () => { + service.emit( + OCPP20IncomingRequestCommand.UPDATE_FIRMWARE, + trackingStation, + request, + response + ) + await flushMicrotasks() + assert.strictEqual( + sentRequests[0].payload.status, + OCPP20FirmwareStatusEnumType.Downloading + ) + t.mock.timers.tick(2000) + await flushMicrotasks() + + assert.strictEqual( + trackingStation.stationInfo?.firmwareStatus, + OCPP20FirmwareStatusEnumType.DownloadFailed + ) + }) + }) + + await it('should reset stationInfo.firmwareStatus to InstallationFailed on exception during install', async t => { + const { + requestHandlerMock, + sentRequests, + station: trackingStation, + } = createMockStationWithRequestTracking() + const service = new OCPP20IncomingRequestService() + + // Reject on the Installed FirmwareStatusNotification: Installing already succeeded so stage='install' + requestHandlerMock.mock.mockImplementation((...args: unknown[]) => { + const payload = args[2] as { requestId?: number; status?: OCPP20FirmwareStatusEnumType } + sentRequests.push({ + command: args[1] as OCPP20RequestCommand, + payload, + }) + if (payload.status === OCPP20FirmwareStatusEnumType.Installed) { + return Promise.reject(new Error('simulated install failure')) + } + return Promise.resolve({}) + }) + + const request: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/update.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 230, + } + const response: OCPP20UpdateFirmwareResponse = { + status: UpdateFirmwareStatusEnumType.Accepted, + } + + await withMockTimers(t, ['setTimeout'], async () => { + service.emit( + OCPP20IncomingRequestCommand.UPDATE_FIRMWARE, + trackingStation, + request, + response + ) + await flushMicrotasks() + t.mock.timers.tick(2000) + await flushMicrotasks() + assert.strictEqual( + sentRequests[2].payload.status, + OCPP20FirmwareStatusEnumType.Installing + ) + t.mock.timers.tick(1000) + await flushMicrotasks() + + assert.strictEqual( + trackingStation.stationInfo?.firmwareStatus, + OCPP20FirmwareStatusEnumType.InstallationFailed + ) + }) + }) + + await it('should leave stationInfo.firmwareStatus as Installed on happy-path completion', async t => { + const { sentRequests, station: trackingStation } = createMockStationWithRequestTracking() + const service = new OCPP20IncomingRequestService() + + const request: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/update.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 240, + } + const response: OCPP20UpdateFirmwareResponse = { + status: UpdateFirmwareStatusEnumType.Accepted, + } + + await withMockTimers(t, ['setTimeout'], async () => { + service.emit( + OCPP20IncomingRequestCommand.UPDATE_FIRMWARE, + trackingStation, + request, + response + ) + await flushMicrotasks() + t.mock.timers.tick(2000) + await flushMicrotasks() + t.mock.timers.tick(1000) + await flushMicrotasks() + + assert.strictEqual(sentRequests[3].payload.status, OCPP20FirmwareStatusEnumType.Installed) + assert.strictEqual( + trackingStation.stationInfo?.firmwareStatus, + OCPP20FirmwareStatusEnumType.Installed + ) + }) + }) + + await it('should NOT clobber a successor lifecycle firmwareStatus (supersession-with-emit race)', async t => { + // Locks the finally-block requestId-guard invariant: when T1 wakes up + // after a superseder T2 has claimed activeFirmwareUpdateRequestId, + // T1's finally must NOT overwrite T2's status. Mirrors the production + // path where the constructor's UPDATE_FIRMWARE listener launches T2 + // on the AcceptedCanceled branch. + const { sentRequests, station: trackingStation } = createMockStationWithRequestTracking() + const service = new OCPP20IncomingRequestService() + const testable = createTestableIncomingRequestService(service) + + const firstRequest: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/v1.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 250, + } + const firstResponse: OCPP20UpdateFirmwareResponse = { + status: UpdateFirmwareStatusEnumType.Accepted, + } + + await withMockTimers(t, ['setTimeout'], async () => { + service.emit( + OCPP20IncomingRequestCommand.UPDATE_FIRMWARE, + trackingStation, + firstRequest, + firstResponse + ) + await flushMicrotasks() + assert.strictEqual( + sentRequests[0].payload.status, + OCPP20FirmwareStatusEnumType.Downloading + ) + + const secondRequest: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/v2.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 251, + } + // Production sequence: handleRequestUpdateFirmware aborts T1 + returns + // AcceptedCanceled, then the base class emits UPDATE_FIRMWARE which + // launches T2's lifecycle. Replicate the emit here. + const secondResponse = testable.handleRequestUpdateFirmware( + trackingStation, + secondRequest + ) + assert.strictEqual(secondResponse.status, UpdateFirmwareStatusEnumType.AcceptedCanceled) + service.emit( + OCPP20IncomingRequestCommand.UPDATE_FIRMWARE, + trackingStation, + secondRequest, + secondResponse + ) + await flushMicrotasks() + + // T2 wrote firmwareStatus='Downloading' synchronously at its start. + // T1's finally saw activeFirmwareUpdateRequestId === 251 !== 250 and + // skipped its reset. Without the guard, T1 would have written Idle. + assert.notStrictEqual( + trackingStation.stationInfo?.firmwareStatus, + OCPP20FirmwareStatusEnumType.Idle + ) + assert.strictEqual( + trackingStation.stationInfo?.firmwareStatus, + OCPP20FirmwareStatusEnumType.Downloading + ) + }) + }) + + await it('should preserve InvalidSignature when signature verification fails (no clobber in finally)', async t => { + const { sentRequests, station: trackingStation } = createMockStationWithRequestTracking() + const service = new OCPP20IncomingRequestService() + const { OCPP20VariableManager } = + await import('../../../../src/charging-station/ocpp/2.0/OCPP20VariableManager.js') + const { AttributeEnumType, OCPP20ComponentName } = + await import('../../../../src/types/index.js') + + OCPP20VariableManager.getInstance().setVariables(trackingStation, [ + { + attributeType: AttributeEnumType.Actual, + attributeValue: 'true', + component: { name: OCPP20ComponentName.FirmwareCtrlr }, + variable: { name: 'SimulateSignatureVerificationFailure' }, + }, + ]) + + const request: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'https://firmware.example.com/update.bin', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + signature: 'dGVzdA==', + }, + requestId: 260, + } + const response: OCPP20UpdateFirmwareResponse = { + status: UpdateFirmwareStatusEnumType.Accepted, + } + + await withMockTimers(t, ['setTimeout'], async () => { + service.emit( + OCPP20IncomingRequestCommand.UPDATE_FIRMWARE, + trackingStation, + request, + response + ) + await flushMicrotasks() + t.mock.timers.tick(2000) + await flushMicrotasks() + t.mock.timers.tick(500) + await flushMicrotasks() + + assert.strictEqual( + sentRequests[2].payload.status, + OCPP20FirmwareStatusEnumType.InvalidSignature + ) + assert.strictEqual( + trackingStation.stationInfo?.firmwareStatus, + OCPP20FirmwareStatusEnumType.InvalidSignature + ) + }) + }) + + await it('should preserve DownloadFailed after retries-exhausted exit (L01.FR.30 idempotence)', async t => { + const { sentRequests, station: trackingStation } = createMockStationWithRequestTracking() + const service = new OCPP20IncomingRequestService() + + const request: OCPP20UpdateFirmwareRequest = { + firmware: { + location: 'not-a-valid-url', + retrieveDateTime: new Date('2020-01-01T00:00:00.000Z'), + }, + requestId: 270, + retries: 1, + retryInterval: 2, + } + const response: OCPP20UpdateFirmwareResponse = { + status: UpdateFirmwareStatusEnumType.Accepted, + } + + await withMockTimers(t, ['setTimeout'], async () => { + service.emit( + OCPP20IncomingRequestCommand.UPDATE_FIRMWARE, + trackingStation, + request, + response + ) + await flushMicrotasks() + // Initial Downloading delay + retry interval + retry Downloading + retry delay + t.mock.timers.tick(2000) + await flushMicrotasks() + t.mock.timers.tick(2000) + await flushMicrotasks() + t.mock.timers.tick(2000) + await flushMicrotasks() + + const lastFirmwareNotification = sentRequests + .filter(req => req.command === OCPP20RequestCommand.FIRMWARE_STATUS_NOTIFICATION) + .at(-1) + assert.strictEqual( + lastFirmwareNotification?.payload.status, + OCPP20FirmwareStatusEnumType.DownloadFailed + ) + assert.strictEqual( + trackingStation.stationInfo?.firmwareStatus, + OCPP20FirmwareStatusEnumType.DownloadFailed + ) + }) + }) + }) }) }) diff --git a/tests/helpers/TestLifecycleHelpers.ts b/tests/helpers/TestLifecycleHelpers.ts index 5d4820de..b988613e 100644 --- a/tests/helpers/TestLifecycleHelpers.ts +++ b/tests/helpers/TestLifecycleHelpers.ts @@ -30,6 +30,7 @@ import { mock } from 'node:test' import type { ChargingStation } from '../../src/charging-station/index.js' import type { MockChargingStationOptions } from '../charging-station/helpers/StationHelpers.js' +import { OCPP20VariableManager } from '../../src/charging-station/ocpp/2.0/OCPP20VariableManager.js' import { createMockChargingStation } from '../charging-station/helpers/StationHelpers.js' import { MockIdTagsCache, MockSharedLRUCache } from '../charging-station/mocks/MockCaches.js' @@ -346,6 +347,7 @@ export function standardCleanup (): void { } MockSharedLRUCache.resetInstance() MockIdTagsCache.resetInstance() + OCPP20VariableManager.getInstance().resetRuntimeOverrides() } /**