From 6f7bcb64be5467894e595dbdc9d3ad4703fb1541 Mon Sep 17 00:00:00 2001 From: Daniel <7558512+DerGenaue@users.noreply.github.com> Date: Fri, 17 Jul 2026 17:22:18 +0200 Subject: [PATCH] fix(charging-station): re-dial after a server-initiated connection close (#2021) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * fix(charging-station): re-dial after a server-initiated connection close When the CSMS or a proxy closed a station's WebSocket with a normal (clean) close, the station stayed "started but not connected" instead of reconnecting. onClose only re-dialed on abnormal close codes and treated clean closes as terminal, but a clean close cannot be told apart by code from the station's own closeWSConnection() (the UI disconnect action, which must stay down). Real charge-point hardware re-dials after losing the connection regardless of the close code. Mark only explicitly-requested closes (the UI disconnect) as terminal via a byRequest flag on closeWSConnection(), and reconnect on any close the station did not request while it is still started -- clean or abnormal. This also fixes certificate rotation, which closes the socket to force a re-dial with the new certificate and previously never reconnected. Closes #2016. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com> * refactor(charging-station): pass closeWSConnection intent via an options object Match the paired openWSConnection signature and make the call site self-documenting: closeWSConnection({ byRequest: true }) rather than a bare positional boolean. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com> * test(charging-station): cover the onClose reconnect decision Drive onClose directly with a spied reconnect: the station re-dials after a server-initiated normal close while started, and stays disconnected after an operator-requested close. Removes a stale test that only asserted the socket reached CLOSED, which held regardless of the reconnect decision. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com> * docs(charging-station): harmonize reconnect terminology and tighten close comments - Use the codebase's "reconnect" term consistently (drop "re-dial"), and "server-initiated"/"requested" to match the PR title and the byRequest option. - Tighten the closeWSConnection JSDoc and onClose comments; align the JSDoc @param style (no trailing period) with the sibling openWSConnection. - Prefix the reconnect test titles with "should" per tests/TEST_STYLE_GUIDE.md. No behavior change. --------- Signed-off-by: Daniel <7558512+DerGenaue@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Jérôme Benoit --- src/charging-station/ChargingStation.ts | 51 ++++++++---- .../ChargingStationWorkerBroadcastChannel.ts | 3 +- .../ChargingStation-Reconnect.test.ts | 83 +++++++++++++++++++ .../ChargingStation-Resilience.test.ts | 14 ---- ...rgingStationWorkerBroadcastChannel.test.ts | 15 ++++ 5 files changed, 136 insertions(+), 30 deletions(-) create mode 100644 tests/charging-station/ChargingStation-Reconnect.test.ts diff --git a/src/charging-station/ChargingStation.ts b/src/charging-station/ChargingStation.ts index 4897a667..6f3d13e2 100644 --- a/src/charging-station/ChargingStation.ts +++ b/src/charging-station/ChargingStation.ts @@ -223,6 +223,7 @@ export class ChargingStation extends EventEmitter { private stopping: boolean private templateFileHash: string private templateFileWatcher?: FSWatcher + private wsConnectionClosedByRequest: boolean private wsConnectionRetryCount: number private wsPingSetInterval?: NodeJS.Timeout @@ -232,6 +233,7 @@ export class ChargingStation extends EventEmitter { this.starting = false this.stopping = false this.wsConnection = null + this.wsConnectionClosedByRequest = false this.wsConnectionRetryCount = 0 this.index = index this.templateFile = templateFile @@ -358,9 +360,18 @@ export class ChargingStation extends EventEmitter { this.setIntervalFlushMessageBuffer() } - /** Closes the WebSocket connection to the central server. */ - public closeWSConnection (): void { + /** + * Closes the WebSocket connection to the central server. + * @param options - Close options + * @param options.byRequest - Whether the close is requested (the UI disconnect + * action); a requested close is terminal (onClose does not reconnect), any + * other close (e.g. certificate rotation) reconnects like a server-initiated drop + */ + public closeWSConnection ({ byRequest = false }: { byRequest?: boolean } = {}): void { if (this.isWebSocketConnectionOpened()) { + if (byRequest) { + this.wsConnectionClosedByRequest = true + } this.wsConnection?.close() } } @@ -2345,6 +2356,10 @@ export class ChargingStation extends EventEmitter { private onClose (code: WebSocketCloseEventStatusCode, reason: Buffer): void { this.emitChargingStationEvent(ChargingStationEvents.disconnected) this.emitChargingStationEvent(ChargingStationEvents.updated) + // Capture and clear the requested-close marker before deciding whether to + // reconnect. + const closedByRequest = this.wsConnectionClosedByRequest + this.wsConnectionClosedByRequest = false switch (code) { // Normal close case WebSocketCloseEventStatusCode.CLOSE_NO_STATUS: @@ -2354,7 +2369,6 @@ export class ChargingStation extends EventEmitter { code )}' and reason '${reason.toString()}'` ) - this.wsConnectionRetryCount = 0 break // Abnormal close default: @@ -2363,20 +2377,27 @@ export class ChargingStation extends EventEmitter { code )}' and reason '${reason.toString()}'` ) - this.started && - this.reconnect() - .then(() => { - this.emitChargingStationEvent(ChargingStationEvents.updated) - return undefined - }) - .catch((error: unknown) => - logger.error( - `${this.logPrefix()} ${moduleName}.onClose: Error while reconnecting:`, - error - ) - ) break } + // Reconnect on any close we did not request while still started: a + // server-initiated drop, or an internal close wanting a fresh connection + // (e.g. certificate rotation), clean or abnormal. Only a requested close + // stays terminal. + if (this.started && !closedByRequest) { + this.reconnect() + .then(() => { + this.emitChargingStationEvent(ChargingStationEvents.updated) + return undefined + }) + .catch((error: unknown) => + logger.error( + `${this.logPrefix()} ${moduleName}.onClose: Error while reconnecting:`, + error + ) + ) + } else { + this.wsConnectionRetryCount = 0 + } } private onError (error: WSError): void { diff --git a/src/charging-station/broadcast-channel/ChargingStationWorkerBroadcastChannel.ts b/src/charging-station/broadcast-channel/ChargingStationWorkerBroadcastChannel.ts index b4aa7aa1..2e5035ed 100644 --- a/src/charging-station/broadcast-channel/ChargingStationWorkerBroadcastChannel.ts +++ b/src/charging-station/broadcast-channel/ChargingStationWorkerBroadcastChannel.ts @@ -129,7 +129,8 @@ export class ChargingStationWorkerBroadcastChannel extends WorkerBroadcastChanne [ BroadcastChannelProcedureName.CLOSE_CONNECTION, () => { - this.chargingStation.closeWSConnection() + // Requested disconnect (UI action): terminal, must not auto-reconnect. + this.chargingStation.closeWSConnection({ byRequest: true }) }, ], [BroadcastChannelProcedureName.DATA_TRANSFER, this.passthrough(RequestCommand.DATA_TRANSFER)], diff --git a/tests/charging-station/ChargingStation-Reconnect.test.ts b/tests/charging-station/ChargingStation-Reconnect.test.ts new file mode 100644 index 00000000..84464f48 --- /dev/null +++ b/tests/charging-station/ChargingStation-Reconnect.test.ts @@ -0,0 +1,83 @@ +/** + * @file Tests the reconnect decision made when a charging station's WebSocket closes. + * @description The station reconnects after any close it did not itself request + * (a server-initiated drop, clean or abnormal) while still started, and stays + * disconnected after a requested close. + */ +import assert from 'node:assert/strict' +import { copyFileSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, it } from 'node:test' +import { WebSocket } from 'ws' + +import { ChargingStation } from '../../src/charging-station/ChargingStation.js' +import { WebSocketCloseEventStatusCode } from '../../src/types/index.js' +import { standardCleanup } from '../helpers/TestLifecycleHelpers.js' + +// onClose and reconnect are private; the tests drive onClose directly with a +// spied reconnect to observe the reconnect decision without opening a socket. +interface StationInternals { + onClose: (code: WebSocketCloseEventStatusCode, reason: Buffer) => void + reconnect: () => Promise + started: boolean + wsConnection: unknown +} + +const tmpRoots: string[] = [] + +// Build a started station whose reconnect() is replaced by a counter. +const makeStation = (): { reconnectCount: () => number; station: ChargingStation } => { + const root = mkdtempSync(join(tmpdir(), 'cs-reconnect-')) + tmpRoots.push(root) + mkdirSync(join(root, 'station-templates'), { recursive: true }) + const templateFile = join(root, 'station-templates', 'virtual-simple.station-template.json') + copyFileSync( + join(process.cwd(), 'src/assets/station-templates/virtual-simple.station-template.json'), + templateFile + ) + const station = new ChargingStation(1, templateFile, { + autoStart: false, + supervisionUrls: 'ws://localhost:9999/', + }) + let reconnects = 0 + const internals = station as unknown as StationInternals + internals.reconnect = () => { + reconnects++ + return Promise.resolve() + } + internals.started = true + return { reconnectCount: () => reconnects, station } +} + +await describe('ChargingStation reconnect decision on WebSocket close', async () => { + afterEach(() => { + standardCleanup() + for (const root of tmpRoots.splice(0)) { + rmSync(root, { force: true, recursive: true }) + } + }) + + await it('should reconnect after a server-initiated normal close while started', () => { + const { reconnectCount, station } = makeStation() + + ;(station as unknown as StationInternals).onClose( + WebSocketCloseEventStatusCode.CLOSE_NORMAL, + Buffer.from('') + ) + + assert.strictEqual(reconnectCount(), 1) + }) + + await it('should stay disconnected after a requested close', () => { + const { reconnectCount, station } = makeStation() + const internals = station as unknown as StationInternals + // An open socket lets closeWSConnection record the request before onClose runs. + internals.wsConnection = { close: () => undefined, readyState: WebSocket.OPEN } + + station.closeWSConnection({ byRequest: true }) + internals.onClose(WebSocketCloseEventStatusCode.CLOSE_NORMAL, Buffer.from('')) + + assert.strictEqual(reconnectCount(), 0) + }) +}) diff --git a/tests/charging-station/ChargingStation-Resilience.test.ts b/tests/charging-station/ChargingStation-Resilience.test.ts index bf562745..6a3cb5be 100644 --- a/tests/charging-station/ChargingStation-Resilience.test.ts +++ b/tests/charging-station/ChargingStation-Resilience.test.ts @@ -48,20 +48,6 @@ await describe('ChargingStation Resilience', async () => { assert.strictEqual(mocks.webSocket.readyState, 3) // CLOSED }) - await it('should not reconnect on normal WebSocket close', () => { - // Arrange - const result = createMockChargingStation({ connectorsCount: 1 }) - station = result.station - const mocks = result.mocks - station.started = true - - // Act - Simulate normal close (code 1000 = normal closure) - mocks.webSocket.simulateClose(1000, 'Normal closure') - - // Assert - WebSocket should be closed - assert.strictEqual(mocks.webSocket.readyState, 3) // CLOSED - }) - await it('should track connection retry count', () => { // Arrange const result = createMockChargingStation({ connectorsCount: 1 }) diff --git a/tests/charging-station/broadcast-channel/ChargingStationWorkerBroadcastChannel.test.ts b/tests/charging-station/broadcast-channel/ChargingStationWorkerBroadcastChannel.test.ts index a11413e5..fec1711d 100644 --- a/tests/charging-station/broadcast-channel/ChargingStationWorkerBroadcastChannel.test.ts +++ b/tests/charging-station/broadcast-channel/ChargingStationWorkerBroadcastChannel.test.ts @@ -530,6 +530,21 @@ await describe('ChargingStationWorkerBroadcastChannel', async () => { assert.strictEqual(sentRequests[0].command, RequestCommand.GET_15118_EV_CERTIFICATE) }) + await it('should dispatch CLOSE_CONNECTION as a requested (terminal) close', async t => { + const { station } = createMockStationWithRequestTracking() + + instance = new ChargingStationWorkerBroadcastChannel(station) + const testable = createTestableWorkerBroadcastChannel(instance) + const closeSpy = t.mock.method(station, 'closeWSConnection', () => undefined) + + await testable.commandHandler(BroadcastChannelProcedureName.CLOSE_CONNECTION, {}) + + // The UI disconnect must close with byRequest so onClose treats it as + // terminal and does not auto-reconnect. + assert.strictEqual(closeSpy.mock.calls.length, 1) + assert.deepStrictEqual(closeSpy.mock.calls[0].arguments, [{ byRequest: true }]) + }) + await it('should dispatch GET_CERTIFICATE_STATUS through commandHandler', async () => { const { sentRequests, station } = createMockStationWithRequestTracking() -- 2.53.0