refactor(ocpp): extract per-station state plumbing into shared OCPPIncomingRequestService base#1983
Merged
Conversation
Contributor
SummaryThe following content is AI-generated and provides a summary of the pull request: refactor(ocpp): Extract per-station state plumbing into shared
|
![hyperspace-insights[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The PR is a well-structured refactor that correctly extracts the per-station state plumbing into the base class. Three substantive issues were found: (1) the stop() template method is missing a try/finally guard, meaning a throwing resetStationState implementation would leave a stale WeakMap entry; (2) the two mock.method spy patches in the new test file lack a TestContext argument (t), so they are never automatically restored by the Node.js test runner; and (3) the test file is missing the mandatory afterEach(() => { standardCleanup() }) call required by the project's TEST_STYLE_GUIDE, risking singleton state leakage across tests. There is also a minor documentation inaccuracy where the base-class JSDoc describes stationsState as "private" while it is declared protected.
PR Bot Information
Version: 1.27.2
- Agent Instructions:
- LLM:
anthropic--claude-4.6-sonnet - Correlation ID:
97597cc0-7af0-11f1-93b1-8eb75de4adf9 - File Content Strategy: Full file content
- Event Trigger:
pull_request.opened
jerome-benoit
commented
Jul 8, 2026
jerome-benoit
force-pushed
the
refactor/ocpp-shared-per-station-state
branch
6 times, most recently
from
bd6c1bb to
f796062
Compare
…comingRequestService base Closes #1963. Companion issues #1971, #1972, #1973 will consume this base. Pure structural refactor + OCPP 1.6 concretization. Zero observable behavior change on OCPP 2.0.1: every field currently cleared in the former OCPP20IncomingRequestService.stop() body is still cleared, in the same order, at the same lifecycle point. Touchpoints: A. src/charging-station/ocpp/OCPPIncomingRequestService.ts Base becomes generic <TStationState extends object = object>. Adds shared 'stationsState' WeakMap (L86), 'getOrCreateStationState' lazy-init, concrete 'stop()' template (bound in the constructor at L94 to prevent detach-and-call regressions), and abstract hooks 'createStationState' / 'resetStationState'. Documents the exception-safety contract on the template: a throw in 'resetStationState' skips WeakMap eviction and any subclass extension after 'super.stop()' — matches pre-refactor semantics. The '= object' default on the generic parameter is load-bearing (removing it would break the static registry Map and the getInstance constraint with TS2314). B. src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts Extends OCPPIncomingRequestService<OCPP20StationState>. Removes local 'stationsState' field and 'getOrCreateStationState'. Adds 'createStationState' factory and 'resetStationState' override with the 5-statement ordering invariant of the pre-refactor stop() body. The abort-before-clear ordering matters because clearing first makes the subsequent '?.abort()' short-circuit on the nulled field, leaving the in-flight operation un-signaled. 'stop()' override calls super first, then keeps the unconditional OCPP20VariableManager cleanup. Class-level JSDoc documents OCPP 2.1 subclass path (extends OCPP20IncomingRequestService inherits state, reset, and stop() unchanged; widening the generic parameter for new fields requires a preparatory refactor with a factory cast per TS2352, or subclass override of createStationState). C. src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts Adds empty 'OCPP16StationState' interface. Extends OCPPIncomingRequestService<OCPP16StationState>. Adds no-op 'createStationState' + 'resetStationState' overrides. Removes the '/* no-op for OCPP 1.6 */' stop() override (now inherited from the base template). The 'resetStationState' JSDoc prescribes the abort-before-clear ordering companion issues MUST follow. D. eslint.config.js Adds 'no-restricted-syntax' rule enforcing the INVARIANT: forbids direct '.set/.delete/.clear' calls on 'stationsState' from outside the base class file, forbids aliasing 'stationsState' to a local binding, and forbids destructuring 'stationsState'. Covers the AST escape hatches identified during hostile-adversarial review. Enforced by 'pnpm lint' in CI on every companion PR. E. src/charging-station/ocpp/OCPPServiceUtils.ts Adds bidirectional cross-reference in the comment on 'warnedInvalidMeasurands' distinguishing the module-scope warn-once diagnostic cache from the class-scope lifecycle-state WeakMap on OCPPIncomingRequestService.stationsState. Companion issues consume this base infrastructure in order: - #1971 GetDiagnostics supersession -> activeDiagnosticsAbortController + Id - #1972 firmware setTimeout cancel -> deferred firmware timer handle - #1973 trigger cross-check -> activeFirmwareUpdateRequestId #1971 and #1973 share 'activeDiagnosticsRequestId': whichever lands first adds the (optional) field to OCPP16StationState; the second lands as-is. Line-count delta (plumbing moved, not duplicated): Base : 182 -> 290 (+108) plumbing + docs (~+80 JSDoc / +28 code) OCPP20 : 4579 -> 4627 (+48) 22 lines of plumbing removed; docs +55, net code -7 (concrete overrides only) OCPP16 : 1958 -> 1985 (+27) interface + 2 concrete implementations + companion guidance eslint : 172 -> 198 (+26) 3-selector no-restricted-syntax rule utils : 2000 -> 2004 (+4) bidirectional cross-ref comment test : new file (+150) 7 plumbing tests Origin of the OCPP 2.0.1 pattern being harmonized: - 47cdf2b refactor(ocpp20): isolate per-station state with WeakMap instead of singleton properties (introduces the WeakMap pattern with initial names 'OCPP20PerStationState' / 'stationStates' / 'getStationState') - f1e33ea refactor(ocpp20): harmonize per-station state naming (renames to 'OCPP20StationState' / 'stationsState') - 17396c1 refactor(ocpp20): rename getStationState to getOrCreateStationState (lazy-getter naming split) - c0b2555 fix(ocpp20): cancel cert-signing retry timer in stop() - be29b4c fix(ocpp20): add AbortController to log-upload lifecycle + stop() abort - d2e9b8b refactor(ocpp20): extract resetActiveFirmwareUpdateState helper - ce875da refactor(ocpp20): harmonize firmware-state cleanup + log superseded Tests: 7 new plumbing tests in tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts covering lazy-init idempotency (with createStationState spy verifying call count = 1 after two getOrCreate calls), WeakMap eviction on stop(), resetStationState invocation count, no-op guard when no state exists, two-station isolation, the OCPP 1.6 default resetStationState no-op (with reference identity check), and the exception-safety contract lock (throw in resetStationState skips WeakMap.delete, and subsequent stop() re-invokes on same state before evicting). Zero edits to existing OCPP 2.0.1 tests. The OCPP 2.0.1 5-statement ordering invariant is enforced by four independent mechanisms: (1) JSDoc rationale on OCPP20IncomingRequestService.resetStationState documenting the '?.abort()' short-circuit consequence of reordering, (2) the 'no-restricted-syntax' ESLint rule preventing direct 'stationsState' mutations from subclass code (with selector coverage for aliasing and destructuring bypasses; enforced in CI), (3) constructor 'stop.bind(this)' defense-in-depth against detach-and-call regressions, and (4) byte-identical preservation of the pre-refactor stop() body.
jerome-benoit
force-pushed
the
refactor/ocpp-shared-per-station-state
branch
from
f796062 to
de2a08a
Compare
This was referenced Jul 8, 2026
jerome-benoit
added a commit
that referenced
this pull request
Jul 8, 2026
The OCPP 1.6 UPDATE_FIRMWARE event listener schedules updateFirmwareSimulation via setTimeout(...).unref() when retrieveDate is in the future. .unref() prevents the timer from blocking process exit, but the callback still fires after stop(): if the station restarts, the deferred simulation runs against the new connection and can emit FirmwareStatusNotification messages the CSMS did not request. Store the timer handle on OCPP16StationState.deferredFirmwareUpdateTimer (per-station state introduced by #1963 / PR #1983) and release it via a shared cancelDeferredFirmwareUpdate helper called from both the schedule site (to supersede a prior pending schedule) and resetStationState (invoked by the inherited stop() template before the base deletes the WeakMap entry). The callback clears the handle before awaiting updateFirmwareSimulation so resetStationState never targets a fired timer. Mirrors the OCPP 2.0.1 pattern (OCPP20IncomingRequestService.resetStationState cancels certSigningRetryManager.cancelRetryTimer, commit c0b2555). Also corrects the pre-existing logger source identifier in both branches of the UPDATE_FIRMWARE listener from '.constructor:' to '.updateFirmwareListener:' — the callback fires at runtime, not at construction time. Closes #1972
jerome-benoit
added a commit
that referenced
this pull request
Jul 8, 2026
The OCPP 1.6 UPDATE_FIRMWARE event listener schedules updateFirmwareSimulation via setTimeout(...).unref() when retrieveDate is in the future. .unref() prevents the timer from blocking process exit, but the callback still fires after stop(): if the station restarts, the deferred simulation runs against the new connection and can emit FirmwareStatusNotification messages the CSMS did not request. Store the timer handle on OCPP16StationState.deferredFirmwareUpdateTimer (per-station state introduced by #1963 / PR #1983) and release it via a shared cancelDeferredFirmwareUpdate helper called from both the schedule site (to supersede a prior pending schedule) and resetStationState (invoked by the inherited stop() template before the base deletes the WeakMap entry). The callback clears the handle before awaiting updateFirmwareSimulation so resetStationState never targets a fired timer. Mirrors the OCPP 2.0.1 pattern (OCPP20IncomingRequestService.resetStationState cancels certSigningRetryManager.cancelRetryTimer, commit c0b2555). Closes #1972
jerome-benoit
added a commit
that referenced
this pull request
Jul 8, 2026
The OCPP 1.6 UPDATE_FIRMWARE event listener schedules updateFirmwareSimulation via setTimeout(...).unref() when retrieveDate is in the future. .unref() prevents the timer from blocking process exit, but the callback still fires after stop(): if the station restarts, the deferred simulation runs against the new connection and can emit FirmwareStatusNotification messages the CSMS did not request. Store the timer handle on OCPP16StationState.deferredFirmwareUpdateTimer (per-station state introduced by #1963 / PR #1983) and release it via a shared cancelDeferredFirmwareUpdate helper called from both the schedule site (to supersede a prior pending schedule) and resetStationState (invoked by the inherited stop() template before the base deletes the WeakMap entry). The callback clears the handle before awaiting updateFirmwareSimulation so resetStationState never targets a fired timer. Mirrors the OCPP 2.0.1 pattern (OCPP20IncomingRequestService.resetStationState cancels certSigningRetryManager.cancelRetryTimer, commit c0b2555). Closes #1972
jerome-benoit
added a commit
that referenced
this pull request
Jul 8, 2026
The OCPP 1.6 UPDATE_FIRMWARE event listener schedules updateFirmwareSimulation via setTimeout(...).unref() when retrieveDate is in the future. .unref() prevents the timer from blocking process exit, but the callback still fires after stop(): if the station restarts, the deferred simulation runs against the new connection and can emit FirmwareStatusNotification messages the CSMS did not request. Store the timer handle on OCPP16StationState.deferredFirmwareUpdateTimer (per-station state introduced by #1963 / PR #1983) and release it via a shared cancelDeferredFirmwareUpdate helper called from both the schedule site (to supersede a prior pending schedule) and resetStationState (invoked by the inherited stop() template before the base deletes the WeakMap entry). The callback clears the handle before awaiting updateFirmwareSimulation so resetStationState never targets a fired timer. Mirrors the OCPP 2.0.1 pattern (OCPP20IncomingRequestService.resetStationState cancels certSigningRetryManager.cancelRetryTimer, commit c0b2555). Closes #1972
jerome-benoit
added a commit
that referenced
this pull request
Jul 8, 2026
The OCPP 1.6 UPDATE_FIRMWARE event listener schedules updateFirmwareSimulation via setTimeout(...).unref() when retrieveDate is in the future. .unref() prevents the timer from blocking process exit, but the callback still fires after stop(): if the station restarts, the deferred simulation runs against the new connection and can emit FirmwareStatusNotification messages the CSMS did not request. Store the timer handle on OCPP16StationState.deferredFirmwareUpdateTimer (per-station state introduced by #1963 / PR #1983) and release it via a shared cancelDeferredFirmwareUpdate helper called from both the schedule site (to supersede a prior pending schedule) and resetStationState (invoked by the inherited stop() template before the base deletes the WeakMap entry). The callback clears the handle before awaiting updateFirmwareSimulation so resetStationState never targets a fired timer. Mirrors the OCPP 2.0.1 pattern (OCPP20IncomingRequestService.resetStationState cancels certSigningRetryManager.cancelRetryTimer, commit c0b2555). Closes #1972
jerome-benoit
added a commit
that referenced
this pull request
Jul 8, 2026
) The OCPP 1.6 UPDATE_FIRMWARE event listener schedules updateFirmwareSimulation via setTimeout(...).unref() when retrieveDate is in the future. .unref() prevents the timer from blocking process exit, but the callback still fires after stop(): if the station restarts, the deferred simulation runs against the new connection and can emit FirmwareStatusNotification messages the CSMS did not request. Store the timer handle on OCPP16StationState.deferredFirmwareUpdateTimer (per-station state introduced by #1963 / PR #1983) and release it via a shared cancelDeferredFirmwareUpdate helper called from both the schedule site (to supersede a prior pending schedule) and resetStationState (invoked by the inherited stop() template before the base deletes the WeakMap entry). The callback clears the handle before awaiting updateFirmwareSimulation so resetStationState never targets a fired timer. Mirrors the OCPP 2.0.1 pattern (OCPP20IncomingRequestService.resetStationState cancels certSigningRetryManager.cancelRetryTimer, commit c0b2555). Closes #1972
jerome-benoit
added a commit
that referenced
this pull request
Jul 9, 2026
…ction Closes #1970 OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry timers, or EventEmitter-dispatched `.on(...)` listeners could run AFTER `stop()` and resurrect the `stationsState` WeakMap entry via `getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also scheduled a retry `setTimeout(...)` whose handle was never stored, so `stop()` had no way to cancel a pending retry, and the recursive retry callback re-entered `sendQueuedSecurityEvents` — resurrecting state. Three coordinated parts. Part 1 — base plumbing (`OCPPIncomingRequestService.ts`): - Generic bound widened to `TStationState extends { stopped?: boolean } = { stopped?: boolean }`. - `stop()` template: after `resetStationState`, marks `stationState.stopped = true` instead of `stationsState.delete(cs)`. Deletion would re-enable resurrection via `getOrCreateStationState` lazy-init on any late dispatch. Keeping the sealed entry lets the getter return the sealed state and raw `.get()` null-guarded callers observe the marker and drop. The WeakMap entry is naturally collected when the ChargingStation reference is dropped. - `getOrCreateStationState` returns the sealed stopped state when `state?.stopped === true`; no fresh entry is lazy-init'd post-stop. - Both concrete state interfaces (`OCPP16StationState`, `OCPP20StationState`) get an alphabetized `stopped?: boolean` field. Part 2 — OCPP 2.0.1 call-site audit (11 sites): Two sites converted to raw `stationsState.get(cs)` + `stopped === true` null-guard, silent-drop: - `sendNotifyReportRequest` — dispatched from `.on(GET_BASE_REPORT).catch(...)` microtask; state usually created by sibling `handleRequestGetBaseReport` but external `emit(...)` paths and post-stop dispatch are covered by the null-guard. - `sendQueuedSecurityEvents` — dispatched from unref'd retry `setTimeout`; guard is the last line of defense against a fired-before-cancel race. Four sites use `getOrCreateStationState` + explicit `stopped` check (state may not exist yet on the pre-stop happy path): - `sendSecurityEventNotification` — lifecycle-entry on the invalid-cert-on-first-request edge case (`handleRequestCertificateSigned` X.509-invalid branch, `handleRequestUpdateFirmware` invalid-PEM branch). - `getCertSigningRetryManager` — public accessor; return type widened to `OCPP20CertSigningRetryManager | undefined`; two callers (`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated with `?.` to tolerate the undefined return. Without this guard, a late `handleResponseSignCertificate` dispatch (a SignCertificate response can arrive between `ocppIncomingRequestService.stop(cs)` and `closeWSConnection()` in `ChargingStation.stop()`) would materialize a fresh manager on the sealed state and schedule a non-`.unref()`'d retry `setTimeout` holding the ChargingStation reference. - `simulateFirmwareUpdateLifecycle` — dispatched from `.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving the full async lifecycle on a sealed state (which would send OCPP requests through a closed WebSocket and clobber the abort-controller fields). - `simulateLogUploadLifecycle` — symmetric, `.on(GET_LOG).catch(...)`. Five sites kept unchanged (Part 1 guard covers them): `getRestoredConnectorStatus`, `handleRequestGetBaseReport`, `handleRequestUpdateFirmware`, `savePreInoperativeStatuses`, `sendFirmwareStatusNotification`. Part 3 — retry timer discipline in `sendQueuedSecurityEvents`: - Store the retry `setTimeout` handle on `OCPP20StationState.securityEventRetryTimer`. - Cancel any previously scheduled retry before scheduling a new one. - Self-clear the handle before the recursive `sendQueuedSecurityEvents` call inside the callback. - New `cancelSecurityEventRetryTimer` helper is called from `resetStationState` (after abort-controller aborts and cert-signing retry cancel, before the two `resetActive*State` helpers). - `.unref()` preserved. Files touched (5 modified + 1 new): - `src/charging-station/ocpp/OCPPIncomingRequestService.ts` - `src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20ResponseService.ts` - `tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts` - `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-PostStopResurrection.test.ts` (new) Design rationale — silent-drop over `StationStoppedError` throw: matches the "post-stop silently drop" semantic and avoids catch-block sprawl at every handler. Design rationale — return `undefined` from `getCertSigningRetryManager` over convert-with-explicit-null-manager: two callers already handle optional chaining cleanly; return-type widening is the minimum change. Verification: pnpm format / typecheck / lint pass; pnpm test 3029 pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in `src/`; `securityEventRetryTimer` lives in exactly three logical places (interface field, retry-site store + self-clear, cancel helper called from `resetStationState`). References: #1983 (shared base plumbing), #1984 (OCPP 1.6 `deferredFirmwareUpdateTimer` timer discipline).
jerome-benoit
added a commit
that referenced
this pull request
Jul 9, 2026
…ction Closes #1970 OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry timers, or EventEmitter-dispatched `.on(...)` listeners could run AFTER `stop()` and resurrect the `stationsState` WeakMap entry via `getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also scheduled a retry `setTimeout(...)` whose handle was never stored, so `stop()` had no way to cancel a pending retry, and the recursive retry callback re-entered `sendQueuedSecurityEvents` — resurrecting state. Three coordinated parts. Part 1 — base plumbing (`OCPPIncomingRequestService.ts`): - Generic bound widened to `TStationState extends { stopped?: boolean } = { stopped?: boolean }`. - `stop()` template: after `resetStationState`, marks `stationState.stopped = true` instead of `stationsState.delete(cs)`. Deletion would re-enable resurrection via `getOrCreateStationState` lazy-init on any late dispatch. Keeping the sealed entry lets the getter return the sealed state and raw `.get()` null-guarded callers observe the marker and drop. The WeakMap entry is naturally collected when the ChargingStation reference is dropped. - `getOrCreateStationState` returns the sealed stopped state when `state?.stopped === true`; no fresh entry is lazy-init'd post-stop. - Both concrete state interfaces (`OCPP16StationState`, `OCPP20StationState`) get an alphabetized `stopped?: boolean` field. Part 2 — OCPP 2.0.1 call-site audit (11 sites): Two sites converted to raw `stationsState.get(cs)` + `stopped === true` null-guard, silent-drop: - `sendNotifyReportRequest` — dispatched from `.on(GET_BASE_REPORT).catch(...)` microtask; state usually created by sibling `handleRequestGetBaseReport` but external `emit(...)` paths and post-stop dispatch are covered by the null-guard. - `sendQueuedSecurityEvents` — dispatched from unref'd retry `setTimeout`; guard is the last line of defense against a fired-before-cancel race. Four sites use `getOrCreateStationState` + explicit `stopped` check (state may not exist yet on the pre-stop happy path): - `sendSecurityEventNotification` — lifecycle-entry on the invalid-cert-on-first-request edge case (`handleRequestCertificateSigned` X.509-invalid branch, `handleRequestUpdateFirmware` invalid-PEM branch). - `getCertSigningRetryManager` — public accessor; return type widened to `OCPP20CertSigningRetryManager | undefined`; two callers (`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated with `?.` to tolerate the undefined return. Without this guard, a late `handleResponseSignCertificate` dispatch (a SignCertificate response can arrive between `ocppIncomingRequestService.stop(cs)` and `closeWSConnection()` in `ChargingStation.stop()`) would materialize a fresh manager on the sealed state and schedule a non-`.unref()`'d retry `setTimeout` holding the ChargingStation reference. - `simulateFirmwareUpdateLifecycle` — dispatched from `.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving the full async lifecycle on a sealed state (which would send OCPP requests through a closed WebSocket and clobber the abort-controller fields). - `simulateLogUploadLifecycle` — symmetric, `.on(GET_LOG).catch(...)`. Five sites kept unchanged (Part 1 guard covers them): `getRestoredConnectorStatus`, `handleRequestGetBaseReport`, `handleRequestUpdateFirmware`, `savePreInoperativeStatuses`, `sendFirmwareStatusNotification`. Part 3 — retry timer discipline in `sendQueuedSecurityEvents`: - Store the retry `setTimeout` handle on `OCPP20StationState.securityEventRetryTimer`. - Cancel any previously scheduled retry before scheduling a new one. - Self-clear the handle before the recursive `sendQueuedSecurityEvents` call inside the callback. - New `cancelSecurityEventRetryTimer` helper is called from `resetStationState` (after abort-controller aborts and cert-signing retry cancel, before the two `resetActive*State` helpers). - `.unref()` preserved. Files touched (5 modified + 1 new): - `src/charging-station/ocpp/OCPPIncomingRequestService.ts` - `src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20ResponseService.ts` - `tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts` - `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-PostStopResurrection.test.ts` (new) Design rationale — silent-drop over `StationStoppedError` throw: matches the "post-stop silently drop" semantic and avoids catch-block sprawl at every handler. Design rationale — return `undefined` from `getCertSigningRetryManager` over convert-with-explicit-null-manager: two callers already handle optional chaining cleanly; return-type widening is the minimum change. Verification: pnpm format / typecheck / lint pass; pnpm test 3029 pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in `src/`; `securityEventRetryTimer` lives in exactly three logical places (interface field, retry-site store + self-clear, cancel helper called from `resetStationState`). References: #1983 (shared base plumbing), #1984 (OCPP 1.6 `deferredFirmwareUpdateTimer` timer discipline).
jerome-benoit
added a commit
that referenced
this pull request
Jul 9, 2026
…ction Closes #1970 OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry timers, or EventEmitter-dispatched `.on(...)` listeners could run AFTER `stop()` and resurrect the `stationsState` WeakMap entry via `getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also scheduled a retry `setTimeout(...)` whose handle was never stored, so `stop()` had no way to cancel a pending retry, and the recursive retry callback re-entered `sendQueuedSecurityEvents` — resurrecting state. Three coordinated parts. Part 1 — base plumbing (`OCPPIncomingRequestService.ts`): - Generic bound widened to `TStationState extends { stopped?: boolean } = { stopped?: boolean }`. - `stop()` template: after `resetStationState`, marks `stationState.stopped = true` instead of `stationsState.delete(cs)`. Deletion would re-enable resurrection via `getOrCreateStationState` lazy-init on any late dispatch. Keeping the sealed entry lets the getter return the sealed state and raw `.get()` null-guarded callers observe the marker and drop. The WeakMap entry is naturally collected when the ChargingStation reference is dropped. - `getOrCreateStationState` returns the sealed stopped state when `state?.stopped === true`; no fresh entry is lazy-init'd post-stop. - Both concrete state interfaces (`OCPP16StationState`, `OCPP20StationState`) get an alphabetized `stopped?: boolean` field. Part 2 — OCPP 2.0.1 call-site audit (11 sites): Two sites converted to raw `stationsState.get(cs)` + `stopped === true` null-guard, silent-drop: - `sendNotifyReportRequest` — dispatched from `.on(GET_BASE_REPORT).catch(...)` microtask; state usually created by sibling `handleRequestGetBaseReport` but external `emit(...)` paths and post-stop dispatch are covered by the null-guard. - `sendQueuedSecurityEvents` — dispatched from unref'd retry `setTimeout`; guard is the last line of defense against a fired-before-cancel race. Four sites use `getOrCreateStationState` + explicit `stopped` check (state may not exist yet on the pre-stop happy path): - `sendSecurityEventNotification` — lifecycle-entry on the invalid-cert-on-first-request edge case (`handleRequestCertificateSigned` X.509-invalid branch, `handleRequestUpdateFirmware` invalid-PEM branch). - `getCertSigningRetryManager` — public accessor; return type widened to `OCPP20CertSigningRetryManager | undefined`; two callers (`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated with `?.` to tolerate the undefined return. Without this guard, a late `handleResponseSignCertificate` dispatch (a SignCertificate response can arrive between `ocppIncomingRequestService.stop(cs)` and `closeWSConnection()` in `ChargingStation.stop()`) would materialize a fresh manager on the sealed state and schedule a non-`.unref()`'d retry `setTimeout` holding the ChargingStation reference. - `simulateFirmwareUpdateLifecycle` — dispatched from `.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving the full async lifecycle on a sealed state (which would send OCPP requests through a closed WebSocket and clobber the abort-controller fields). - `simulateLogUploadLifecycle` — symmetric, `.on(GET_LOG).catch(...)`. Five sites kept unchanged (Part 1 guard covers them): `getRestoredConnectorStatus`, `handleRequestGetBaseReport`, `handleRequestUpdateFirmware`, `savePreInoperativeStatuses`, `sendFirmwareStatusNotification`. Part 3 — retry timer discipline in `sendQueuedSecurityEvents`: - Store the retry `setTimeout` handle on `OCPP20StationState.securityEventRetryTimer`. - Cancel any previously scheduled retry before scheduling a new one. - Self-clear the handle before the recursive `sendQueuedSecurityEvents` call inside the callback. - New `cancelSecurityEventRetryTimer` helper is called from `resetStationState` (after abort-controller aborts and cert-signing retry cancel, before the two `resetActive*State` helpers). - `.unref()` preserved. Files touched (6 modified + 1 new): - `src/charging-station/ocpp/OCPPIncomingRequestService.ts` - `src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20ResponseService.ts` - `tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts` - `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-PostStopResurrection.test.ts` (new) Design rationale — silent-drop over `StationStoppedError` throw: matches the "post-stop silently drop" semantic and avoids catch-block sprawl at every handler. Design rationale — return `undefined` from `getCertSigningRetryManager` over convert-with-explicit-null-manager: two callers already handle optional chaining cleanly; return-type widening is the minimum change. Verification: pnpm format / typecheck / lint pass; pnpm test 3029 pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in `src/`; `securityEventRetryTimer` lives in exactly three logical places (interface field, retry-site store + self-clear, cancel helper called from `resetStationState`). References: #1983 (shared base plumbing), #1984 (OCPP 1.6 `deferredFirmwareUpdateTimer` timer discipline).
jerome-benoit
added a commit
that referenced
this pull request
Jul 9, 2026
…ction Closes #1970 OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry timers, or EventEmitter-dispatched `.on(...)` listeners could run AFTER `stop()` and resurrect the `stationsState` WeakMap entry via `getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also scheduled a retry `setTimeout(...)` whose handle was never stored, so `stop()` had no way to cancel a pending retry, and the recursive retry callback re-entered `sendQueuedSecurityEvents` — resurrecting state. Three coordinated parts. Part 1 — base plumbing (`OCPPIncomingRequestService.ts`): - Generic bound widened to `TStationState extends { stopped?: boolean } = { stopped?: boolean }`. - `stop()` template: after `resetStationState`, marks `stationState.stopped = true` instead of `stationsState.delete(cs)`. Deletion would re-enable resurrection via `getOrCreateStationState` lazy-init on any late dispatch. Keeping the sealed entry lets the getter return the sealed state and raw `.get()` null-guarded callers observe the marker and drop. The WeakMap entry is naturally collected when the ChargingStation reference is dropped. - `getOrCreateStationState` returns the sealed stopped state when `state?.stopped === true`; no fresh entry is lazy-init'd post-stop. - Both concrete state interfaces (`OCPP16StationState`, `OCPP20StationState`) get an alphabetized `stopped?: boolean` field. Part 2 — OCPP 2.0.1 call-site audit (11 sites): Two sites converted to raw `stationsState.get(cs)` + `stopped === true` null-guard, silent-drop: - `sendNotifyReportRequest` — dispatched from `.on(GET_BASE_REPORT).catch(...)` microtask; state usually created by sibling `handleRequestGetBaseReport` but external `emit(...)` paths and post-stop dispatch are covered by the null-guard. - `sendQueuedSecurityEvents` — dispatched from unref'd retry `setTimeout`; guard is the last line of defense against a fired-before-cancel race. Four sites use `getOrCreateStationState` + explicit `stopped` check (state may not exist yet on the pre-stop happy path): - `sendSecurityEventNotification` — lifecycle-entry on the invalid-cert-on-first-request edge case (`handleRequestCertificateSigned` X.509-invalid branch, `handleRequestUpdateFirmware` invalid-PEM branch). - `getCertSigningRetryManager` — public accessor; return type widened to `OCPP20CertSigningRetryManager | undefined`; two callers (`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated with `?.` to tolerate the undefined return. Without this guard, a late `handleResponseSignCertificate` dispatch (a SignCertificate response can arrive between `ocppIncomingRequestService.stop(cs)` and `closeWSConnection()` in `ChargingStation.stop()`) would materialize a fresh manager on the sealed state and schedule a non-`.unref()`'d retry `setTimeout` holding the ChargingStation reference. - `simulateFirmwareUpdateLifecycle` — dispatched from `.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving the full async lifecycle on a sealed state (which would send OCPP requests through a closed WebSocket and clobber the abort-controller fields). - `simulateLogUploadLifecycle` — symmetric, `.on(GET_LOG).catch(...)`. Five sites kept unchanged (Part 1 guard covers them): `getRestoredConnectorStatus`, `handleRequestGetBaseReport`, `handleRequestUpdateFirmware`, `savePreInoperativeStatuses`, `sendFirmwareStatusNotification`. Part 3 — retry timer discipline in `sendQueuedSecurityEvents`: - Store the retry `setTimeout` handle on `OCPP20StationState.securityEventRetryTimer`. - Cancel any previously scheduled retry before scheduling a new one. - Self-clear the handle before the recursive `sendQueuedSecurityEvents` call inside the callback. - New `cancelSecurityEventRetryTimer` helper is called from `resetStationState` (after abort-controller aborts and cert-signing retry cancel, before the two `resetActive*State` helpers). - `.unref()` preserved. Files touched (6 modified + 1 new): - `src/charging-station/ocpp/OCPPIncomingRequestService.ts` - `src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20ResponseService.ts` - `tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts` - `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-PostStopResurrection.test.ts` (new) Design rationale — silent-drop over `StationStoppedError` throw: matches the "post-stop silently drop" semantic and avoids catch-block sprawl at every handler. Design rationale — return `undefined` from `getCertSigningRetryManager` over convert-with-explicit-null-manager: two callers already handle optional chaining cleanly; return-type widening is the minimum change. Verification: pnpm format / typecheck / lint pass; pnpm test 3029 pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in `src/`; `securityEventRetryTimer` lives in exactly three logical places (interface field, retry-site store + self-clear, cancel helper called from `resetStationState`). References: #1983 (shared base plumbing), #1984 (OCPP 1.6 `deferredFirmwareUpdateTimer` timer discipline).
jerome-benoit
added a commit
that referenced
this pull request
Jul 9, 2026
…ction Closes #1970 OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry timers, or EventEmitter-dispatched `.on(...)` listeners could run AFTER `stop()` and resurrect the `stationsState` WeakMap entry via `getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also scheduled a retry `setTimeout(...)` whose handle was never stored, so `stop()` had no way to cancel a pending retry, and the recursive retry callback re-entered `sendQueuedSecurityEvents` — resurrecting state. Three coordinated parts. Part 1 — base plumbing (`OCPPIncomingRequestService.ts`): - Generic bound widened to `TStationState extends { stopped?: boolean } = { stopped?: boolean }`. - `stop()` template: after `resetStationState`, marks `stationState.stopped = true` instead of `stationsState.delete(cs)`. Deletion would re-enable resurrection via `getOrCreateStationState` lazy-init on any late dispatch. Keeping the sealed entry lets the getter return the sealed state and raw `.get()` null-guarded callers observe the marker and drop. The WeakMap entry is naturally collected when the ChargingStation reference is dropped. - `getOrCreateStationState` returns the sealed stopped state when `state?.stopped === true`; no fresh entry is lazy-init'd post-stop. - Both concrete state interfaces (`OCPP16StationState`, `OCPP20StationState`) get an alphabetized `stopped?: boolean` field. Part 2 — OCPP 2.0.1 call-site audit (11 sites): Two sites converted to raw `stationsState.get(cs)` + `stopped === true` null-guard, silent-drop: - `sendNotifyReportRequest` — dispatched from `.on(GET_BASE_REPORT).catch(...)` microtask; state usually created by sibling `handleRequestGetBaseReport` but external `emit(...)` paths and post-stop dispatch are covered by the null-guard. - `sendQueuedSecurityEvents` — dispatched from unref'd retry `setTimeout`; guard is the last line of defense against a fired-before-cancel race. Four sites use `getOrCreateStationState` + explicit `stopped` check (state may not exist yet on the pre-stop happy path): - `sendSecurityEventNotification` — lifecycle-entry on the invalid-cert-on-first-request edge case (`handleRequestCertificateSigned` X.509-invalid branch, `handleRequestUpdateFirmware` invalid-PEM branch). - `getCertSigningRetryManager` — public accessor; return type widened to `OCPP20CertSigningRetryManager | undefined`; two callers (`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated with `?.` to tolerate the undefined return. Without this guard, a late `handleResponseSignCertificate` dispatch (a SignCertificate response can arrive between `ocppIncomingRequestService.stop(cs)` and `closeWSConnection()` in `ChargingStation.stop()`) would materialize a fresh manager on the sealed state and schedule a non-`.unref()`'d retry `setTimeout` holding the ChargingStation reference. - `simulateFirmwareUpdateLifecycle` — dispatched from `.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving the full async lifecycle on a sealed state (which would send OCPP requests through a closed WebSocket and clobber the abort-controller fields). - `simulateLogUploadLifecycle` — symmetric, `.on(GET_LOG).catch(...)`. Five sites kept unchanged (Part 1 guard covers them): `getRestoredConnectorStatus`, `handleRequestGetBaseReport`, `handleRequestUpdateFirmware`, `savePreInoperativeStatuses`, `sendFirmwareStatusNotification`. Part 3 — retry timer discipline in `sendQueuedSecurityEvents`: - Store the retry `setTimeout` handle on `OCPP20StationState.securityEventRetryTimer`. - Cancel any previously scheduled retry before scheduling a new one. - Self-clear the handle before the recursive `sendQueuedSecurityEvents` call inside the callback. - New `cancelSecurityEventRetryTimer` helper is called from `resetStationState` (after abort-controller aborts and cert-signing retry cancel, before the two `resetActive*State` helpers). - `.unref()` preserved. Files touched (6 modified + 1 new): - `src/charging-station/ocpp/OCPPIncomingRequestService.ts` - `src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20ResponseService.ts` - `tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts` - `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-PostStopResurrection.test.ts` (new) Design rationale — silent-drop over `StationStoppedError` throw: matches the "post-stop silently drop" semantic and avoids catch-block sprawl at every handler. Design rationale — return `undefined` from `getCertSigningRetryManager` over convert-with-explicit-null-manager: two callers already handle optional chaining cleanly; return-type widening is the minimum change. Verification: pnpm format / typecheck / lint pass; pnpm test 3029 pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in `src/`; `securityEventRetryTimer` lives in exactly three logical places (interface field, retry-site store + self-clear, cancel helper called from `resetStationState`). References: #1983 (shared base plumbing), #1984 (OCPP 1.6 `deferredFirmwareUpdateTimer` timer discipline).
jerome-benoit
added a commit
that referenced
this pull request
Jul 9, 2026
…ction Closes #1970 OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry timers, or EventEmitter-dispatched `.on(...)` listeners could run AFTER `stop()` and resurrect the `stationsState` WeakMap entry via `getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also scheduled a retry `setTimeout(...)` whose handle was never stored, so `stop()` had no way to cancel a pending retry, and the recursive retry callback re-entered `sendQueuedSecurityEvents` — resurrecting state. Three coordinated parts. Part 1 — base plumbing (`OCPPIncomingRequestService.ts`): - Generic bound widened to `TStationState extends { stopped?: boolean } = { stopped?: boolean }`. - `stop()` template: after `resetStationState`, marks `stationState.stopped = true` instead of `stationsState.delete(cs)`. Deletion would re-enable resurrection via `getOrCreateStationState` lazy-init on any late dispatch. Keeping the sealed entry lets the getter return the sealed state and raw `.get()` null-guarded callers observe the marker and drop. The WeakMap entry is naturally collected when the ChargingStation reference is dropped. - `getOrCreateStationState` returns the sealed stopped state when `state?.stopped === true`; no fresh entry is lazy-init'd post-stop. - Both concrete state interfaces (`OCPP16StationState`, `OCPP20StationState`) get an alphabetized `stopped?: boolean` field. Part 2 — OCPP 2.0.1 call-site audit (11 sites): Two sites converted to raw `stationsState.get(cs)` + `stopped === true` null-guard, silent-drop: - `sendNotifyReportRequest` — dispatched from `.on(GET_BASE_REPORT).catch(...)` microtask; state usually created by sibling `handleRequestGetBaseReport` but external `emit(...)` paths and post-stop dispatch are covered by the null-guard. - `sendQueuedSecurityEvents` — dispatched from unref'd retry `setTimeout`; guard is the last line of defense against a fired-before-cancel race. Four sites use `getOrCreateStationState` + explicit `stopped` check (state may not exist yet on the pre-stop happy path): - `sendSecurityEventNotification` — lifecycle-entry on the invalid-cert-on-first-request edge case (`handleRequestCertificateSigned` X.509-invalid branch, `handleRequestUpdateFirmware` invalid-PEM branch). - `getCertSigningRetryManager` — public accessor; return type widened to `OCPP20CertSigningRetryManager | undefined`; two callers (`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated with `?.` to tolerate the undefined return. Without this guard, a late `handleResponseSignCertificate` dispatch (a SignCertificate response can arrive between `ocppIncomingRequestService.stop(cs)` and `closeWSConnection()` in `ChargingStation.stop()`) would materialize a fresh manager on the sealed state and schedule a non-`.unref()`'d retry `setTimeout` holding the ChargingStation reference. - `simulateFirmwareUpdateLifecycle` — dispatched from `.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving the full async lifecycle on a sealed state (which would send OCPP requests through a closed WebSocket and clobber the abort-controller fields). - `simulateLogUploadLifecycle` — symmetric, `.on(GET_LOG).catch(...)`. Five sites kept unchanged (Part 1 guard covers them): `getRestoredConnectorStatus`, `handleRequestGetBaseReport`, `handleRequestUpdateFirmware`, `savePreInoperativeStatuses`, `sendFirmwareStatusNotification`. Part 3 — retry timer discipline in `sendQueuedSecurityEvents`: - Store the retry `setTimeout` handle on `OCPP20StationState.securityEventRetryTimer`. - Cancel any previously scheduled retry before scheduling a new one. - Self-clear the handle before the recursive `sendQueuedSecurityEvents` call inside the callback. - New `cancelSecurityEventRetryTimer` helper is called from `resetStationState` (after abort-controller aborts and cert-signing retry cancel, before the two `resetActive*State` helpers). - `.unref()` preserved. Files touched (7 modified + 1 new): - `src/charging-station/ocpp/OCPPIncomingRequestService.ts` - `src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20ResponseService.ts` - `tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-GetDiagnostics.test.ts` - `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-PostStopResurrection.test.ts` (new) Design rationale — silent-drop over `StationStoppedError` throw: matches the "post-stop silently drop" semantic and avoids catch-block sprawl at every handler. Design rationale — return `undefined` from `getCertSigningRetryManager` over convert-with-explicit-null-manager: two callers already handle optional chaining cleanly; return-type widening is the minimum change. Verification: pnpm format / typecheck / lint pass; pnpm test 3029 pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in `src/`; `securityEventRetryTimer` lives in exactly three logical places (interface field, retry-site store + self-clear, cancel helper called from `resetStationState`). References: #1983 (shared base plumbing), #1984 (OCPP 1.6 `deferredFirmwareUpdateTimer` timer discipline).
jerome-benoit
added a commit
that referenced
this pull request
Jul 9, 2026
…ction Closes #1970 OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry timers, or EventEmitter-dispatched `.on(...)` listeners could run AFTER `stop()` and resurrect the `stationsState` WeakMap entry via `getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also scheduled a retry `setTimeout(...)` whose handle was never stored, so `stop()` had no way to cancel a pending retry, and the recursive retry callback re-entered `sendQueuedSecurityEvents` — resurrecting state. Three coordinated parts. Part 1 — base plumbing (`OCPPIncomingRequestService.ts`): - Generic bound widened to `TStationState extends { stopped?: boolean } = { stopped?: boolean }`. - `stop()` template: after `resetStationState`, marks `stationState.stopped = true` instead of `stationsState.delete(cs)`. Deletion would re-enable resurrection via `getOrCreateStationState` lazy-init on any late dispatch. Keeping the sealed entry lets the getter return the sealed state and raw `.get()` null-guarded callers observe the marker and drop. The WeakMap entry is naturally collected when the ChargingStation reference is dropped. - `getOrCreateStationState` returns the sealed stopped state when `state?.stopped === true`; no fresh entry is lazy-init'd post-stop. - Both concrete state interfaces (`OCPP16StationState`, `OCPP20StationState`) get an alphabetized `stopped?: boolean` field. Part 2 — OCPP 2.0.1 call-site audit (11 sites): Two sites converted to raw `stationsState.get(cs)` + `stopped === true` null-guard, silent-drop: - `sendNotifyReportRequest` — dispatched from `.on(GET_BASE_REPORT).catch(...)` microtask; state usually created by sibling `handleRequestGetBaseReport` but external `emit(...)` paths and post-stop dispatch are covered by the null-guard. - `sendQueuedSecurityEvents` — dispatched from unref'd retry `setTimeout`; guard is the last line of defense against a fired-before-cancel race. Four sites use `getOrCreateStationState` + explicit `stopped` check (state may not exist yet on the pre-stop happy path): - `sendSecurityEventNotification` — lifecycle-entry on the invalid-cert-on-first-request edge case (`handleRequestCertificateSigned` X.509-invalid branch, `handleRequestUpdateFirmware` invalid-PEM branch). - `getCertSigningRetryManager` — public accessor; return type widened to `OCPP20CertSigningRetryManager | undefined`; two callers (`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated with `?.` to tolerate the undefined return. Without this guard, a late `handleResponseSignCertificate` dispatch (a SignCertificate response can arrive between `ocppIncomingRequestService.stop(cs)` and `closeWSConnection()` in `ChargingStation.stop()`) would materialize a fresh manager on the sealed state and schedule a non-`.unref()`'d retry `setTimeout` holding the ChargingStation reference. - `simulateFirmwareUpdateLifecycle` — dispatched from `.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving the full async lifecycle on a sealed state (which would send OCPP requests through a closed WebSocket and clobber the abort-controller fields). - `simulateLogUploadLifecycle` — symmetric, `.on(GET_LOG).catch(...)`. Five sites kept unchanged (Part 1 guard covers them): `getRestoredConnectorStatus`, `handleRequestGetBaseReport`, `handleRequestUpdateFirmware`, `savePreInoperativeStatuses`, `sendFirmwareStatusNotification`. Part 3 — retry timer discipline in `sendQueuedSecurityEvents`: - Store the retry `setTimeout` handle on `OCPP20StationState.securityEventRetryTimer`. - Cancel any previously scheduled retry before scheduling a new one. - Self-clear the handle before the recursive `sendQueuedSecurityEvents` call inside the callback. - New `cancelSecurityEventRetryTimer` helper is called from `resetStationState` (after abort-controller aborts and cert-signing retry cancel, before the two `resetActive*State` helpers). - `.unref()` preserved. Files touched (7 modified + 1 new): - `src/charging-station/ocpp/OCPPIncomingRequestService.ts` - `src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20ResponseService.ts` - `tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-GetDiagnostics.test.ts` - `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-PostStopResurrection.test.ts` (new) Design rationale — silent-drop over `StationStoppedError` throw: matches the "post-stop silently drop" semantic and avoids catch-block sprawl at every handler. Design rationale — return `undefined` from `getCertSigningRetryManager` over convert-with-explicit-null-manager: two callers already handle optional chaining cleanly; return-type widening is the minimum change. Verification: pnpm format / typecheck / lint pass; pnpm test 3029 pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in `src/`; `securityEventRetryTimer` lives in exactly three logical places (interface field, retry-site store + self-clear, cancel helper called from `resetStationState`). References: #1983 (shared base plumbing), #1984 (OCPP 1.6 `deferredFirmwareUpdateTimer` timer discipline).
jerome-benoit
added a commit
that referenced
this pull request
Jul 9, 2026
…ction Closes #1970 OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry timers, or EventEmitter-dispatched `.on(...)` listeners could run AFTER `stop()` and resurrect the `stationsState` WeakMap entry via `getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also scheduled a retry `setTimeout(...)` whose handle was never stored, so `stop()` had no way to cancel a pending retry, and the recursive retry callback re-entered `sendQueuedSecurityEvents` — resurrecting state. Three coordinated parts. Part 1 — base plumbing (`OCPPIncomingRequestService.ts`): - Generic bound widened to `TStationState extends { stopped?: boolean } = { stopped?: boolean }`. - `stop()` template: after `resetStationState`, marks `stationState.stopped = true` instead of `stationsState.delete(cs)`. Deletion would re-enable resurrection via `getOrCreateStationState` lazy-init on any late dispatch. Keeping the sealed entry lets the getter return the sealed state and raw `.get()` null-guarded callers observe the marker and drop. The WeakMap entry is naturally collected when the ChargingStation reference is dropped. - `getOrCreateStationState` returns the sealed stopped state when `state?.stopped === true`; no fresh entry is lazy-init'd post-stop. - Both concrete state interfaces (`OCPP16StationState`, `OCPP20StationState`) get an alphabetized `stopped?: boolean` field. Part 2 — OCPP 2.0.1 call-site audit (11 sites): Two sites converted to raw `stationsState.get(cs)` + `stopped === true` null-guard, silent-drop: - `sendNotifyReportRequest` — dispatched from `.on(GET_BASE_REPORT).catch(...)` microtask; state usually created by sibling `handleRequestGetBaseReport` but external `emit(...)` paths and post-stop dispatch are covered by the null-guard. - `sendQueuedSecurityEvents` — dispatched from unref'd retry `setTimeout`; guard is the last line of defense against a fired-before-cancel race. Four sites use `getOrCreateStationState` + explicit `stopped` check (state may not exist yet on the pre-stop happy path): - `sendSecurityEventNotification` — lifecycle-entry on the invalid-cert-on-first-request edge case (`handleRequestCertificateSigned` X.509-invalid branch, `handleRequestUpdateFirmware` invalid-PEM branch). - `getCertSigningRetryManager` — public accessor; return type widened to `OCPP20CertSigningRetryManager | undefined`; two callers (`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated with `?.` to tolerate the undefined return. Without this guard, a late `handleResponseSignCertificate` dispatch (a SignCertificate response can arrive between `ocppIncomingRequestService.stop(cs)` and `closeWSConnection()` in `ChargingStation.stop()`) would materialize a fresh manager on the sealed state and schedule a non-`.unref()`'d retry `setTimeout` holding the ChargingStation reference. - `simulateFirmwareUpdateLifecycle` — dispatched from `.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving the full async lifecycle on a sealed state (which would send OCPP requests through a closed WebSocket and clobber the abort-controller fields). - `simulateLogUploadLifecycle` — symmetric, `.on(GET_LOG).catch(...)`. Five sites kept unchanged (Part 1 guard covers them): `getRestoredConnectorStatus`, `handleRequestGetBaseReport`, `handleRequestUpdateFirmware`, `savePreInoperativeStatuses`, `sendFirmwareStatusNotification`. Part 3 — retry timer discipline in `sendQueuedSecurityEvents`: - Store the retry `setTimeout` handle on `OCPP20StationState.securityEventRetryTimer`. - Cancel any previously scheduled retry before scheduling a new one. - Self-clear the handle before the recursive `sendQueuedSecurityEvents` call inside the callback. - New `cancelSecurityEventRetryTimer` helper is called from `resetStationState` (after abort-controller aborts and cert-signing retry cancel, before the two `resetActive*State` helpers). - `.unref()` preserved. Files touched (7 modified + 1 new): - `src/charging-station/ocpp/OCPPIncomingRequestService.ts` - `src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20ResponseService.ts` - `tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-GetDiagnostics.test.ts` - `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-PostStopResurrection.test.ts` (new) Design rationale — silent-drop over `StationStoppedError` throw: matches the "post-stop silently drop" semantic and avoids catch-block sprawl at every handler. Design rationale — return `undefined` from `getCertSigningRetryManager` over convert-with-explicit-null-manager: two callers already handle optional chaining cleanly; return-type widening is the minimum change. Verification: pnpm format / typecheck / lint pass; pnpm test 3029 pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in `src/`; `securityEventRetryTimer` lives in exactly three logical places (interface field, retry-site store + self-clear, cancel helper called from `resetStationState`). References: #1983 (shared base plumbing), #1984 (OCPP 1.6 `deferredFirmwareUpdateTimer` timer discipline).
jerome-benoit
added a commit
that referenced
this pull request
Jul 9, 2026
…ction (#1992) Closes #1970 OCPP 2.0.1 handlers dispatched from queued microtasks, unref'd retry timers, or EventEmitter-dispatched `.on(...)` listeners could run AFTER `stop()` and resurrect the `stationsState` WeakMap entry via `getOrCreateStationState` lazy-init. `sendQueuedSecurityEvents` also scheduled a retry `setTimeout(...)` whose handle was never stored, so `stop()` had no way to cancel a pending retry, and the recursive retry callback re-entered `sendQueuedSecurityEvents` — resurrecting state. Three coordinated parts. Part 1 — base plumbing (`OCPPIncomingRequestService.ts`): - Generic bound widened to `TStationState extends { stopped?: boolean } = { stopped?: boolean }`. - `stop()` template: after `resetStationState`, marks `stationState.stopped = true` instead of `stationsState.delete(cs)`. Deletion would re-enable resurrection via `getOrCreateStationState` lazy-init on any late dispatch. Keeping the sealed entry lets the getter return the sealed state and raw `.get()` null-guarded callers observe the marker and drop. The WeakMap entry is naturally collected when the ChargingStation reference is dropped. - `getOrCreateStationState` returns the sealed stopped state when `state?.stopped === true`; no fresh entry is lazy-init'd post-stop. - Both concrete state interfaces (`OCPP16StationState`, `OCPP20StationState`) get an alphabetized `stopped?: boolean` field. Part 2 — OCPP 2.0.1 call-site audit (11 sites): Two sites converted to raw `stationsState.get(cs)` + `stopped === true` null-guard, silent-drop: - `sendNotifyReportRequest` — dispatched from `.on(GET_BASE_REPORT).catch(...)` microtask; state usually created by sibling `handleRequestGetBaseReport` but external `emit(...)` paths and post-stop dispatch are covered by the null-guard. - `sendQueuedSecurityEvents` — dispatched from unref'd retry `setTimeout`; guard is the last line of defense against a fired-before-cancel race. Four sites use `getOrCreateStationState` + explicit `stopped` check (state may not exist yet on the pre-stop happy path): - `sendSecurityEventNotification` — lifecycle-entry on the invalid-cert-on-first-request edge case (`handleRequestCertificateSigned` X.509-invalid branch, `handleRequestUpdateFirmware` invalid-PEM branch). - `getCertSigningRetryManager` — public accessor; return type widened to `OCPP20CertSigningRetryManager | undefined`; two callers (`handleRequestCertificateSigned`, `OCPP20ResponseService`) updated with `?.` to tolerate the undefined return. Without this guard, a late `handleResponseSignCertificate` dispatch (a SignCertificate response can arrive between `ocppIncomingRequestService.stop(cs)` and `closeWSConnection()` in `ChargingStation.stop()`) would materialize a fresh manager on the sealed state and schedule a non-`.unref()`'d retry `setTimeout` holding the ChargingStation reference. - `simulateFirmwareUpdateLifecycle` — dispatched from `.on(UPDATE_FIRMWARE).catch(...)` microtask; guards against driving the full async lifecycle on a sealed state (which would send OCPP requests through a closed WebSocket and clobber the abort-controller fields). - `simulateLogUploadLifecycle` — symmetric, `.on(GET_LOG).catch(...)`. Five sites kept unchanged (Part 1 guard covers them): `getRestoredConnectorStatus`, `handleRequestGetBaseReport`, `handleRequestUpdateFirmware`, `savePreInoperativeStatuses`, `sendFirmwareStatusNotification`. Part 3 — retry timer discipline in `sendQueuedSecurityEvents`: - Store the retry `setTimeout` handle on `OCPP20StationState.securityEventRetryTimer`. - Cancel any previously scheduled retry before scheduling a new one. - Self-clear the handle before the recursive `sendQueuedSecurityEvents` call inside the callback. - New `cancelSecurityEventRetryTimer` helper is called from `resetStationState` (after abort-controller aborts and cert-signing retry cancel, before the two `resetActive*State` helpers). - `.unref()` preserved. Files touched (7 modified + 1 new): - `src/charging-station/ocpp/OCPPIncomingRequestService.ts` - `src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts` - `src/charging-station/ocpp/2.0/OCPP20ResponseService.ts` - `tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts` - `tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-GetDiagnostics.test.ts` - `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-PostStopResurrection.test.ts` (new) Design rationale — silent-drop over `StationStoppedError` throw: matches the "post-stop silently drop" semantic and avoids catch-block sprawl at every handler. Design rationale — return `undefined` from `getCertSigningRetryManager` over convert-with-explicit-null-manager: two callers already handle optional chaining cleanly; return-type widening is the minimum change. Verification: pnpm format / typecheck / lint pass; pnpm test 3029 pass, 0 fail; pnpm build exit 0. No `stationsState.delete` remains in `src/`; `securityEventRetryTimer` lives in exactly three logical places (interface field, retry-site store + self-clear, cancel helper called from `resetStationState`). References: #1983 (shared base plumbing), #1984 (OCPP 1.6 `deferredFirmwareUpdateTimer` timer discipline).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1963. Companion issues #1971, #1972, #1973 will consume this base.
Pure structural refactor + OCPP 1.6 concretization. Zero observable behavior change on OCPP 2.0.1: every field currently cleared in the former
OCPP20IncomingRequestService.stop()body is still cleared, in the same order, at the same lifecycle point.Design goal
Maximize code sharing between the OCPP 1.6 and OCPP 2.0.1 stacks. The
WeakMap<ChargingStation, TStationState>+ lazy-init getter +stop()template lives ONCE inOCPPIncomingRequestService, parameterized over the concrete station-state type. Version-specific state interfaces (OCPP16StationState,OCPP20StationState) stay distinct.Touchpoints
src/charging-station/ocpp/OCPPIncomingRequestService.ts<TStationState extends object = object>. Adds sharedstationsStateWeakMap (L86),getOrCreateStationStatelazy-init, concretestop()template (bound in the constructor at L94 to prevent detach-and-call regressions), and abstract hookscreateStationState/resetStationState. Thestop()template documents an exception-safety contract: ifresetStationStatethrows,WeakMapeviction is skipped and a subsequentstop()re-invokes the hook on the same state. The= objectdefault on the generic parameter is load-bearing (removing it breaks the staticinstancesregistry and thegetInstanceconstraint with TS2314 — see the base's JSDoc for details).src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.tsextends OCPPIncomingRequestService<OCPP20StationState>. Removes localstationsStatefield and localgetOrCreateStationState. AddscreateStationStatefactory andresetStationStateoverride with the 5-statement ordering invariant preserved bit-for-bit from the pre-refactorstop()body. The abort-before-clear ordering matters because clearing first would make the subsequent?.abort()short-circuit on the nulled field, leaving the in-flight operation un-signaled.stop()override callssuper.stop()first, then keeps the unconditionalOCPP20VariableManagercleanup. Class-level JSDoc documents the OCPP 2.1 subclass path (see the file's JSDoc for cast requirements).src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.tsOCPP16StationStateinterface — companion issues populate.extends OCPPIncomingRequestService<OCPP16StationState>. Adds no-opcreateStationState+resetStationStateoverrides. Removes the/* no-op for OCPP 1.6 */stop()override (now inherited from the base template). TheresetStationStateJSDoc prescribes the abort-before-clear ordering companion issues MUST follow.eslint.config.jsno-restricted-syntaxrule enforcing thestationsStateINVARIANT. Three selectors cover direct mutation (.set/.delete/.clear), aliasing (const x = something.stationsState), and destructuring (const { stationsState } = something). Enforced bypnpm lintin CI on every companion PR — attempting to bypass will fail the pipeline.src/charging-station/ocpp/OCPPServiceUtils.tswarnedInvalidMeasurands(a sibling module-scope diagnosticWeakMap<ChargingStation, Set<string>>) distinguishing it from the class-scope lifecycle-stateWeakMaponOCPPIncomingRequestService.stationsState— nostop()lifecycle, no abort semantics, noresetStationStatehook.Companion issues (tracking issues, not open PRs) consuming this base
activeDiagnosticsAbortController,activeDiagnosticsRequestId.setTimeoutcancellation onstop()→ populates the deferred firmware timer handle.TriggerMessage(DiagnosticsStatusNotification / FirmwareStatusNotification)dual-source cross-check → populatesactiveFirmwareUpdateRequestIdand readsactiveDiagnosticsRequestId(added by [BUG] OCPP 1.6: handleRequestGetDiagnostics lacks supersession semantics and abort infrastructure #1971).Coordination note: #1971 and #1973 both touch
activeDiagnosticsRequestId. Convention: whichever of the two lands first adds the field toOCPP16StationState(marked optional?: number); the second lands as-is and relies on the field being present. No merge-order dependency for the interface itself, but if #1973 lands first, its author declares the field defensively.Each companion PR-to-come only adds fields to
OCPP16StationStateand populates the currently-no-opresetStationState— zero plumbing duplication. Verified empirically: none of the three companion patterns trip theno-restricted-syntaxrule.Line-count delta (plumbing moved, not duplicated)
OCPPIncomingRequestService.ts(base)stop.bind+stoptemplate body +getOrCreateStationState+ 2 abstract signatures)OCPP20IncomingRequestService.tsOCPP16IncomingRequestService.tseslint.config.jsno-restricted-syntaxruleOCPPServiceUtils.tswarnedInvalidMeasurandstests/…/OCPPIncomingRequestService-StationState.test.tsBase and OCPP20 additions are documentation-heavy: the code footprint is smaller than the raw delta suggests. Plumbing is moved from OCPP20 to the base, not duplicated.
Behavior-preservation audit (bit-for-bit)
Pre-refactor
OCPP20IncomingRequestService.stop()executed:activeFirmwareUpdateAbortController?.abort()activeLogUploadAbortController?.abort()certSigningRetryManager?.cancelRetryTimer()resetActiveFirmwareUpdateState(state)resetActiveLogUploadState(state)stationsState.delete(chargingStation)— inside theif (stationState != null)guardvariableManager.resetRuntimeOverrides(...)+invalidateMappingsCache(...)in try/catchPost-refactor: steps 1–5 live in
OCPP20IncomingRequestService.resetStationState(invoked exactly once perstop()and only when a state entry exists). Step 6 is the base template'sstationsState.delete(invoked immediately afterresetStationStatereturns, still inside the same guard). Step 7 remains in theOCPP20IncomingRequestService.stop()override, aftersuper.stop(), still unconditional. Same order, same guards, same exception-propagation semantics.Origin of the OCPP 2.0.1 pattern being harmonized
47cdf2bbe—refactor(ocpp20): isolate per-station state with WeakMap instead of singleton properties(introduces the WeakMap pattern).f1e33ea42—refactor(ocpp20): harmonize per-station state naming(renames toOCPP20StationState/stationsState).17396c1c4—refactor(ocpp20): rename getStationState to getOrCreateStationState(lazy-getter naming split).c0b25553—fix(ocpp20): cancel cert-signing retry timer in stop().be29b4c8—fix(ocpp20): add AbortController to log-upload lifecycle + stop() abort.d2e9b8b0—refactor(ocpp20): extract resetActiveFirmwareUpdateState helper.ce875dae—refactor(ocpp20): harmonize firmware-state cleanup + log superseded.Tests
New file:
tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts— 7 targeted plumbing tests:mock.methodspy oncreateStationState, verifiescallCount === 1after twogetOrCreateStationStatecalls (proves laziness, not just WeakMap memoization).stop()deletes the WeakMap entry.stop()callsresetStationStateexactly once with the correct state reference.stop()is a no-op when the state was never created.stop(A)does not affect B.resetStationStateperforms no observable mutation + reference identity is preserved after the call.resetStationStateskipsWeakMap.delete, and a subsequentstop()re-invokes the hook on the same state before evicting.Follows
tests/TEST_STYLE_GUIDE.md:afterEach(() => { standardCleanup() })for canonical mock restoration viamock.restoreAll().Zero edits to existing tests. Full suite: 0 fail, 7 new passing tests, all pre-existing OCPP 2.0.1 stop/supersession/trigger tests green.
Verification gates
pnpm formatcleanpnpm typecheckexit 0pnpm lint0 errors (expandedno-restricted-syntaxrule doesn't false-positive on any existing code — empirically confirms the invariant is upheld codebase-wide; also enforced in CI on companion PRs)pnpm test0 fail (2992+ pass; +7 new tests all passing)pnpm buildexit 0Manual grep sanity
stationsState = new WeakMap— exactly 1 declaration in the codebase (base class only).getOrCreateStationState— exactly 1 definition (base class).createStationState— 2 concretes + 1 abstract.resetStationState— 2 concretes + 1 abstract + 1 call site (from the basestop()template).Enforcement of the OCPP 2.0.1 5-statement ordering invariant
Four independent mechanisms enforce the abort-before-clear ordering:
OCPP20IncomingRequestService.resetStationStatedocumenting the causal explanation: clearing first would make the subsequent?.abort()short-circuit on the nulled field, leaving the in-flight operation un-signaled.no-restricted-syntaxESLint rule with 3 selectors preventing directstationsStatemutations, aliasing, and destructuring from subclass code (enforced bypnpm lintin CI).stop.bind(this)defense-in-depth against detach-and-call regressions, symmetric with the pre-existing bind policy onincomingRequestHandlerandvalidateIncomingRequestPayload.stop()body — verified callsite-by-callsite againstgit show HEAD~1.