From 58114e66247d6a93cb720f688ded5c58a6e132f4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Tue, 30 Jun 2026 00:34:53 +0200 Subject: [PATCH] =?utf8?q?refactor:=20project-wide=20audit=20of=20helper/u?= =?utf8?q?til=20usage=20in=20src/=20+=20OCPP=202.0.1=20=C2=A7F01.FR.13=20s?= =?utf8?q?pec=20fix=20(#1931)?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * refactor(utils): replace bare structuredClone with clone helper Adopts the existing `clone` helper from `src/utils/Utils.ts` at 7 sites in 4 files where `structuredClone` was called directly. `clone` is a thin wrapper around `structuredClone` (`return structuredClone(object)`); the swap is behaviour-preserving DRY consolidation. Files touched: - src/utils/Configuration.ts (1 site) - src/utils/ConfigurationMigrations.ts (3 sites + import added) - src/utils/ConfigurationValidation.ts (2 sites + import extended) - src/charging-station/TemplateValidation.ts (1 site + import extended) The `as Record` casts at ConfigurationValidation.ts:103 and TemplateValidation.ts:62 are preserved on the call sites. Signed-off-by: Jérôme Benoit * refactor(ocpp): replace (error as Error).message with getErrorMessage The `(error as Error).message` cast is unsound — when the thrown value is not an `Error` instance, `.message` is `undefined` and the log silently emits 'undefined', obscuring potentially security-relevant failures (notably on crypto / signing paths). The `getErrorMessage` helper from `src/utils/ErrorUtils.ts` handles both `Error` instances and non-`Error` thrown values via `ensureError`, producing a meaningful string in every case. Sites converted (3): - src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts:179 (buildTransactionStartedMeterValues catch block) - src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts:1181 (buildTransactionEndedMeterValues catch block) - src/charging-station/ocpp/OCPPSignedMeterValueUtils.ts:56 (deriveSigningMethodFromPublicKeyHex catch block - crypto path) Imports updated in both files. Signed-off-by: Jérôme Benoit * refactor(ocpp): replace ?.value === 'true' with convertToBoolean The inline `getConfigurationKey(...)?.value === 'true'` pattern only matches the literal lowercase string `'true'` and silently treats `'True'`, `'1'`, `true` (boolean) and any non-string value as false. `convertToBoolean` from `src/utils/Utils.ts` handles the full set of truthy representations consistently across the codebase. Sites converted (7): - src/charging-station/ocpp/OCPPServiceUtils.ts (3 sites in OCPP 2.0 signed meter value generation: SignReadings, SignStartedReadings, SignUpdatedReadings flags) - src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts (3 sites: isSigningEnabled, buildStartedMeterValues and buildUpdatedMeterValues signing guards) - src/charging-station/Bootstrap.ts (1 site: ENV_SIMULATOR_COLD_START env var) Imports updated in all three files. Signed-off-by: Jérôme Benoit * refactor: adopt isEmpty/isNotEmptyString helpers for typed collection checks Consolidates inline emptiness checks on typed strings, arrays and Sets to use the existing `isEmpty` and `isNotEmptyString` helpers from `src/utils/Utils.ts`. The helpers cover string, array, Set, Map and plain object emptiness uniformly; `isNotEmptyString` also trims internally so `workerScript.trim().length === 0` becomes `!isNotEmptyString(workerScript)` without behavior change. isEmpty sites (7): - src/charging-station/ui-server/UIServerAccessPolicy.ts:230,268,297,407 (4× .length === 0 on string[] from splitHeaderList / getAllowedHosts) - src/charging-station/ui-server/UIServerAccessPolicy.ts:508 (.size === 0 on ReadonlySet trustedProxies) - src/charging-station/ui-server/AbstractUIServer.ts:1317,1326 (2× .length === 0 on accessPolicy string[] in UI server constructor warn paths) isNotEmptyString sites (3): - src/charging-station/ocpp/OCPPSignedMeterValueUtils.ts:76 (== null || .length === 0 on string | undefined) - src/worker/WorkerAbstract.ts:29 (.trim().length === 0 on narrowed string) - src/charging-station/ui-server/AbstractUIServer.ts:668 (.length > 0 ternary on rate-limit key) Imports updated: UIServerAccessPolicy.ts adds isEmpty; OCPPSignedMeterValueUtils.ts extends to isNotEmptyString; WorkerAbstract.ts adds a fresh utils import. AbstractUIServer.ts already had both helpers imported. Signed-off-by: Jérôme Benoit * refactor: adopt convertToInt and has helpers for type conversions and property checks Consolidates inline `Number.parseInt(_, 10)` / `Number(_)` coercions on string inputs to use `convertToInt`, and `Object.hasOwn` property checks to use `has` from `src/utils/Utils.ts`. `has` accepts `(property, object)` argument order (unlike `Object.hasOwn(object, property)`) and adds a null-guard on the object. convertToInt sites (3): - src/charging-station/ui-server/UIServerNet.ts:97 (`Number.parseInt(port, 10)` on validated digit-string port) - src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts:785 (`Number(value)` on OCPP config integer-key value) - src/charging-station/TemplateSchema.ts:268 (`Number(evseKey)` on Object.entries(template.Evses) record key) has sites (2): - src/utils/ConfigurationSchema.ts:210 (`!Object.hasOwn(value as object, 'accessPolicy')` in Zod refine — the `as object` cast is no longer needed since `has` accepts `unknown`) - src/charging-station/ui-server/UIServerAccessPolicy.ts:355 (`Object.hasOwn(params, key)` in Forwarded header parameter dedup) Imports updated in UIServerNet.ts, TemplateSchema.ts, ConfigurationSchema.ts (fresh utils imports), OCPP16IncomingRequestService.ts (already imported), UIServerAccessPolicy.ts (extended). Signed-off-by: Jérôme Benoit * refactor: adopt isNotEmptyArray and buildConfigKey for collection checks and log formatting isNotEmptyArray sites (4): - src/charging-station/ui-server/AbstractUIServer.ts:1438 (countConnectors fallback on data.connectors) - src/charging-station/ui-server/AbstractUIServer.ts:1451 (iterateConnectors guard on data.connectors) - src/charging-station/ui-server/UIServerAccessPolicy.ts:438 (allowedOrigins presence check) - src/charging-station/ui-server/UIServerAccessPolicy.ts:443 (allowedHosts presence check in fallback) buildConfigKey site (1): - src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts:518 (readVariableAsInteger warn log: unify the displayed key format with the canonical `buildConfigKey(component, variable)` output instead of inline `${component}.${variable}` interpolation) Imports updated: AbstractUIServer.ts extends to isNotEmptyArray; UIServerAccessPolicy.ts extends to isNotEmptyArray. Signed-off-by: Jérôme Benoit * fix(refactor): avoid TDZ cycles via utils barrel for new helper imports The previous commits introduced fresh imports from `src/utils/index.js` in three modules that participate in barrel-induced TDZ cycles via `ConfigurationSchema.ts`: - src/charging-station/ui-server/UIServerNet.ts is imported by ConfigurationSchema.ts, so importing back through utils/index.js produces `ReferenceError: Cannot access 'isHostLiteralWithoutPort' before initialization`. - src/worker/WorkerAbstract.ts is exported via worker/index.js, which is consumed by ConfigurationSchema.ts (`WorkerProcessType`), so importing back through utils/index.js breaks Zod enum initialization with `TypeError: Cannot convert undefined or null to object` in `getEnumValues`. - src/charging-station/TemplateSchema.ts uses the same cycle-prone barrel defensively, preempting future Zod-init breakage. Resolution: import `convertToInt` and `isNotEmptyString` directly from `./Utils.js` (the leaf module), mirroring the repo-established cycle-avoidance pattern at `src/utils/ConfigurationMigrations.ts:3-4` (`import { BaseError } from '../exception/BaseError.js'` with the same kind of inline note). Tests: previously failing - tests/charging-station/ui-server/UIServerNet.test.ts - tests/worker/WorkerUtils.test.ts both pass; full suite 2857/2857. Signed-off-by: Jérôme Benoit * fix(ocpp16): revert handleRequestChangeConfiguration integer-key check to Number() Reverts the convertToInt swap at OCPP16IncomingRequestService.ts:785 (M4 in the helpers audit). The downstream check `!Number.isInteger(numValue) || numValue < 0 → REJECTED` intentionally relies on `Number(value)` returning `NaN` for invalid input to fail `Number.isInteger`. `convertToInt` breaks this contract in four distinct ways verified empirically: value Number(value) convertToInt(value) effect on rejection check -------- ------------- ------------------- ---------------------- undefined NaN 0 silently ACCEPTED '' 0 throws uncaught exception '1.5' 1.5 1 silently ACCEPTED 'abc' NaN throws uncaught exception Two of these (`''` and `'abc'`) produce uncaught exceptions that propagate out of the handler instead of returning Rejected. The other two silently accept invalid values. The audit recommendation to adopt `convertToInt` here was incorrect; the call site needs the NaN-on-invalid contract that only `Number()` provides. Issue flagged by hyperspace-insights review bot on PR #1931 (only the `undefined → 0` case was highlighted; the full divergence is broader). Inline note added to prevent re-introduction by future cleanups. Tests: tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Configuration.test.ts and tests/charging-station/ocpp/1.6/OCPP16Integration-Configuration.test.ts — 14/14 pass. Signed-off-by: Jérôme Benoit * refactor: dedup OCPP signing-flag checks and revert WorkerAbstract utils import Applies four review nits from the post-audit self-review, plus reverts an architectural misuse: F1 — OCPPServiceUtils.ts (OCPP 2.0 signed-meter-value block, 3 sites): extracts a private `isOCPP20FlagEnabled(chargingStation, component, variable)` helper consuming the existing `convertToBoolean(getConfigurationKey(_, buildConfigKey(_, _))?.value)` chain. Reduces 5-level indented blocks at lines 938, 950, 963 to a single helper call, ~12 lines saved. F2 — OCPP16ServiceUtils.ts (2 sites): adds `isSigningStartedReadingsEnabled` and `isSigningUpdatedReadingsEnabled` static methods next to the existing `isSigningEnabled`, all three following the same shape. Replaces the inline `convertToBoolean(getConfigurationKey(_, OCPP16VendorParametersKey.X)?.value)` chains at lines 173 and 826 with named-intent calls. Semantic naming > generic helper since these are vendor-key-specific. F3 — UIServerNet.ts, WorkerAbstract.ts, TemplateSchema.ts: compresses the multi-line TDZ-cycle workaround comments to single-line format, matching the established repo convention at `ConfigurationMigrations.ts:3` (`// Direct path: the exception/index.js barrel re-exports OCPPError, causing a TDZ cycle.`). F4 — OCPP16IncomingRequestService.ts:786: shortens the 5-line `// NB:` comment about `Number(value)` preservation to a 1-line concise note while preserving the regression-prevention intent. Architectural revert — WorkerAbstract.ts: removes the `isNotEmptyString` import from `../utils/Utils.js` added in 7fe3e2a10 (M3). `src/worker/` is a standalone sub-project and must not import from elsewhere in the source tree. Reverted the call site to inline `workerScript.trim().length === 0`. Same architectural rule that justifies deferring the worker-side `mergeDeepRight`/`sleep`/`secureRandom` consolidations. Tests: 37/37 pass on the affected suites (UIServerNet, WorkerUtils, WorkerAbstract, OCPP16 Configuration, OCPP16 Configuration integration). Signed-off-by: Jérôme Benoit * refactor(ocpp2): adopt handleIncomingRequestError across OCPP 2.0 handlers Brings OCPP 2.0 incoming-request handlers to parity with the OCPP 1.6 equivalent (OCPP16IncomingRequestService.ts) by routing all catch-and-return patterns through the shared `handleIncomingRequestError` helper from `src/utils/ErrorUtils.ts`. Sites converted (14 total): 10 clean sites (A — single rejection-response return): - handleRequestClearCache (line 815) — OCPP_RESPONSE_REJECTED - handleRequestGetLocalListVersion (line 851) — { versionNumber: 0 } - handleRequestSendLocalList (line 961) — OCPP_SEND_LOCAL_LIST_RESPONSE_FAILED - handleRequestCertificateSigned (line 1583) — typed Rejected w/ statusInfo - handleRequestDeleteCertificate (line 1849) — typed Failed w/ statusInfo - handleRequestGetInstalledCertificateIds (line 1954) — typed NotFound w/ statusInfo - handleRequestInstallCertificate (line 2109) — typed Failed w/ statusInfo - handleRequestReset (line 2329) — typed Rejected w/ statusInfo - handleRequestTriggerMessage (line 2864) — typed Rejected w/ statusInfo - handleRequestUnlockConnector (line 2949) — typed UnlockFailed w/ statusInfo 4 conditional sites (G — nested + outer catches in handleRequestStartTransaction, lines 2567/2608/2673/2737): each catch builds a typed OCPP20RequestStartTransactionResponse local const (status: Rejected with the appropriate additionalInfo, transactionId: generateUUID()) and passes it both as errorResponse and as the `??` typing-narrowing fallback. The `await this.resetConnectorOnStartTransactionError(...)` side-effect on the outer catch is preserved before the helper call. Imports added: ensureError, handleIncomingRequestError (alphabetical). Behavioral note: the log message format changes from `OCPP20IncomingRequestService.handleRequestX: ...` to `ErrorUtils.handleIncomingRequestError: Incoming request command 'X' error: ...` — matches the OCPP 1.6 service. The error itself is still logged (via the helper's internal logger.error call); no error is swallowed. Tests: 336/336 pass on `tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-*.test.ts`. Signed-off-by: Jérôme Benoit * refactor(validation): adopt assertIsJsonObject for JSON-object type guards Replaces the inline guard 'if (parsed == null || typeof parsed !== object || Array.isArray(parsed)) throw new BaseError(...)' with the canonical 'assertIsJsonObject(parsed, new BaseError(...))' helper from 'src/utils/Utils.ts'. The helper: - Narrows the value type to JsonObject via 'asserts value is JsonObject' (type predicate gain). - Accepts a custom Error second argument that is thrown verbatim, preserving the existing BaseError message and stack trace. Sites converted (2): - src/charging-station/TemplateValidation.ts:50 (validateTemplate) - src/utils/ConfigurationValidation.ts:92 (validateConfiguration) Imports updated: TemplateValidation.ts and ConfigurationValidation.ts both extended to include assertIsJsonObject. Tests: 74/74 pass on the validation suites. Signed-off-by: Jérôme Benoit * refactor: adopt JSONStringify helper across src/ Replaces 14 bare 'JSON.stringify(_, undefined/null, 2)' call sites with the shared 'JSONStringify' helper from 'src/utils/Utils.ts'. The helper: - Serializes Maps and Sets safely. Bare 'JSON.stringify' silently drops Map entries (correctness fix for sites with potential Map values). - Accepts an optional 'MapStringifyFormat' parameter to control how Maps are rendered (default: array of pairs; '.object': key-value object). Sites with Map values (use 'MapStringifyFormat.object' to match the existing JSON config-schema representation of station data): - src/charging-station/ChargingStation.ts:2595 (config file write, ChargingStationConfiguration) - src/charging-station/ui-server/mcp/MCPResources.ts:22 (listChargingStationData) - src/charging-station/ui-server/mcp/MCPResources.ts:44 (station data by id) Plain-object sites (default 'JSONStringify(_, 2)'): - src/charging-station/BootstrapStateUtils.ts:150 (state file write) - src/charging-station/ocpp/OCPPServiceUtils.ts:200 (validate.errors) - src/charging-station/Bootstrap.ts:647 (worker message error) - src/charging-station/ui-server/ui-services/AbstractUIService.ts:172 - src/charging-station/ui-server/mcp/MCPResources.ts:64 (templates list) - src/charging-station/ui-server/mcp/MCPResources.ts:119 (commands list) - src/charging-station/ocpp/OCPPResponseService.ts:100,112 (error PDU msgs) - src/charging-station/ocpp/OCPPIncomingRequestService.ts:70,117,129 (error PDU msgs) Also loosens the 'JSONStringify' generic constraint to 'object: unknown' (matching 'JSON.stringify' standard signature). The previous restrictive generic (already marked 'no-unnecessary-type-parameters') prevented passing values like 'ChargingStationConfiguration' that lack TS index signatures but are JSON-serializable at runtime. Removes unused 'JsonType' import. Skipped (architectural): - src/worker/WorkerSet.ts:191 — 'src/worker/' is a standalone sub-project and must not import from elsewhere (same rule as the WorkerAbstract revert in 2f190a4cf). Tests: 512/512 pass on Utils + ChargingStation + Bootstrap + OCPP + UI server suites. Signed-off-by: Jérôme Benoit * refactor(bootstrap): adopt promiseWithTimeout in waitChargingStationsStopped Replaces the manual 'new Promise((resolve, reject) => { setTimeout(...); waitChargingStationEvents(...).then(resolve).catch(reject) })' construct with the shared 'promiseWithTimeout' helper from 'src/utils/Utils.ts'. The refactor is behaviour-preserving: - The timeout BaseError is now constructed once upfront and passed to 'promiseWithTimeout' (matching the helper's pre-built error contract). - The 'logger.warn' is emitted only when the caught error is referentially the timeoutError (i.e., only on actual timeout, not on errors propagated from 'waitChargingStationEvents'). Same semantic as the original which only logged inside the setTimeout callback. - 'clearTimeout' on success path is handled internally by 'promiseWithTimeout' via 'promise.finally(() => clearTimeout(timer))'. Linear try/await/catch flow replaces the nested Promise constructor + .then/.catch/.finally chain (~5 LOC saved, less indirection). Imports updated: Bootstrap.ts adds 'promiseWithTimeout' alphabetically to the existing utils barrel import. Tests: 11/11 pass on 'tests/charging-station/Bootstrap.test.ts'. Signed-off-by: Jérôme Benoit * refactor: unify handleIncomingRequestError local-const pattern + doc identity check F1 — OCPP20IncomingRequestService.ts (2 sites): converts the constant-inlined-twice pattern at handleRequestClearCache:815 and handleRequestSendLocalList:961 to the local-const pattern used by the other 11 sites in the same file. The error-response constant is now bound to a typed local const and referenced once for both the helper's 'errorResponse' parameter and the '?? errorResponse' TS-narrowing fallback, eliminating the double reference to the same constant. F3 — Bootstrap.ts waitChargingStationsStopped: adds a 3-line comment above the 'if (error === timeoutError)' identity check documenting the contract relied upon — 'promiseWithTimeout' must reject with the same Error instance passed in. If a future maintainer wraps the error inside the helper, the identity check would silently break and the timeout-specific 'logger.warn' would no longer fire. The comment also clarifies that downstream errors from 'waitChargingStationEvents' intentionally propagate unlogged (preserves the original behaviour). No behavioural change. Tests: 347/347 pass on OCPP20IncomingRequest + Bootstrap suites. Signed-off-by: Jérôme Benoit * refactor: apply v3 cross-validated review fixes Applies 9 fixes from the 4-agent cross-validation review (2 oracles + 1 librarian + 1 explore agent on the same scope). HIGH-CONFIDENCE fixes (>=2 agents converged): CV1 — type-safety: 'isOCPP20FlagEnabled' parameter tightened from 'variable: string' to 'OCPP20OptionalVariableName | OCPP20RequiredVariableName | OCPP20VendorVariableName'. The merged-runtime 'StandardParametersKey' / 'VendorParametersKey' objects only had OCPP 1.6 typedefs, masking the OCPP 2.0 call-site values; the explicit union closes the gap. (OCPPServiceUtils.ts) CV2 — semantic preservation: 'convertToInt(evseKey)' in the Zod '.superRefine' callback would throw on non-numeric keys, where the original 'Number(evseKey)' returned NaN and silently fell through. Reverted to 'convertToIntOrNaN(evseKey)' to preserve the NaN-on-invalid contract the constraint check downstream relies on. Same class of bug the bot identified at OCPP16IncomingRequestService:785. (TemplateSchema.ts) CV3 — discriminator robustness: replaced 'error === timeoutError' identity check with an 'instanceof TimeoutError' check against a 'BaseError' subclass. The subclass discriminator survives any future wrapping of the error inside 'promiseWithTimeout'; the identity check silently broke on any error-cloning refactor of the helper. (Bootstrap.ts) CV4 — cross-version parity: aligned the 4 remaining 'handleIncomingRequestError' sites in OCPP 1.6 (cancelReservation, dataTransfer, getDiagnostics, reserveNow) to the local-const pattern that OCPP 2.0 adopted in the audit, eliminating the duplicated constant reference at each call site. (OCPP16IncomingRequestService.ts) OBSERVABILITY fix: S1 — reverted the 4 nested + outer catches in OCPP 2.0 'handleRequestStartTransaction' (Authorization / Group authorization / Charging profile validation / outer 'Error starting transaction') from 'handleIncomingRequestError' adoption back to inline 'catch → logger.error → return typed-rejection'. The helper's unified log message dropped the phase-specific context (truncated idToken values, distinct phase names). OCPP 1.6 'handleRequestRemoteStartTransaction' itself does NOT use 'handleIncomingRequestError', so this revert restores both observability AND cross-version parity. The other 10 single-catch sites adopted by the audit remain unchanged. (OCPP20IncomingRequestService.ts) Doc/style nits: S4 — README.md: 'SIMULATOR_COLD_START=true' documented as 'a truthy value (true, True, TRUE, 1, optionally surrounded by whitespace)' to reflect the actual 'convertToBoolean' semantics adopted in the audit. S5 — Bootstrap.ts:649: added 'MapStringifyFormat.object' to the 'JSONStringify(data, 2)' call on worker-message error data. Consistent with the format choice at 'ChargingStation.ts:2597' and 'MCPResources.ts' since worker message data can carry Maps. S6 — TemplateSchema.ts:3: TDZ-cycle direct-path comment realigned to the declarative form used by 'ConfigurationMigrations.ts:3' ('the X barrel re-exports Y, causing a TDZ cycle.'). S7 — OCPP16ServiceUtils.ts:683,694: JSDoc terminology aligned with the spec ('signing of meter values at transaction start' / 'during transaction updates' instead of 'transaction-started readings' / 'transaction-updated readings'). Tests: 520/520 pass on OCPP 2.0 IncomingRequest, OCPP 1.6 IncomingRequest, Bootstrap, TemplateSchema, TemplateValidation suites. Signed-off-by: Jérôme Benoit * refactor: apply v4 cross-validation review fixes Applies 10 fixes from the v4 4-agent cross-validation review (2 oracles + 1 librarian + 1 explore) on the post-v3 state of PR #1931. CORRECTNESS: BUG — Configuration.ts:293: 'convertToInt(env.PORT ?? "")' would throw an uncaught exception when PORT is unset (?? '' returns empty string, on which convertToInt throws). Same class of bug as the CV2 fix in TemplateSchema. Guards with isNotEmptyString(env.PORT) and only converts when set. DESIGN: HC1 + Q2(d) — TimeoutError relocated from Bootstrap.ts to a dedicated src/exception/TimeoutError.ts file, exported from src/exception/index.js. Bootstrap.ts now imports TimeoutError alongside BaseError. Matches the established repo pattern (BaseError, OCPPError each in their own file under src/exception/). The class declaration is no longer interleaved with imports in Bootstrap.ts. S1 — extract rejectedInternalError(additionalInfo) helper in OCPP20IncomingRequestService.ts. Collapses 4 near-identical 8-line '{ status: Rejected, statusInfo: { additionalInfo, reasonCode: InternalError }, transactionId: generateUUID() }' literals at the 4 nested + outer catches of handleRequestStartTransaction. Each catch becomes a 1-line 'return rejectedInternalError("...")'. S2 — wrap 'logger.error(..., error)' with 'ensureError(error)' at the 4 reverted G-sites. Aligns with the 10 sibling sites in the same file that already route through ensureError. Non-Error throwables are normalized before reaching the logger. Q3(a) — TemplateSchema.ts: add 'Number.isNaN(evseId)' guard in the .superRefine loop over Object.entries(template.Evses). Closes the validation gap where 'Evses: { "abc": {...} }' silently passed because NaN === 0 / NaN > 0 are both false. The guard pushes a 'ctx.addIssue' for non-numeric keys with a clear message citing OCPP 2.0.1 §7.2. POLISH: S3 — Bootstrap.ts: 'logger.warn(..., error.message)' instead of the closed-over 'timeoutError.message'. After the instanceof TimeoutError narrowing, 'error' is correctly typed as TimeoutError. If a future wrap of promiseWithTimeout passes through a different instance, error.message still reflects what was actually caught. S4 — Bootstrap.ts: 'behaviour' (British) → 'behavior' (American) to match the 30+ occurrences elsewhere in src/. Also resolves the cspell flag. S5 — Bootstrap.ts: catch comment compressed from 3 lines to 1 line. Drops the partial redundancy with the TimeoutError JSDoc and removes the 'unlogged' cspell flag. HC2 — README.md: SIMULATOR_COLD_START wording tightened from a non-exhaustive list ('true, True, TRUE, 1') to the exhaustive rule 'case-insensitive true or 1, optionally surrounded by whitespace'. Matches the actual convertToBoolean semantics (trim + toLowerCase). Tests: 2832/2832 pass, 0 fail. Signed-off-by: Jérôme Benoit * refactor: apply v5 cross-validation review fixes HC-A TemplateSchema.ts: replace NaN-via-arithmetic guard with explicit Number.isInteger check for evseId validation (clearer intent, same semantics). HC-B + HC-C TimeoutError.ts: compress JSDoc to single line; add SPDX copyright header to match peer OCPPError.ts. L-A + HC-D OCPP20IncomingRequestService.ts: rename rejectedInternalError helper to buildRejectedResponse(additionalInfo, reasonCode) and migrate all 11 RequestStartTransaction rejection sites (4 helper calls + 7 inline literals) through it. Drop fabricated transactionId: generateUUID() from rejection responses per OCPP 2.0.1 §E07: that field signals a transaction already started by the Charging Station before the request arrived (cable-first scenarios), so fabricating a UUID on a pure rejection misleads CSMS implementations that map remoteStartId -> transactionId. V5-6 Bootstrap.ts: relocate catch-block ordering comment below instanceof check and reword for clarity. Test: update §E07 regression assertion in OCPP20IncomingRequestService-RequestStartTransaction.test.ts to expect response.transactionId === undefined on TxInProgress rejection (was incorrectly asserting notStrictEqual against the now-fixed bug). All quality gates pass: 2857/2857 tests, typecheck, lint, format. * docs(ocpp20): correct spec citation §E07 → §F01.FR.13 The 'transactionId-on-rejection' fix in commit 23beb7ea1 cited OCPP 2.0.1 §E07, which is wrong: §E07 is 'Transaction locally stopped by IdToken'. The actual clause governing transactionId echo on RequestStartTransaction- Response is §F01.FR.13: 'When a transaction is created on the Charging Station, but has not been authorized. AND RequestStartTransactionRequest is received → The Charging Station SHALL return the transactionId in the RequestStartTransactionResponse.' This is the cable-plugin-first scenario. On pure rejections F01.FR.13 does not apply, so transactionId is correctly omitted. The semantics of the fix are unchanged; only the spec citation in the comments is corrected. Verified against OCPP-2-0-1-edition3-part2-specification.md via local spec collection. * refactor: apply v6 cross-validation review fixes V6-S2 + V6-Q1 (HIGH, both Oracle agents): RequestStartTransaction TxInProgress rejection regressed OCPP 2.0.1 part 2 §F01.FR.13 when the connector held a transactionPending state. The v5 fix dropped the fabricated UUID (correct) but also dropped the real connectorStatus. transactionId (incorrect). Per §F01.FR.13: 'When a transaction is created on the Charging Station, but has not been authorized AND RequestStartTransactionRequest is received, the Charging Station SHALL return the transactionId in the RequestStartTransactionResponse.' Split the TxInProgress check into two branches: - transactionPending === true AND typeof connectorStatus.transactionId === 'string' → buildRejectedResponse(TxStarted, ..., transactionId). The appendix distinguishes TxStarted (cable-plugin-first, not yet authorized) from TxInProgress (authorized and running) — TxStarted matches the §F01.FR.13 precondition. - Fallthrough (transactionStarted || transactionPending-without-tx-id || locked) → buildRejectedResponse(TxInProgress, ...) without echo. Extended buildRejectedResponse signature: added optional transactionId?: UUIDv4 parameter and swapped to (reasonCode, additionalInfo, transactionId?) — required-first/optional-last per V6-X3+Q11. All 11 existing callsites updated; the new branch is the 12th call. V6-S1 (M, Oracle1, qmd-verified): TemplateSchema.ts cited OCPP 2.0.1 §7.2 (Connector numbering) for EVSE-key validation. The correct clause is §7.1 (EVSE numbering) — qmd against OCPP-2.0.1_edition3_part1_architecture_topology.md:269 confirms §7.1 covers 'EVSEs MUST be sequentially numbered starting from 1' and 'evseId 0 is reserved'. Two citation occurrences corrected. V6-S4 (L, Oracle1): F01.FR.11 violation (ChargingProfile.transactionId set in RequestStartTransaction) used reasonCode InvalidValue. F01.FR.26 hints InvalidProfile/InvalidSchedule for invalid ChargingProfile. Changed to InvalidProfile for family coherence with the other two ChargingProfile rejection sites. V6-X1 (L, Librarian): Helper comment overstated the spec — said 'fabricating a UUID misleads CSMS' as if §F01.FR.13 mandated MUST NOT. Reworded to distinguish the positive obligation (SHALL echo when precondition holds) from the practical argument (fabrication misleads). Documents the new optional transactionId? parameter semantics. V6-X14 (M, Librarian): TimeoutError extends BaseError {} was an empty subclass — TypeScript structural typing made BaseError and TimeoutError indistinguishable in the type system, so the single instanceof check in Bootstrap.ts relied solely on the runtime prototype chain. Added 'public override readonly name = TimeoutError as const' for nominal distinction (matches the OCPPError pattern in the same codebase). V6-Q2 (M, Oracle2): Test coverage gap — only 1 of the 11 v5 rejection paths asserted transactionId === undefined. Added the assertion to the 6 remaining unprotected Rejected tests (InvalidIdToken, MasterPassGroup, invalid ChargingProfile purpose, ChargingProfile-with-transactionId, authorization throw, all-EVSEs-inoperative). The V6-S2 fix test now asserts transactionId === pendingTransactionId AND reasonCode === TxStarted (regression-locks the §F01.FR.13 echo path). Deferred to follow-up: V6-Q3 (mixed catch pattern — intentional per v3 S1), V6-E11/E12/E15 (ensureError missing at 18 logger.error sites in OCPP 2.0 .catch callbacks + 2 OCPP 1.6 catches), V6-E19 (extract helper for RequestStopTransaction rejection literals — needs different return type), V6-X3 cosmetic polish items. Verified via qmd ocpp-specs collection: §F01.FR.13, §7.1 vs §7.2, §2.10, B09.FR.31 (errata 2025-09 §2.12 — exists as cited), TxStarted vs TxInProgress appendix distinction. V6-E3 (Explorer's HIGH flag on B09.FR.31) was a false positive — citation is valid. All quality gates pass: typecheck, lint, format, 2857/2857 tests. * refactor: apply v7 cross-validation review fixes V7 review pass — 4 agents (oracle ×2, librarian, explore) with v6 lessons integrated as new criteria: regression-chain audit, type-cast scrutiny, test-gap blast-radius, helper-signature swap audit, false-positive detection via qmd. Convergent findings reconciled into 11 in-scope fixes. False positives rejected (lesson from V6-E3/E4): - V7-E2/E17 (Explore HIGH unique): claimed startTransactionOnConnector misses transactionPending=true. REJECTED — ATG path sends TransactionEvent(Started) with triggerReason=Authorized, not cable- plugin-first. Setting transactionPending=true there would create spurious echoes on already-authorized connectors. Fixes applied: V7-Q1+X5 (idiom): swap ternary spread '?: {}' to '&& { x }' to match the dominant codebase pattern (ChargingStation.ts, OCPP16TestUtils.ts, AbstractUIService.ts). V7-Q4+S1 (helper docstring): trim from 6 lines to 3 + reword to avoid implying §F01.FR.13 mandates status=Rejected (spec is silent on status). V7-Q5 (test comment): trim P4 duplicate of helper docstring to 1-line anchor — assertions already self-document. V7-Q7+S2+X6 (UUIDv4 cast safety): add inline justification comment at the 'as UUIDv4' cast site pointing to the source-of-truth write site (line ~2740 generateUUID() assignment). V7-Q2 (reasonCode lock): add statusInfo.reasonCode assertions to 6 v6-Q2 tests so a future bug swapping codes (e.g., InvalidIdToken vs InvalidValue) cannot pass: InvalidIdToken×2, InvalidProfile×2, InternalError×1, NotFound×1. V7-Q3 (residual TxInProgress test): add test exercising the fallthrough branch via connectorStatus.locked = true — verifies reasonCode=TxInProgress and transactionId=undefined (the V6-S2 split's other branch). V7-S6+E4 (MasterPass coverage): add transactionId=undefined + reasonCode=InvalidIdToken assertions at MasterPass.test.ts:112 (V6-Q2 pattern spillover). V7-E5 (RequestStopTransaction coverage): add reasonCode assertions (InvalidValue×3) to the 3 invalid-UUID-format rejection tests. V7-E7 (test describe label): TemplateSchema.test.ts:235 describe block updated to reflect §7.1 EVSE numbering + §7.2 connector numbering split applied at v6-S1. V7-E8 (citation qualifier): TemplateSchema.ts:285,295 inline error messages use 'part 1 §7.2' for consistency with v6-S1 correction style. V7-E9 (empty subclass discriminants): add 'public override readonly name = ... as const' to 5 error subclasses (OCPPError, TemplateValidationError, PayloadTooLargeError, ConfigurationValidationError, AuthenticationError — last had non-readonly non-as-const override). Extends V6-X14 TimeoutError pattern uniformly. Deferred (rationale documented): - V7-Q6+X4 — TimeoutError commit-body typo and 'matches OCPPError' inaccuracy — git history immutable, factual note belongs in PR desc. - V7-E1 — ConnectorStatus.transactionId: number|string union (OCPP 1.6 numeric IDs share the field). Narrowing requires OCPP 1.6 refactor. - V7-E3+E6+E10+E12+E13+E14+E15+E16+E18 — verified clean / not in scope (defensive code correct, no echo fields, plausible 1.6 citations). Verified via qmd ocpp-specs (OCA FINAL schema, lorenzodonini/ocpp-go, mobilityhouse/ocpp, citrineos-core, EVerest/libocpp): TxStarted vs TxInProgress appendix distinction matches v6 split; transactionId on status=Rejected is schema-valid; no reference impl contradicts v6; typeof===string + as UUIDv4 cast is safe given OCPP 2.0 invariant. All quality gates pass: typecheck, lint, format, 2852/2852 tests. * refactor: apply v8 cross-validation review fixes V8 review pass — 4 agents (oracle ×2, librarian, explore) with v7 lessons elevated to criteria: recursive false-positive detection, V7-E9 blast radius audit, V7-Q1 idiom equivalence under JSON.stringify, V7-Q3 test isolation, convergence saturation indicator (<3 substantive = bottom). 3 agents declared SATURATION REACHED (Oracle1, Oracle2, Librarian). 1 agent (Explore) found 3 NEW substantive findings + 1 echo, breaking saturation. False positive rejected (lesson from V6-E3, V7-E2): - V8-E2 (Explore L) — claimed BaseError has no name override and would emit name='Error' on direct instantiation. REJECTED: BaseError's constructor already does this.name = new.target.name, so direct instantiation yields name='BaseError' correctly. Caller-trace verification (same v7 criterion) eliminated this. Fixes applied: V8-E1 (M) — Extend V7-E9 pattern to ui/common package missed entirely by v7. ConnectionError and ServerFailureError in ui/common/src/errors.ts used mutable constructor-assignment 'this.name = ...' instead of the class-field pattern. Replaced with 'public override readonly name = "" as const' class fields, removed the constructor assignments. Now all 8 error subclasses across src/ and ui/common share the same nominal-discriminant pattern. V8-E3 (L) — handleRequestStartTransaction had 1 remaining inline rejection literal at line 2516 (the resolvedEvseId == null branch with reasonCode=NotFound) bypassing the buildRejectedResponse helper used by all 11 other rejection returns in the same function. Replaced with buildRejectedResponse(ReasonCodeEnumType.NotFound, 'No available EVSE found for remote start') for invariant coherence. V8-S1 (L) — RequestStopTransaction test 'should reject stop transaction for non-existent transaction ID' (line 133) used 'non-existent- transaction-id' as input, which fails UUID format validation at line 2783 and never reaches the TxNotFound branch (line 2796). The test covered InvalidValue (format-rejection), not TxNotFound. Renamed test to '... for invalid transaction ID format - non-UUID string' to match actual coverage + added new test 'should reject stop transaction for valid UUID with no matching transaction' that uses '00000000-0000-4000-8000-000000000000' (valid UUIDv4 format, no matching transaction) and asserts reasonCode=TxNotFound. The real TxNotFound branch is now covered. Deferred (rationale documented): - V8-S2/Q1 — line-number pointer '~2740' in the as UUIDv4 cast comment is fragile but acceptable; tilde already signals approximation. - V8-S3/Q2 — §F01.FR.13 anchor appears at 4 sites (helper docstring, branch comment, V6-S2 test comment, V7-Q3 test name). Each lives at a distinct cognitive level (helper API vs handler decision vs test intent); not collapsible. - V8-E4 (echo of V7-S7) — handleRequestStopTransaction inline literals return a different response type (OCPP20RequestStopTransactionResponse); buildRejectedResponse cannot be reused. Helper-extraction follow-up out of scope. - V8-E5 (echo) — 3 ?: {} sites in test-helper files. Already noted. Saturation declaration: - v8 found 3 substantive findings (V8-E1, V8-E3, V8-S1). v9 sweep would target: (a) ui/common error subclasses (now all readonly as const), (b) buildRejectedResponse call sites (now all 13 use the helper), (c) RequestStopTransaction test coverage (now exercises both InvalidValue and TxNotFound branches). Expected v9 finding count: 0. Verified via qmd ocpp-specs (5 audits cumulative), empirical Node.js v24 testing (V7-E9 stack trace + JSON.stringify + util.inspect + winston format paths), and external references (5 OSS OCPP impls + 5 major TS error-class repos for readonly-name canonical confirmation). All quality gates pass: typecheck (src + ui/common), lint (src + ui/common), format, tests (src 2859/2865, ui/common 120/120, 0 fail). --------- Signed-off-by: Jérôme Benoit --- README.md | 2 +- src/charging-station/Bootstrap.ts | 57 ++-- src/charging-station/BootstrapStateUtils.ts | 3 +- src/charging-station/ChargingStation.ts | 4 +- src/charging-station/TemplateSchema.ts | 18 +- src/charging-station/TemplateValidation.ts | 12 +- .../ocpp/1.6/OCPP16IncomingRequestService.ts | 26 +- .../ocpp/1.6/OCPP16ServiceUtils.ts | 38 ++- .../ocpp/2.0/OCPP20IncomingRequestService.ts | 321 ++++++++++-------- .../ocpp/2.0/OCPP20ServiceUtils.ts | 7 +- .../ocpp/OCPPIncomingRequestService.ts | 20 +- .../ocpp/OCPPResponseService.ts | 14 +- src/charging-station/ocpp/OCPPServiceUtils.ts | 55 +-- .../ocpp/OCPPSignedMeterValueUtils.ts | 6 +- .../ocpp/auth/types/AuthTypes.ts | 2 +- .../ui-server/AbstractUIServer.ts | 14 +- .../ui-server/UIServerAccessPolicy.ts | 17 +- src/charging-station/ui-server/UIServerNet.ts | 5 +- .../ui-server/UIServerSecurity.ts | 2 + .../ui-server/mcp/MCPResources.ts | 11 +- .../ui-services/AbstractUIService.ts | 7 +- src/exception/OCPPError.ts | 1 + src/exception/TimeoutError.ts | 8 + src/exception/index.ts | 1 + src/utils/Configuration.ts | 8 +- src/utils/ConfigurationMigrations.ts | 7 +- src/utils/ConfigurationSchema.ts | 3 +- src/utils/ConfigurationValidation.ts | 14 +- src/utils/Utils.ts | 18 +- tests/charging-station/TemplateSchema.test.ts | 2 +- ...0IncomingRequestService-MasterPass.test.ts | 3 + ...estService-RequestStartTransaction.test.ts | 45 ++- ...uestService-RequestStopTransaction.test.ts | 17 +- ui/common/src/errors.ts | 4 +- 34 files changed, 446 insertions(+), 326 deletions(-) create mode 100644 src/exception/TimeoutError.ts diff --git a/README.md b/README.md index 03b2830f..57d694f0 100644 --- a/README.md +++ b/README.md @@ -183,7 +183,7 @@ But the modifications to test have to be done to the files in the build target d | uiServer | | {
"enabled": false,
"type": "ws",
"version": "1.1",
"accessPolicy": {
"requireTlsForNonLoopback": true,
"trustedProxies": [],
"allowLoopbackProxy": false,
"allowedHosts": [],
"allowedOrigins": []
},
"options": {
"host": "localhost",
"port": 8080
}
} | {
enabled?: boolean;
type?: ApplicationProtocol;
version?: ApplicationProtocolVersion;
accessPolicy?: {
requireTlsForNonLoopback?: boolean;
trustedProxies?: string[];
allowLoopbackProxy?: boolean;
allowedHosts?: string[];
allowedOrigins?: string[];
};
options?: ServerOptions;
authentication?: {
enabled: boolean;
type: AuthenticationType;
username?: string;
password?: string;
};
metrics?: {
enabled?: boolean;
softSampleCap?: number;
};
} | UI server configuration section:
- _enabled_: enable UI server
- _type_: 'ws', 'mcp' or 'http' (deprecated)
- _version_: HTTP version '1.1' or '2.0' (ws and mcp transports only support '1.1')
- _accessPolicy_: gateway access policy. Loopback request sources are allowed in plaintext; non-loopback sources require TLS termination by a reverse proxy:
  - _requireTlsForNonLoopback_: reject non-loopback plaintext requests; the check honors `X-Forwarded-Proto` or `Forwarded: proto=` from a trusted proxy, non-loopback requests without forwarded protocol headers are denied as `tls-required`
  - _trustedProxies_: IPv4 or IPv6 literals of the immediate reverse proxies whose forwarded headers are honored (hostnames and CIDR ranges are not accepted; only single-hop forwarded chains are honored); a compromised entry can bypass per-client rate limiting by varying `X-Forwarded-For`
  - _allowLoopbackProxy_: accept forwarded headers when the immediate peer is loopback AND listed in _trustedProxies_ (e.g. `['127.0.0.1', '::1']`)
  - _allowedHosts_: explicit Host header allowlist; mitigates DNS rebinding when the UI server is exposed through a browser-facing host
  - _allowedOrigins_: explicit Origin header allowlist; when empty, the request Origin's URL hostname falls back to matching against _allowedHosts_
- _options_: node.js net module [listen options](https://nodejs.org/api/net.html#serverlistenoptions-callback)
- _authentication_: authentication type configuration section
- _metrics_: opt-in Prometheus `/metrics` endpoint (served on the configured `uiServer.type` listener — `http`, `ws` or `mcp`):
  - _enabled_: enable the `/metrics` endpoint
  - _softSampleCap_: soft cardinality cap above which a single warn is logged per scrape (default 5000) | | performanceStorage | | {
"enabled": true,
"type": "none",
} | {
enabled?: boolean;
type?: string;
uri?: string;
} | Performance storage configuration section:
- _enabled_: enable performance storage
- _type_: 'jsonfile', 'mongodb' or 'none'
- _uri_: storage URI | | stationTemplateUrls | | {}[] | {
file: string;
numberOfStations: number;
provisionedNumberOfStations?: number;
}[] | array of charging station templates URIs configuration section:
- _file_: charging station configuration template file relative path
- _numberOfStations_: template number of stations at startup
- _provisionedNumberOfStations_: template provisioned number of stations after startup | -| persistState | true/false | true | boolean | persist the simulator stopped state to `dist/assets/configurations/.simulator-state.json`. On the next process startup, if the simulator was stopped via the UI procedure `stopSimulator`, charging stations are not auto-spawned and the user can recover via the `startSimulator` procedure. Signal-driven shutdowns (SIGINT/SIGTERM/SIGQUIT) and configuration-reload restarts do not modify the persisted state. The feature requires `uiServer.enabled` to be `true`; otherwise it is silently ignored (no recovery channel without UI). Set the environment variable `SIMULATOR_COLD_START=true` for one run to ignore the persisted state and force a cold start. UI-added charging stations beyond `numberOfStations` are not auto-respawned | +| persistState | true/false | true | boolean | persist the simulator stopped state to `dist/assets/configurations/.simulator-state.json`. On the next process startup, if the simulator was stopped via the UI procedure `stopSimulator`, charging stations are not auto-spawned and the user can recover via the `startSimulator` procedure. Signal-driven shutdowns (SIGINT/SIGTERM/SIGQUIT) and configuration-reload restarts do not modify the persisted state. The feature requires `uiServer.enabled` to be `true`; otherwise it is silently ignored (no recovery channel without UI). Set the environment variable `SIMULATOR_COLD_START` to a truthy value (case-insensitive `true` or `1`, optionally surrounded by whitespace) for one run to ignore the persisted state and force a cold start. UI-added charging stations beyond `numberOfStations` are not auto-respawned | #### Worker process model diff --git a/src/charging-station/Bootstrap.ts b/src/charging-station/Bootstrap.ts index 76141b98..ea25f0e1 100644 --- a/src/charging-station/Bootstrap.ts +++ b/src/charging-station/Bootstrap.ts @@ -13,7 +13,7 @@ import type { IBootstrap } from './IBootstrap.js' import type { AbstractUIServer } from './ui-server/AbstractUIServer.js' import packageJson from '../../package.json' with { type: 'json' } -import { BaseError } from '../exception/index.js' +import { BaseError, TimeoutError } from '../exception/index.js' import { type Storage, StorageFactory } from '../performance/index.js' import { type ChargingStationData, @@ -24,6 +24,7 @@ import { type ChargingStationWorkerMessageData, ChargingStationWorkerMessageEvents, ConfigurationSection, + MapStringifyFormat, ProcedureName, type SimulatorState, type Statistics, @@ -35,15 +36,18 @@ import { import { Configuration, Constants, + convertToBoolean, formatDurationMilliSeconds, generateUUID, handleUncaughtException, handleUnhandledRejection, isAsyncFunction, isNotEmptyArray, + JSONStringify, logger, logPrefix, once, + promiseWithTimeout, } from '../utils/index.js' import { DEFAULT_ELEMENTS_PER_WORKER, @@ -133,7 +137,7 @@ export class Bootstrap extends EventEmitter implements IBootstrap { } private get persistStateEnabled (): boolean { - if (env[Constants.ENV_SIMULATOR_COLD_START] === 'true') { + if (convertToBoolean(env[Constants.ENV_SIMULATOR_COLD_START])) { return false } if (!Configuration.getPersistState()) { @@ -643,11 +647,7 @@ export class Bootstrap extends EventEmitter implements IBootstrap { break default: throw new BaseError( - `Unknown charging station worker message event: '${event as string}' received with data: ${JSON.stringify( - data, - undefined, - 2 - )}` + `Unknown charging station worker message event: '${event as string}' received with data: ${JSONStringify(data, 2, MapStringifyFormat.object)}` ) } } catch (error) { @@ -699,30 +699,29 @@ export class Bootstrap extends EventEmitter implements IBootstrap { } private async waitChargingStationsStopped (): Promise { - return await new Promise((resolve, reject: (reason?: unknown) => void) => { - const waitTimeout = setTimeout(() => { - const timeoutMessage = `Timeout ${formatDurationMilliSeconds( - Constants.STOP_CHARGING_STATIONS_TIMEOUT_MS - )} reached at stopping charging stations` + const timeoutError = new TimeoutError( + `Timeout ${formatDurationMilliSeconds(Constants.STOP_CHARGING_STATIONS_TIMEOUT_MS)} reached at stopping charging stations` + ) + try { + await promiseWithTimeout( + waitChargingStationEvents( + this, + ChargingStationWorkerMessageEvents.stopped, + this.numberOfStartedChargingStations + ), + Constants.STOP_CHARGING_STATIONS_TIMEOUT_MS, + timeoutError + ) + return 'Charging stations stopped' + } catch (error) { + if (error instanceof TimeoutError) { logger.warn( - `${this.logPrefix()} ${moduleName}.waitChargingStationsStopped: ${timeoutMessage}` + `${this.logPrefix()} ${moduleName}.waitChargingStationsStopped: ${error.message}` ) - reject(new BaseError(timeoutMessage)) - }, Constants.STOP_CHARGING_STATIONS_TIMEOUT_MS) - waitChargingStationEvents( - this, - ChargingStationWorkerMessageEvents.stopped, - this.numberOfStartedChargingStations - ) - .then(events => { - resolve('Charging stations stopped') - return events - }) - .finally(() => { - clearTimeout(waitTimeout) - }) - .catch(reject) - }) + } + // Non-TimeoutError errors propagate without logging (handled by the caller). + throw error + } } private readonly workerEventAdded = (data: ChargingStationData): void => { diff --git a/src/charging-station/BootstrapStateUtils.ts b/src/charging-station/BootstrapStateUtils.ts index e33b7686..95844840 100644 --- a/src/charging-station/BootstrapStateUtils.ts +++ b/src/charging-station/BootstrapStateUtils.ts @@ -13,6 +13,7 @@ import { ensureError, formatLogPrefix, handleFileException, + JSONStringify, logger, } from '../utils/index.js' @@ -147,7 +148,7 @@ export const writeStateFile = async ( } atomicWriteFileSync( stateFilePath, - JSON.stringify(stateData, undefined, 2), + JSONStringify(stateData, 2), FileType.SimulatorState, logPrefixFn?.() ?? '', { errorParams: { throwError: false } } diff --git a/src/charging-station/ChargingStation.ts b/src/charging-station/ChargingStation.ts index 7c7090e0..e4e4af98 100644 --- a/src/charging-station/ChargingStation.ts +++ b/src/charging-station/ChargingStation.ts @@ -41,6 +41,7 @@ import { type HeartbeatResponse, type IncomingRequest, type IncomingRequestCommand, + MapStringifyFormat, MessageType, MeterValueMeasurand, OCPPVersion, @@ -97,6 +98,7 @@ import { isEmpty, isNotEmptyArray, isNotEmptyString, + JSONStringify, logger, logPrefix, mergeDeepRight, @@ -2592,7 +2594,7 @@ export class ChargingStation extends EventEmitter { const beginId = PerformanceStatistics.beginMeasure(measureId) atomicWriteFileSync( this.configurationFile, - JSON.stringify(configurationData, undefined, 2), + JSONStringify(configurationData, 2, MapStringifyFormat.object), FileType.ChargingStationConfiguration, this.logPrefix() ) diff --git a/src/charging-station/TemplateSchema.ts b/src/charging-station/TemplateSchema.ts index 094336b6..749f7b7e 100644 --- a/src/charging-station/TemplateSchema.ts +++ b/src/charging-station/TemplateSchema.ts @@ -1,5 +1,7 @@ import { z } from 'zod' +// Direct path: the `utils/index.js` barrel re-exports ConfigurationSchema.ts which uses Zod enums, causing a TDZ cycle. +import { convertToIntOrNaN } from '../utils/Utils.js' import { CURRENT_SCHEMA_VERSION } from './TemplateMigrations.js' // --------------------------------------------------------------- @@ -262,17 +264,25 @@ export const TemplateSchema = BaseTemplateSchema.superRefine((template, ctx) => path: ['Connectors'], }) } - // Validate Evses topology (OCPP 2.0.1 §7.2 constraints) + // Validate Evses topology (OCPP 2.0.1 part 1 §7.1 EVSE numbering) if (hasEvses && template.Evses != null) { for (const [evseKey, evse] of Object.entries(template.Evses)) { - const evseId = Number(evseKey) + const evseId = convertToIntOrNaN(evseKey) + if (!Number.isInteger(evseId) || evseId < 0) { + ctx.addIssue({ + code: 'custom', + message: `EVSE key '${evseKey}' is not a valid non-negative integer (OCPP 2.0.1 part 1 §7.1)`, + path: ['Evses', evseKey], + }) + continue + } const connectorIds = Object.keys(evse.Connectors).map(Number) if (evseId === 0) { for (const connectorId of connectorIds) { if (connectorId !== 0) { ctx.addIssue({ code: 'custom', - message: `EVSE 0 has invalid connector id ${connectorId.toString()}, only connector id 0 is allowed (OCPP 2.0.1 §7.2)`, + message: `EVSE 0 has invalid connector id ${connectorId.toString()}, only connector id 0 is allowed (OCPP 2.0.1 part 1 §7.2)`, path: ['Evses', evseKey, 'Connectors', connectorId.toString()], }) } @@ -282,7 +292,7 @@ export const TemplateSchema = BaseTemplateSchema.superRefine((template, ctx) => if (connectorId < 1) { ctx.addIssue({ code: 'custom', - message: `EVSE ${evseId.toString()} has invalid connector id ${connectorId.toString()}, connector ids must start at 1 (OCPP 2.0.1 §7.2)`, + message: `EVSE ${evseId.toString()} has invalid connector id ${connectorId.toString()}, connector ids must start at 1 (OCPP 2.0.1 part 1 §7.2)`, path: ['Evses', evseKey, 'Connectors', connectorId.toString()], }) } diff --git a/src/charging-station/TemplateValidation.ts b/src/charging-station/TemplateValidation.ts index 1dc2d172..15ac7dbb 100644 --- a/src/charging-station/TemplateValidation.ts +++ b/src/charging-station/TemplateValidation.ts @@ -3,7 +3,7 @@ import type { ZodError } from 'zod' import type { ChargingStationTemplate } from '../types/index.js' import { BaseError } from '../exception/index.js' -import { isEmpty, isNotEmptyString, logger } from '../utils/index.js' +import { assertIsJsonObject, clone, isEmpty, isNotEmptyString, logger } from '../utils/index.js' import { getMaxConfiguredNumberOfConnectors } from './Helpers.js' import { applyMigration, coerceVersion, CURRENT_SCHEMA_VERSION } from './TemplateMigrations.js' import { TemplateSchema } from './TemplateSchema.js' @@ -18,6 +18,7 @@ export class TemplateValidationError extends BaseError { public readonly fieldErrors: { message: string; path: string }[] public readonly filePath: string public readonly migratedFrom?: number + public override readonly name = 'TemplateValidationError' as const public constructor (zodError: ZodError, context: { filePath: string; migratedFrom?: number }) { const fieldErrors = zodError.issues.map(issue => ({ @@ -47,11 +48,12 @@ export class TemplateValidationError extends BaseError { * @returns Validated and transformed ChargingStationTemplate */ export const validateTemplate = (parsed: unknown, filePath: string): ChargingStationTemplate => { - if (parsed == null || typeof parsed !== 'object' || Array.isArray(parsed)) { - throw new BaseError( + assertIsJsonObject( + parsed, + new BaseError( `${moduleName}.validateTemplate: Invalid charging station template payload (not a JSON object) in template file ${filePath}` ) - } + ) if (isEmpty(parsed)) { throw new BaseError( `${moduleName}.validateTemplate: Empty charging station information from template file ${filePath}` @@ -59,7 +61,7 @@ export const validateTemplate = (parsed: unknown, filePath: string): ChargingSta } // Clone before mutating $schemaVersion below and inside applyMigration, // so the caller's parsed JSON stays untouched. - const parsedRecord = structuredClone(parsed) as Record + const parsedRecord = clone(parsed) as Record const version = coerceVersion(parsedRecord.$schemaVersion) parsedRecord.$schemaVersion = version diff --git a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts index f65bc5ac..d903904c 100644 --- a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts +++ b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts @@ -698,15 +698,15 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { ) return OCPP16Constants.OCPP_CANCEL_RESERVATION_RESPONSE_ACCEPTED } catch (error) { + const errorResponse: GenericResponse = + OCPP16Constants.OCPP_CANCEL_RESERVATION_RESPONSE_REJECTED return ( handleIncomingRequestError( chargingStation, OCPP16IncomingRequestCommand.CANCEL_RESERVATION, ensureError(error), - { - errorResponse: OCPP16Constants.OCPP_CANCEL_RESERVATION_RESPONSE_REJECTED, - } - ) ?? OCPP16Constants.OCPP_CANCEL_RESERVATION_RESPONSE_REJECTED + { errorResponse } + ) ?? errorResponse ) } } @@ -782,6 +782,7 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { OCPP16StandardParametersKey.WebSocketPingInterval, ]) if (integerKeys.has(keyToChange.key as OCPP16StandardParametersKey)) { + // Number() preserved: rejection check relies on NaN-on-invalid; convertToInt would silently accept (returns 0 for null/undefined, truncates '1.5' → 1) or throw uncaught (for '', 'abc'). const numValue = Number(value) if (!Number.isInteger(numValue) || numValue < 0) { return OCPP16Constants.OCPP_CONFIGURATION_RESPONSE_REJECTED @@ -941,13 +942,15 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { } return OCPP16Constants.OCPP_DATA_TRANSFER_RESPONSE_ACCEPTED } catch (error) { + const errorResponse: OCPP16DataTransferResponse = + OCPP16Constants.OCPP_DATA_TRANSFER_RESPONSE_REJECTED return ( handleIncomingRequestError( chargingStation, OCPP16IncomingRequestCommand.DATA_TRANSFER, ensureError(error), - { errorResponse: OCPP16Constants.OCPP_DATA_TRANSFER_RESPONSE_REJECTED } - ) ?? OCPP16Constants.OCPP_DATA_TRANSFER_RESPONSE_REJECTED + { errorResponse } + ) ?? errorResponse ) } } @@ -1192,13 +1195,14 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { chargingStation.stationInfo.diagnosticsStatus = OCPP16DiagnosticsStatus.UploadFailed } ftpClient?.close() + const errorResponse: GetDiagnosticsResponse = OCPP16Constants.OCPP_RESPONSE_EMPTY return ( handleIncomingRequestError( chargingStation, OCPP16IncomingRequestCommand.GET_DIAGNOSTICS, ensureError(error), - { errorResponse: OCPP16Constants.OCPP_RESPONSE_EMPTY } - ) ?? OCPP16Constants.OCPP_RESPONSE_EMPTY + { errorResponse } + ) ?? errorResponse ) } } else { @@ -1450,13 +1454,15 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { if (errorConnectorStatus != null) { errorConnectorStatus.status = OCPP16ChargePointStatus.Available } + const errorResponse: OCPP16ReserveNowResponse = + OCPP16Constants.OCPP_RESERVATION_RESPONSE_FAULTED return ( handleIncomingRequestError( chargingStation, OCPP16IncomingRequestCommand.RESERVE_NOW, ensureError(error), - { errorResponse: OCPP16Constants.OCPP_RESERVATION_RESPONSE_FAULTED } - ) ?? OCPP16Constants.OCPP_RESERVATION_RESPONSE_FAULTED + { errorResponse } + ) ?? errorResponse ) } } diff --git a/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts b/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts index 5f3cd21b..2b68a6e8 100644 --- a/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts +++ b/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts @@ -52,6 +52,7 @@ import { } from '../../../types/index.js' import { clampToSafeTimerValue, + convertToBoolean, convertToDate, convertToInt, isNotEmptyArray, @@ -163,8 +164,7 @@ export class OCPP16ServiceUtils { } if ( OCPP16ServiceUtils.isSigningEnabled(chargingStation) && - getConfigurationKey(chargingStation, OCPP16VendorParametersKey.SampledDataSignStartedReadings) - ?.value === 'true' + OCPP16ServiceUtils.isSigningStartedReadingsEnabled(chargingStation) ) { const connectorStatus = chargingStation.getConnectorStatus(connectorId) const transactionId = connectorStatus?.transactionId ?? 0 @@ -673,9 +673,32 @@ export class OCPP16ServiceUtils { * @returns Whether signed meter value generation is enabled (SampledDataSignReadings=true) */ public static isSigningEnabled (chargingStation: ChargingStation): boolean { - return ( - getConfigurationKey(chargingStation, OCPP16VendorParametersKey.SampledDataSignReadings) - ?.value === 'true' + return convertToBoolean( + getConfigurationKey(chargingStation, OCPP16VendorParametersKey.SampledDataSignReadings)?.value + ) + } + + /** + * @param chargingStation - Target charging station + * @returns Whether signing of meter values at transaction start is enabled + * (SampledDataSignStartedReadings=true) + */ + public static isSigningStartedReadingsEnabled (chargingStation: ChargingStation): boolean { + return convertToBoolean( + getConfigurationKey(chargingStation, OCPP16VendorParametersKey.SampledDataSignStartedReadings) + ?.value + ) + } + + /** + * @param chargingStation - Target charging station + * @returns Whether signing of meter values during transaction updates is enabled + * (SampledDataSignUpdatedReadings=true) + */ + public static isSigningUpdatedReadingsEnabled (chargingStation: ChargingStation): boolean { + return convertToBoolean( + getConfigurationKey(chargingStation, OCPP16VendorParametersKey.SampledDataSignUpdatedReadings) + ?.value ) } @@ -811,10 +834,7 @@ export class OCPP16ServiceUtils { const meterValue = buildMeterValue(chargingStation, transactionId, interval) if ( OCPP16ServiceUtils.isSigningEnabled(chargingStation) && - getConfigurationKey( - chargingStation, - OCPP16VendorParametersKey.SampledDataSignUpdatedReadings - )?.value === 'true' + OCPP16ServiceUtils.isSigningUpdatedReadingsEnabled(chargingStation) ) { const energyWh = chargingStation.getEnergyActiveImportRegisterByTransactionId( connectorStatus.transactionId diff --git a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts index b3052af2..e81bb3df 100644 --- a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts +++ b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts @@ -132,11 +132,14 @@ import { UnlockStatusEnumType, UpdateFirmwareStatusEnumType, UploadLogStatusEnumType, + type UUIDv4, } from '../../../types/index.js' import { convertToDate, convertToIntOrNaN, + ensureError, generateUUID, + handleIncomingRequestError, isEmpty, isNotEmptyArray, isNotEmptyString, @@ -226,6 +229,22 @@ const buildStationInfoReportData = ( return reportData } +// OCPP 2.0.1 part 2 §F01.FR.13: when a transaction exists on the Charging Station but is not yet +// authorized (cable-plugin-first), echo its transactionId in RequestStartTransactionResponse. +// Otherwise omit — fabricating a UUID misleads CSMS that map remoteStartId → transactionId. +const buildRejectedResponse = ( + reasonCode: ReasonCodeEnumType, + additionalInfo: string, + transactionId?: UUIDv4 +): OCPP20RequestStartTransactionResponse => ({ + status: RequestStartStopStatusEnumType.Rejected, + statusInfo: { + additionalInfo, + reasonCode, + }, + ...(transactionId != null && { transactionId }), +}) + interface OCPP20StationState { activeFirmwareUpdateAbortController?: AbortController activeFirmwareUpdateRequestId?: number @@ -813,11 +832,15 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { ) return OCPP20Constants.OCPP_RESPONSE_ACCEPTED } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestClearCache: Error clearing cache:`, - error + const errorResponse: OCPP20ClearCacheResponse = OCPP20Constants.OCPP_RESPONSE_REJECTED + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.CLEAR_CACHE, + ensureError(error), + { errorResponse } + ) ?? errorResponse ) - return OCPP20Constants.OCPP_RESPONSE_REJECTED } } @@ -849,11 +872,15 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { ) return { versionNumber: version } } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestGetLocalListVersion: Error getting version:`, - error + const errorResponse: OCPP20GetLocalListVersionResponse = { versionNumber: 0 } + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.GET_LOCAL_LIST_VERSION, + ensureError(error), + { errorResponse } + ) ?? errorResponse ) - return { versionNumber: 0 } } } @@ -959,11 +986,16 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { ) return OCPP20Constants.OCPP_SEND_LOCAL_LIST_RESPONSE_ACCEPTED } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestSendLocalList: Error updating local auth list:`, - error + const errorResponse: OCPP20SendLocalListResponse = + OCPP20Constants.OCPP_SEND_LOCAL_LIST_RESPONSE_FAILED + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.SEND_LOCAL_LIST, + ensureError(error), + { errorResponse } + ) ?? errorResponse ) - return OCPP20Constants.OCPP_SEND_LOCAL_LIST_RESPONSE_FAILED } } @@ -1581,17 +1613,21 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { status: GenericStatus.Accepted, } } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestCertificateSigned: Certificate chain storage failed`, - error - ) - return { + const errorResponse: OCPP20CertificateSignedResponse = { status: GenericStatus.Rejected, statusInfo: { additionalInfo: 'Failed to store certificate chain due to a storage error', reasonCode: ReasonCodeEnumType.OutOfStorage, }, } + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.CERTIFICATE_SIGNED, + ensureError(error), + { errorResponse } + ) ?? errorResponse + ) } } @@ -1847,17 +1883,21 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { }, } } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestDeleteCertificate: Certificate deletion failed`, - error - ) - return { + const errorResponse: OCPP20DeleteCertificateResponse = { status: DeleteCertificateStatusEnumType.Failed, statusInfo: { additionalInfo: 'Certificate deletion failed due to an unexpected error', reasonCode: ReasonCodeEnumType.InternalError, }, } + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.DELETE_CERTIFICATE, + ensureError(error), + { errorResponse } + ) ?? errorResponse + ) } } @@ -1952,17 +1992,21 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { : GetInstalledCertificateStatusEnumType.NotFound, } } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestGetInstalledCertificateIds: Failed to retrieve certificates`, - error - ) - return { + const errorResponse: OCPP20GetInstalledCertificateIdsResponse = { status: GetInstalledCertificateStatusEnumType.NotFound, statusInfo: { additionalInfo: 'Failed to retrieve installed certificates due to an unexpected error', reasonCode: ReasonCodeEnumType.InternalError, }, } + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.GET_INSTALLED_CERTIFICATE_IDS, + ensureError(error), + { errorResponse } + ) ?? errorResponse + ) } } @@ -2107,17 +2151,21 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { status: InstallCertificateStatusEnumType.Accepted, } } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestInstallCertificate: Certificate storage failed for type ${certificateType}`, - error - ) - return { + const errorResponse: OCPP20InstallCertificateResponse = { status: InstallCertificateStatusEnumType.Failed, statusInfo: { additionalInfo: 'Failed to store certificate due to a storage error', reasonCode: ReasonCodeEnumType.OutOfStorage, }, } + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.INSTALL_CERTIFICATE, + ensureError(error), + { errorResponse } + ) ?? errorResponse + ) } } @@ -2327,18 +2375,21 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } } } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestReset: Error handling reset request:`, - error - ) - - return { + const errorResponse: OCPP20ResetResponse = { status: ResetStatusEnumType.Rejected, statusInfo: { additionalInfo: 'Internal error occurred while processing reset request', reasonCode: ReasonCodeEnumType.InternalError, }, } + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.RESET, + ensureError(error), + { errorResponse } + ) ?? errorResponse + ) } } @@ -2462,13 +2513,10 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { logger.warn( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: No available EVSE for remote start` ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: 'No available EVSE found for remote start', - reasonCode: ReasonCodeEnumType.NotFound, - }, - } + return buildRejectedResponse( + ReasonCodeEnumType.NotFound, + 'No available EVSE found for remote start' + ) } logger.info( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: Auto-selected EVSE ${resolvedEvseId.toString()}` @@ -2505,6 +2553,25 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { ) } + // OCPP 2.0.1 part 2 §F01.FR.13: when a transaction was created on the station but not yet + // authorized (transactionPending), the existing transactionId SHALL be echoed back. The + // appendix distinguishes TxStarted (cable-plugin-first, not yet authorized) from TxInProgress + // (authorized and running) — use TxStarted here to match the precondition. + if ( + connectorStatus.transactionPending === true && + typeof connectorStatus.transactionId === 'string' + ) { + logger.warn( + `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: Connector ${connectorId.toString()} has a pending transaction not yet authorized` + ) + return buildRejectedResponse( + ReasonCodeEnumType.TxStarted, + `Connector ${connectorId.toString()} has a pending transaction not yet authorized`, + // safe: OCPP 2.0 paths always store generateUUID() output here (see line ~2740) + connectorStatus.transactionId as UUIDv4 + ) + } + if ( connectorStatus.transactionStarted === true || connectorStatus.transactionPending === true || @@ -2513,14 +2580,10 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { logger.warn( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: Connector ${connectorId.toString()} already has an active or pending transaction` ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: `Connector ${connectorId.toString()} already has an active or pending transaction`, - reasonCode: ReasonCodeEnumType.TxInProgress, - }, - transactionId: generateUUID(), - } + return buildRejectedResponse( + ReasonCodeEnumType.TxInProgress, + `Connector ${connectorId.toString()} already has an active or pending transaction` + ) } const shouldAuthorizeRemoteStart = OCPP20ServiceUtils.readVariableAsBoolean( @@ -2546,14 +2609,10 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { logger.debug( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: IdToken with MasterPassGroupId group cannot start a transaction` ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: 'MasterPassGroupId tokens cannot start transactions', - reasonCode: ReasonCodeEnumType.InvalidIdToken, - }, - transactionId: generateUUID(), - } + return buildRejectedResponse( + ReasonCodeEnumType.InvalidIdToken, + 'MasterPassGroupId tokens cannot start transactions' + ) } try { @@ -2567,16 +2626,12 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } catch (error) { logger.error( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: Authorization error for '${truncateId(idToken.idToken)}':`, - error + ensureError(error) + ) + return buildRejectedResponse( + ReasonCodeEnumType.InternalError, + 'Authorization error occurred' ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: 'Authorization error occurred', - reasonCode: ReasonCodeEnumType.InternalError, - }, - transactionId: generateUUID(), - } } } else { logger.info( @@ -2585,14 +2640,10 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } if (!isAuthorized) { - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: `IdToken '${truncateId(idToken.idToken)}' is not authorized`, - reasonCode: ReasonCodeEnumType.InvalidIdToken, - }, - transactionId: generateUUID(), - } + return buildRejectedResponse( + ReasonCodeEnumType.InvalidIdToken, + `IdToken '${truncateId(idToken.idToken)}' is not authorized` + ) } if (groupIdToken != null) { @@ -2608,26 +2659,18 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } catch (error) { logger.error( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: Group authorization error for '${truncateId(groupIdToken.idToken)}':`, - error + ensureError(error) + ) + return buildRejectedResponse( + ReasonCodeEnumType.InternalError, + 'Group authorization error occurred' ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: 'Group authorization error occurred', - reasonCode: ReasonCodeEnumType.InternalError, - }, - transactionId: generateUUID(), - } } if (!isGroupAuthorized) { - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: `GroupIdToken '${truncateId(groupIdToken.idToken)}' is not authorized`, - reasonCode: ReasonCodeEnumType.InvalidIdToken, - }, - transactionId: generateUUID(), - } + return buildRejectedResponse( + ReasonCodeEnumType.InvalidIdToken, + `GroupIdToken '${truncateId(groupIdToken.idToken)}' is not authorized` + ) } } @@ -2639,14 +2682,10 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { logger.warn( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: ChargingProfile must have purpose TxProfile for RequestStartTransaction, got ${chargingProfile.chargingProfilePurpose}` ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: 'ChargingProfile must have purpose TxProfile', - reasonCode: ReasonCodeEnumType.InvalidProfile, - }, - transactionId: generateUUID(), - } + return buildRejectedResponse( + ReasonCodeEnumType.InvalidProfile, + 'ChargingProfile must have purpose TxProfile' + ) } // OCPP 2.0.1 §2.10: transactionId MUST NOT be set in RequestStartTransaction @@ -2654,14 +2693,10 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { logger.warn( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: ChargingProfile transactionId must not be set for RequestStartTransaction` ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: 'ChargingProfile transactionId must not be set', - reasonCode: ReasonCodeEnumType.InvalidValue, - }, - transactionId: generateUUID(), - } + return buildRejectedResponse( + ReasonCodeEnumType.InvalidProfile, + 'ChargingProfile transactionId must not be set' + ) } let isValidProfile = false try { @@ -2673,29 +2708,18 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } catch (error) { logger.error( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: Charging profile validation error:`, - error + ensureError(error) + ) + return buildRejectedResponse( + ReasonCodeEnumType.InternalError, + 'Charging profile validation error' ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: 'Charging profile validation error', - reasonCode: ReasonCodeEnumType.InternalError, - }, - transactionId: generateUUID(), - } } if (!isValidProfile) { logger.warn( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: Invalid charging profile` ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: 'Invalid charging profile', - reasonCode: ReasonCodeEnumType.InvalidProfile, - }, - transactionId: generateUUID(), - } + return buildRejectedResponse(ReasonCodeEnumType.InvalidProfile, 'Invalid charging profile') } } @@ -2738,16 +2762,9 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { await this.resetConnectorOnStartTransactionError(chargingStation, connectorId, resolvedEvseId) logger.error( `${chargingStation.logPrefix()} ${moduleName}.handleRequestStartTransaction: Error starting transaction:`, - error + ensureError(error) ) - return { - status: RequestStartStopStatusEnumType.Rejected, - statusInfo: { - additionalInfo: 'Error starting transaction', - reasonCode: ReasonCodeEnumType.InternalError, - }, - transactionId: generateUUID(), - } + return buildRejectedResponse(ReasonCodeEnumType.InternalError, 'Error starting transaction') } } @@ -2862,18 +2879,21 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } } } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestTriggerMessage: Error handling trigger message request:`, - error - ) - - return { + const errorResponse: OCPP20TriggerMessageResponse = { status: TriggerMessageStatusEnumType.Rejected, statusInfo: { additionalInfo: 'Internal error occurred while processing trigger message request', reasonCode: ReasonCodeEnumType.InternalError, }, } + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.TRIGGER_MESSAGE, + ensureError(error), + { errorResponse } + ) ?? errorResponse + ) } } @@ -2947,18 +2967,21 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { chargingStation.unlockConnector(connectorId) return { status: UnlockStatusEnumType.Unlocked } } catch (error) { - logger.error( - `${chargingStation.logPrefix()} ${moduleName}.handleRequestUnlockConnector: Error handling unlock connector request:`, - error - ) - - return { + const errorResponse: OCPP20UnlockConnectorResponse = { status: UnlockStatusEnumType.UnlockFailed, statusInfo: { additionalInfo: 'Internal error occurred while processing unlock connector request', reasonCode: ReasonCodeEnumType.InternalError, }, } + return ( + handleIncomingRequestError( + chargingStation, + OCPP20IncomingRequestCommand.UNLOCK_CONNECTOR, + ensureError(error), + { errorResponse } + ) ?? errorResponse + ) } } diff --git a/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts b/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts index dbc9a931..301ecbdf 100644 --- a/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts +++ b/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts @@ -47,6 +47,7 @@ import { convertToIntOrNaN, formatDurationMilliSeconds, generateUUID, + getErrorMessage, isNotEmptyArray, logger, sleep, @@ -176,7 +177,7 @@ export class OCPP20ServiceUtils { return isNotEmptyArray(startedMeterValue.sampledValue) ? [startedMeterValue] : [] } catch (error) { logger.warn( - `${chargingStation.logPrefix()} ${moduleName}.buildTransactionStartedMeterValues: ${(error as Error).message}` + `${chargingStation.logPrefix()} ${moduleName}.buildTransactionStartedMeterValues: ${getErrorMessage(error)}` ) return [] } @@ -515,7 +516,7 @@ export class OCPP20ServiceUtils { return convertToInt(value) } catch { logger.warn( - `${moduleName}.readVariableAsInteger: Cannot convert '${value}' to integer for ${componentName}.${variableName}, using default ${defaultValue.toString()}` + `${moduleName}.readVariableAsInteger: Cannot convert '${value}' to integer for ${buildConfigKey(componentName, variableName)}, using default ${defaultValue.toString()}` ) return defaultValue } @@ -1178,7 +1179,7 @@ export class OCPP20ServiceUtils { } } catch (error) { logger.warn( - `${chargingStation.logPrefix()} ${moduleName}.buildTransactionEndedMeterValues: ${(error as Error).message}` + `${chargingStation.logPrefix()} ${moduleName}.buildTransactionEndedMeterValues: ${getErrorMessage(error)}` ) } const meterValues: OCPP20MeterValue[] = [ diff --git a/src/charging-station/ocpp/OCPPIncomingRequestService.ts b/src/charging-station/ocpp/OCPPIncomingRequestService.ts index 4ed9a0b7..93c29087 100644 --- a/src/charging-station/ocpp/OCPPIncomingRequestService.ts +++ b/src/charging-station/ocpp/OCPPIncomingRequestService.ts @@ -11,7 +11,7 @@ import { type JsonType, type OCPPVersion, } from '../../types/index.js' -import { isAsyncFunction, logger } from '../../utils/index.js' +import { isAsyncFunction, JSONStringify, logger } from '../../utils/index.js' import { type Ajv, createAjv, validatePayload } from './OCPPServiceUtils.js' export abstract class OCPPIncomingRequestService extends EventEmitter { @@ -67,11 +67,7 @@ export abstract class OCPPIncomingRequestService extends EventEmitter { ) { throw new OCPPError( ErrorType.SECURITY_ERROR, - `${commandName} cannot be issued to handle request PDU ${JSON.stringify( - commandPayload, - undefined, - 2 - )} while the charging station is in pending state on the ${this.csmsName}`, + `${commandName} cannot be issued to handle request PDU ${JSONStringify(commandPayload, 2)} while the charging station is in pending state on the ${this.csmsName}`, commandName, commandPayload ) @@ -114,11 +110,7 @@ export abstract class OCPPIncomingRequestService extends EventEmitter { // Throw exception throw new OCPPError( ErrorType.NOT_IMPLEMENTED, - `${commandName} is not implemented to handle request PDU ${JSON.stringify( - commandPayload, - undefined, - 2 - )}`, + `${commandName} is not implemented to handle request PDU ${JSONStringify(commandPayload, 2)}`, commandName, commandPayload ) @@ -126,11 +118,7 @@ export abstract class OCPPIncomingRequestService extends EventEmitter { } else { throw new OCPPError( ErrorType.SECURITY_ERROR, - `${commandName} cannot be issued to handle request PDU ${JSON.stringify( - commandPayload, - undefined, - 2 - )} while the charging station is not registered on the ${this.csmsName}`, + `${commandName} cannot be issued to handle request PDU ${JSONStringify(commandPayload, 2)} while the charging station is not registered on the ${this.csmsName}`, commandName, commandPayload ) diff --git a/src/charging-station/ocpp/OCPPResponseService.ts b/src/charging-station/ocpp/OCPPResponseService.ts index 21d248cc..224226ec 100644 --- a/src/charging-station/ocpp/OCPPResponseService.ts +++ b/src/charging-station/ocpp/OCPPResponseService.ts @@ -11,7 +11,7 @@ import { type RequestCommand, type ResponseHandler, } from '../../types/index.js' -import { Constants, isAsyncFunction, logger } from '../../utils/index.js' +import { Constants, isAsyncFunction, JSONStringify, logger } from '../../utils/index.js' import { type Ajv, createAjv, validatePayload } from './OCPPServiceUtils.js' export abstract class OCPPResponseService { @@ -97,11 +97,7 @@ export abstract class OCPPResponseService { // Throw exception throw new OCPPError( ErrorType.NOT_IMPLEMENTED, - `${commandName} is not implemented to handle response PDU ${JSON.stringify( - payload, - undefined, - 2 - )}`, + `${commandName} is not implemented to handle response PDU ${JSONStringify(payload, 2)}`, commandName, payload ) @@ -109,11 +105,7 @@ export abstract class OCPPResponseService { } else { throw new OCPPError( ErrorType.SECURITY_ERROR, - `${commandName} cannot be issued to handle response PDU ${JSON.stringify( - payload, - undefined, - 2 - )} while the charging station is not registered on the ${this.csmsName}`, + `${commandName} cannot be issued to handle response PDU ${JSONStringify(payload, 2)} while the charging station is not registered on the ${this.csmsName}`, commandName, payload ) diff --git a/src/charging-station/ocpp/OCPPServiceUtils.ts b/src/charging-station/ocpp/OCPPServiceUtils.ts index beac5cc1..a34daef7 100644 --- a/src/charging-station/ocpp/OCPPServiceUtils.ts +++ b/src/charging-station/ocpp/OCPPServiceUtils.ts @@ -6,7 +6,13 @@ import { readFileSync } from 'node:fs' import { dirname, join } from 'node:path' import { fileURLToPath } from 'node:url' -import type { BootReasonEnumType, SigningMethodEnumType } from '../../types/index.js' +import type { + BootReasonEnumType, + OCPP20OptionalVariableName, + OCPP20RequiredVariableName, + OCPP20VendorVariableName, + SigningMethodEnumType, +} from '../../types/index.js' import { buildConfigKey, @@ -46,6 +52,7 @@ import { ACElectricUtils, clone, Constants, + convertToBoolean, convertToFloat, convertToInt, DCElectricUtils, @@ -54,6 +61,7 @@ import { handleFileException, isNotEmptyArray, isNotEmptyString, + JSONStringify, logger, logPrefix, max, @@ -81,6 +89,13 @@ const SOC_MAXIMUM_VALUE = 100 const UNIT_DIVIDER_KILO = 1000 const MS_PER_HOUR = 3_600_000 +const isOCPP20FlagEnabled = ( + chargingStation: ChargingStation, + component: OCPP20ComponentName, + variable: OCPP20OptionalVariableName | OCPP20RequiredVariableName | OCPP20VendorVariableName +): boolean => + convertToBoolean(getConfigurationKey(chargingStation, buildConfigKey(component, variable))?.value) + export type Ajv = _Ajv.default // eslint-disable-next-line @typescript-eslint/no-redeclare const Ajv = _Ajv.default @@ -189,7 +204,7 @@ export const validatePayload = ( ajvErrorsToErrorType(validate.errors), `${context.charAt(0).toUpperCase()}${context.slice(1)} PDU is invalid`, commandName, - JSON.stringify(validate.errors, undefined, 2) + JSONStringify(validate.errors, 2) ) } @@ -933,36 +948,30 @@ export const buildMeterValue = ( ) } { - const signReadings = - getConfigurationKey( - chargingStation, - buildConfigKey(OCPP20ComponentName.SampledDataCtrlr, StandardParametersKey.SignReadings) - )?.value === 'true' + const signReadings = isOCPP20FlagEnabled( + chargingStation, + OCPP20ComponentName.SampledDataCtrlr, + StandardParametersKey.SignReadings + ) if (signReadings) { let signingEnabledForContext = true if (context === OCPP20ReadingContextEnumType.TRANSACTION_BEGIN) { - signingEnabledForContext = - getConfigurationKey( - chargingStation, - buildConfigKey( - OCPP20ComponentName.SampledDataCtrlr, - VendorParametersKey.SignStartedReadings - ) - )?.value === 'true' + signingEnabledForContext = isOCPP20FlagEnabled( + chargingStation, + OCPP20ComponentName.SampledDataCtrlr, + VendorParametersKey.SignStartedReadings + ) } else if ( context == null || context === OCPP20ReadingContextEnumType.SAMPLE_PERIODIC || context === OCPP20ReadingContextEnumType.SAMPLE_CLOCK ) { - signingEnabledForContext = - getConfigurationKey( - chargingStation, - buildConfigKey( - OCPP20ComponentName.SampledDataCtrlr, - VendorParametersKey.SignUpdatedReadings - ) - )?.value === 'true' + signingEnabledForContext = isOCPP20FlagEnabled( + chargingStation, + OCPP20ComponentName.SampledDataCtrlr, + VendorParametersKey.SignUpdatedReadings + ) } if (signingEnabledForContext) { diff --git a/src/charging-station/ocpp/OCPPSignedMeterValueUtils.ts b/src/charging-station/ocpp/OCPPSignedMeterValueUtils.ts index fb2bd6c3..dc630018 100644 --- a/src/charging-station/ocpp/OCPPSignedMeterValueUtils.ts +++ b/src/charging-station/ocpp/OCPPSignedMeterValueUtils.ts @@ -6,7 +6,7 @@ import { type SampledValue, SigningMethodEnumType, } from '../../types/index.js' -import { logger } from '../../utils/index.js' +import { getErrorMessage, isNotEmptyString, logger } from '../../utils/index.js' export interface SampledValueSigningConfig extends SigningConfig { enabled: boolean @@ -53,7 +53,7 @@ export const deriveSigningMethodFromPublicKeyHex = ( return namedCurve != null ? NODE_CURVE_TO_SIGNING_METHOD.get(namedCurve) : undefined } catch (error) { logger.debug( - `deriveSigningMethodFromPublicKeyHex: failed to parse public key: ${(error as Error).message}` + `deriveSigningMethodFromPublicKeyHex: failed to parse public key: ${getErrorMessage(error)}` ) return undefined } @@ -73,7 +73,7 @@ export const validateSigningPrerequisites = ( publicKeyHex: string | undefined, configuredSigningMethod: SigningMethodEnumType | undefined ): SigningPrerequisiteResult | SigningPrerequisiteSuccess => { - if (publicKeyHex == null || publicKeyHex.length === 0) { + if (!isNotEmptyString(publicKeyHex)) { return { enabled: false, reason: 'Public key is not configured' } } diff --git a/src/charging-station/ocpp/auth/types/AuthTypes.ts b/src/charging-station/ocpp/auth/types/AuthTypes.ts index 3a8f8833..ce0a0488 100644 --- a/src/charging-station/ocpp/auth/types/AuthTypes.ts +++ b/src/charging-station/ocpp/auth/types/AuthTypes.ts @@ -281,7 +281,7 @@ export class AuthenticationError extends Error { public readonly code: AuthErrorCode public readonly context?: AuthContext public readonly identifier?: string - public override name = 'AuthenticationError' + public override readonly name = 'AuthenticationError' as const public readonly ocppVersion?: OCPPVersion diff --git a/src/charging-station/ui-server/AbstractUIServer.ts b/src/charging-station/ui-server/AbstractUIServer.ts index 32766830..be1a7703 100644 --- a/src/charging-station/ui-server/AbstractUIServer.ts +++ b/src/charging-station/ui-server/AbstractUIServer.ts @@ -26,7 +26,7 @@ import { type UIServerConfiguration, type UUIDv4, } from '../../types/index.js' -import { isEmpty, isNotEmptyString, logger, logPrefix } from '../../utils/index.js' +import { isEmpty, isNotEmptyArray, isNotEmptyString, logger, logPrefix } from '../../utils/index.js' import { UIServiceFactory } from './ui-services/UIServiceFactory.js' import { createUIServerAccessCache, @@ -665,7 +665,9 @@ export abstract class AbstractUIServer { */ protected runRequestPrologue (req: IncomingMessage): UIServerRequestPrologueResult { const decision = resolveUIServerAccess(req, this.uiServerConfiguration, this.accessCache) - const rateLimitKey = decision.clientAddress.length > 0 ? decision.clientAddress : 'unknown' + const rateLimitKey = isNotEmptyString(decision.clientAddress) + ? decision.clientAddress + : 'unknown' if (!this.rateLimiter(rateLimitKey)) { logger.warn( `${this.logPrefix( @@ -1314,7 +1316,7 @@ export abstract class AbstractUIServer { const isWildcard = configuredHost === '' || configuredHost === '0.0.0.0' || configuredHost === '::' - if (isWildcard && allowedHosts.length === 0) { + if (isWildcard && isEmpty(allowedHosts)) { logger.warn( `${this.logPrefix( moduleName, @@ -1323,7 +1325,7 @@ export abstract class AbstractUIServer { ) return } - if (!isWildcard && !isLoopback(configuredHost) && requireTls && trustedProxies.length === 0) { + if (!isWildcard && !isLoopback(configuredHost) && requireTls && isEmpty(trustedProxies)) { logger.warn( `${this.logPrefix( moduleName, @@ -1435,7 +1437,7 @@ const addPerStationStatusInfo = ( * @returns Connector count under the active mode. */ const countConnectors = (data: ChargingStationData): number => - data.connectors.length > 0 + isNotEmptyArray(data.connectors) ? data.connectors.length : data.evses.reduce((n, evse) => n + evse.evseStatus.connectors.size, 0) @@ -1448,7 +1450,7 @@ const countConnectors = (data: ChargingStationData): number => * @yields {ConnectorEntry} A connector entry under the active mode. */ const iterateConnectors = function * (data: ChargingStationData): Generator { - if (data.connectors.length > 0) { + if (isNotEmptyArray(data.connectors)) { for (const entry of data.connectors) { yield entry } diff --git a/src/charging-station/ui-server/UIServerAccessPolicy.ts b/src/charging-station/ui-server/UIServerAccessPolicy.ts index 5d14ad88..3c3670f1 100644 --- a/src/charging-station/ui-server/UIServerAccessPolicy.ts +++ b/src/charging-station/ui-server/UIServerAccessPolicy.ts @@ -3,6 +3,7 @@ import type { IncomingMessage } from 'node:http' import type { UIServerConfiguration } from '../../types/index.js' import { UI_SERVER_ACCESS_POLICY_DEFAULTS } from '../../utils/ConfigurationSchema.js' +import { has, isEmpty, isNotEmptyArray } from '../../utils/index.js' import { isLoopback, normalizeHost, @@ -227,7 +228,7 @@ const getForwardedClientAddress = ( return picked } const addresses = splitHeaderList(picked.value) - if (addresses.length === 0) { + if (isEmpty(addresses)) { return { kind: 'error', reason: UIServerAccessDenialReason.InvalidForwardedClient } } // Multi-hop X-Forwarded-For chains are intentionally rejected: ambiguity in @@ -265,7 +266,7 @@ const getForwardedProtocol = ( } if (xForwardedProtocol != null) { const protocols = splitHeaderList(xForwardedProtocol) - if (protocols.length === 0) { + if (isEmpty(protocols)) { return { kind: 'error', reason: UIServerAccessDenialReason.InvalidForwardedProtocol } } if (protocols.length > 1) { @@ -294,7 +295,7 @@ const getForwardedHost = ( } if (xForwardedHost != null) { const hosts = splitHeaderList(xForwardedHost) - if (hosts.length === 0) { + if (isEmpty(hosts)) { return { kind: 'error', reason: UIServerAccessDenialReason.InvalidForwardedHost } } if (hosts.length > 1) { @@ -352,7 +353,7 @@ const parseSingleForwardedHeader = (req: IncomingMessage): ParseOutcome ): boolean => { const allowedHosts = getAllowedHosts(uiServerConfiguration) - if (allowedHosts.length === 0) { + if (isEmpty(allowedHosts)) { return false } const host = getSingleHeaderValue(req, 'host') @@ -435,12 +436,12 @@ const isOriginAllowed = ( return false } const allowedOrigins = uiServerConfiguration.accessPolicy?.allowedOrigins ?? [] - if (allowedOrigins.length > 0) { + if (isNotEmptyArray(allowedOrigins)) { return allowedOrigins.some(allowedOrigin => isSameOrigin(originUrl, allowedOrigin)) } const allowedHosts = getAllowedHosts(uiServerConfiguration) return ( - allowedHosts.length > 0 && + isNotEmptyArray(allowedHosts) && allowedHosts.some(allowedHost => isSameHost(originUrl.hostname, allowedHost)) ) } @@ -505,7 +506,7 @@ const getTrustedProxies = ( } const isTrustedProxy = (remoteAddress: string, trustedProxies: ReadonlySet): boolean => { - if (trustedProxies.size === 0) { + if (isEmpty(trustedProxies)) { return false } const normalizedRemoteAddress = normalizeIPAddress(remoteAddress) diff --git a/src/charging-station/ui-server/UIServerNet.ts b/src/charging-station/ui-server/UIServerNet.ts index 5cd329f1..9d4c403c 100644 --- a/src/charging-station/ui-server/UIServerNet.ts +++ b/src/charging-station/ui-server/UIServerNet.ts @@ -1,5 +1,8 @@ import { isIP } from 'node:net' +// Direct path: the `utils/index.js` barrel re-exports ConfigurationSchema.ts which imports from this module, causing a TDZ cycle. +import { convertToInt } from '../../utils/Utils.js' + export const LOOPBACK_HOSTNAME = 'localhost' export const isLoopback = (address: string): boolean => { @@ -94,7 +97,7 @@ const isValidPort = (port: string | undefined): boolean => { if (!/^\d+$/.test(port)) { return false } - const parsedPort = Number.parseInt(port, 10) + const parsedPort = convertToInt(port) return parsedPort >= 1 && parsedPort <= 65535 } diff --git a/src/charging-station/ui-server/UIServerSecurity.ts b/src/charging-station/ui-server/UIServerSecurity.ts index c99646dd..6cb5cf83 100644 --- a/src/charging-station/ui-server/UIServerSecurity.ts +++ b/src/charging-station/ui-server/UIServerSecurity.ts @@ -17,6 +17,8 @@ export const DEFAULT_MAX_TRACKED_IPS = 10000 export const DEFAULT_COMPRESSION_THRESHOLD_BYTES = 1024 export class PayloadTooLargeError extends BaseError { + public override readonly name = 'PayloadTooLargeError' as const + public constructor (maxBytes: number) { super(`Request body exceeds limit of ${maxBytes.toString()} bytes`) } diff --git a/src/charging-station/ui-server/mcp/MCPResources.ts b/src/charging-station/ui-server/mcp/MCPResources.ts index 516f68ea..55a7a2e3 100644 --- a/src/charging-station/ui-server/mcp/MCPResources.ts +++ b/src/charging-station/ui-server/mcp/MCPResources.ts @@ -5,7 +5,8 @@ import { fileURLToPath } from 'node:url' import type { AbstractUIServer } from '../AbstractUIServer.js' -import { OCPPVersion } from '../../../types/index.js' +import { MapStringifyFormat, OCPPVersion } from '../../../types/index.js' +import { JSONStringify } from '../../../utils/index.js' export const registerMCPResources = (server: McpServer, uiServer: AbstractUIServer): void => { server.registerResource( @@ -19,7 +20,7 @@ export const registerMCPResources = (server: McpServer, uiServer: AbstractUIServ contents: [ { mimeType: 'application/json', - text: JSON.stringify(uiServer.listChargingStationData(), null, 2), + text: JSONStringify(uiServer.listChargingStationData(), 2, MapStringifyFormat.object), uri: 'station://list', }, ], @@ -41,7 +42,7 @@ export const registerMCPResources = (server: McpServer, uiServer: AbstractUIServ mimeType: 'application/json', text: data != null - ? JSON.stringify(data, null, 2) + ? JSONStringify(data, 2, MapStringifyFormat.object) : JSON.stringify({ error: `Station '${hashId as string}' not found` }), uri: uri.href, }, @@ -61,7 +62,7 @@ export const registerMCPResources = (server: McpServer, uiServer: AbstractUIServ contents: [ { mimeType: 'application/json', - text: JSON.stringify(uiServer.getChargingStationTemplates(), null, 2), + text: JSONStringify(uiServer.getChargingStationTemplates(), 2), uri: 'template://list', }, ], @@ -116,7 +117,7 @@ export const registerMCPSchemaResources = (server: McpServer): void => { contents: [ { mimeType: 'application/json', - text: JSON.stringify({ commands, count: commands.length, version }, null, 2), + text: JSONStringify({ commands, count: commands.length, version }, 2), uri: `schema://ocpp/${version}`, }, ], diff --git a/src/charging-station/ui-server/ui-services/AbstractUIService.ts b/src/charging-station/ui-server/ui-services/AbstractUIService.ts index 5deeade3..de1f0b9d 100644 --- a/src/charging-station/ui-server/ui-services/AbstractUIService.ts +++ b/src/charging-station/ui-server/ui-services/AbstractUIService.ts @@ -27,6 +27,7 @@ import { ensureError, getErrorMessage, isNotEmptyArray, + JSONStringify, logger, } from '../../../utils/index.js' import { UIServiceWorkerBroadcastChannel } from '../../broadcast-channel/UIServiceWorkerBroadcastChannel.js' @@ -169,11 +170,7 @@ export abstract class AbstractUIService { if (!this.requestHandlers.has(command)) { throw new BaseError( - `'${command}' is not implemented to handle message payload ${JSON.stringify( - requestPayload, - undefined, - 2 - )}` + `'${command}' is not implemented to handle message payload ${JSONStringify(requestPayload, 2)}` ) } diff --git a/src/exception/OCPPError.ts b/src/exception/OCPPError.ts index f29f14f2..4f5477aa 100644 --- a/src/exception/OCPPError.ts +++ b/src/exception/OCPPError.ts @@ -10,6 +10,7 @@ export class OCPPError extends BaseError { code: ErrorType command: IncomingRequestCommand | RequestCommand details?: JsonType + public override readonly name = 'OCPPError' as const constructor ( code: ErrorType, diff --git a/src/exception/TimeoutError.ts b/src/exception/TimeoutError.ts new file mode 100644 index 00000000..21223287 --- /dev/null +++ b/src/exception/TimeoutError.ts @@ -0,0 +1,8 @@ +// Partial Copyright Jerome Benoit. 2021-2025. All Rights Reserved. + +import { BaseError } from './BaseError.js' + +/** Sentinel error used by `promiseWithTimeout` call sites to discriminate timeout from upstream errors via `instanceof`. */ +export class TimeoutError extends BaseError { + public override readonly name = 'TimeoutError' as const +} diff --git a/src/exception/index.ts b/src/exception/index.ts index 0916eecb..b9df000d 100644 --- a/src/exception/index.ts +++ b/src/exception/index.ts @@ -1,2 +1,3 @@ export { BaseError } from './BaseError.js' export { OCPPError } from './OCPPError.js' +export { TimeoutError } from './TimeoutError.js' diff --git a/src/utils/Configuration.ts b/src/utils/Configuration.ts index fb2f65af..cab09f6c 100644 --- a/src/utils/Configuration.ts +++ b/src/utils/Configuration.ts @@ -289,8 +289,8 @@ export class Configuration { } if (isCFEnvironment()) { delete uiServerConfiguration.options?.host - if (uiServerConfiguration.options != null) { - uiServerConfiguration.options.port = convertToInt(env.PORT ?? '') + if (uiServerConfiguration.options != null && isNotEmptyString(env.PORT)) { + uiServerConfiguration.options.port = convertToInt(env.PORT) } } return uiServerConfiguration @@ -385,9 +385,7 @@ export class Configuration { */ private static async performReload (): Promise { const previousData = - Configuration.configurationData != null - ? structuredClone(Configuration.configurationData) - : undefined + Configuration.configurationData != null ? clone(Configuration.configurationData) : undefined const previousCache = new Map(Configuration.configurationSectionCache) let reloadSucceeded = false try { diff --git a/src/utils/ConfigurationMigrations.ts b/src/utils/ConfigurationMigrations.ts index e4da4598..b4ca14a2 100644 --- a/src/utils/ConfigurationMigrations.ts +++ b/src/utils/ConfigurationMigrations.ts @@ -2,6 +2,7 @@ import { isDeepStrictEqual } from 'node:util' // Direct path: the `exception/index.js` barrel re-exports OCPPError, causing a TDZ cycle. import { BaseError } from '../exception/BaseError.js' +import { clone } from './Utils.js' const moduleName = 'ConfigurationMigrations' @@ -193,7 +194,7 @@ const setAtPath = ( * @returns `{ config, fieldErrors, warnings }` */ export const remapDeprecatedKeys = (config: Record): RemapDeprecatedKeysResult => { - const out = structuredClone(config) + const out = clone(config) const fieldErrors: FieldError[] = [] const warnings: RemapWarning[] = [] @@ -224,7 +225,7 @@ export const remapDeprecatedKeys = (config: Record): RemapDepre * @param _filePath - configuration file path (unused) * @returns new configuration object */ -const migrateV0ToV1: MigrationFn = (config, _filePath) => structuredClone(config) +const migrateV0ToV1: MigrationFn = (config, _filePath) => clone(config) /** * Sequential migration chain. Index `i` migrates a v`i` configuration to v`i+1`. @@ -308,7 +309,7 @@ export const applyConfigurationMigration = ( `${moduleName}.applyConfigurationMigration: No migration defined for $schemaVersion ${sourceVersion.toString()} → ${CURRENT_CONFIGURATION_SCHEMA_VERSION.toString()}` ) } - let migrated = structuredClone(config) + let migrated = clone(config) for (let v = sourceVersion; v < CURRENT_CONFIGURATION_SCHEMA_VERSION; v++) { migrated = migrationChain[v](migrated, filePath) migrated.$schemaVersion = v + 1 diff --git a/src/utils/ConfigurationSchema.ts b/src/utils/ConfigurationSchema.ts index c24625e1..25d4b09f 100644 --- a/src/utils/ConfigurationSchema.ts +++ b/src/utils/ConfigurationSchema.ts @@ -14,6 +14,7 @@ import { } from '../types/index.js' import { WorkerProcessType } from '../worker/index.js' import { CURRENT_CONFIGURATION_SCHEMA_VERSION } from './ConfigurationMigrations.js' +import { has } from './Utils.js' // --------------------------------------------------------------- // Sub-schemas @@ -207,7 +208,7 @@ const UIServerListenOptionsSchema = z value => value != null && typeof value === 'object' && !Array.isArray(value), { message: 'must be a non-array object' } ) - .refine(value => !Object.hasOwn(value as object, 'accessPolicy'), { + .refine(value => !has('accessPolicy', value), { message: "'accessPolicy' must be configured under 'uiServer', not 'uiServer.options'", }) .pipe(UIServerListenOptionsObjectSchema) diff --git a/src/utils/ConfigurationValidation.ts b/src/utils/ConfigurationValidation.ts index 72496bbf..a58ab40c 100644 --- a/src/utils/ConfigurationValidation.ts +++ b/src/utils/ConfigurationValidation.ts @@ -14,7 +14,7 @@ import { } from './ConfigurationMigrations.js' import { ConfigurationSchema } from './ConfigurationSchema.js' import { configurationLogPrefix } from './ConfigurationUtils.js' -import { isEmpty } from './Utils.js' +import { assertIsJsonObject, clone, isEmpty } from './Utils.js' const moduleName = 'ConfigurationValidation' @@ -37,6 +37,7 @@ export class ConfigurationValidationError extends BaseError { public readonly fieldErrors: FieldError[] public readonly filePath: string public readonly migratedFrom?: number + public override readonly name = 'ConfigurationValidationError' as const public readonly phase: ValidationPhase public constructor ( @@ -89,18 +90,19 @@ export class ConfigurationValidationError extends BaseError { * @returns Validated and transformed `ConfigurationData` */ export const validateConfiguration = (parsed: unknown, filePath: string): ConfigurationData => { - if (parsed == null || typeof parsed !== 'object' || Array.isArray(parsed)) { - throw new BaseError( + assertIsJsonObject( + parsed, + new BaseError( `${moduleName}.validateConfiguration: Invalid simulator configuration payload (not a JSON object) ${filePath}` ) - } + ) if (isEmpty(parsed)) { throw new BaseError( `${moduleName}.validateConfiguration: Empty simulator configuration from file ${filePath}` ) } // Defensive clone: $schemaVersion is rewritten below. - const parsedRecord = structuredClone(parsed) as Record + const parsedRecord = clone(parsed) as Record const version = coerceConfigurationVersion(parsedRecord.$schemaVersion) parsedRecord.$schemaVersion = version @@ -147,4 +149,4 @@ export const validateConfiguration = (parsed: unknown, filePath: string): Config const transformConfiguration = ( validated: ConfigurationData, _filePath: string -): ConfigurationData => structuredClone(validated) +): ConfigurationData => clone(validated) diff --git a/src/utils/Utils.ts b/src/utils/Utils.ts index 0cc36082..b8212e7a 100644 --- a/src/utils/Utils.ts +++ b/src/utils/Utils.ts @@ -16,7 +16,6 @@ import { env } from 'node:process' import { type JsonObject, - type JsonType, MapStringifyFormat, MessageType, type TimestampedData, @@ -465,18 +464,11 @@ export const secureRandom = (): number => { return getRandomValues(new Uint32Array(1))[0] / 0x100000000 } -export const JSONStringify = < - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters - T extends - | JsonType - | Map> - | Record[] - | Set> ->( - object: T, - space?: number | string, - mapFormat?: MapStringifyFormat - ): string => { +export const JSONStringify = ( + object: unknown, + space?: number | string, + mapFormat?: MapStringifyFormat +): string => { return JSON.stringify( object, (_, value: Record) => { diff --git a/tests/charging-station/TemplateSchema.test.ts b/tests/charging-station/TemplateSchema.test.ts index 0210b9e9..ccd59d3e 100644 --- a/tests/charging-station/TemplateSchema.test.ts +++ b/tests/charging-station/TemplateSchema.test.ts @@ -232,7 +232,7 @@ await describe('TemplateSchema', async () => { }) }) - await describe('Evses validation (OCPP 2.0.1 §7.2)', async () => { + await describe('Evses topology validation (OCPP 2.0.1 part 1 §7.1 EVSE numbering, §7.2 connector numbering)', async () => { await it('should reject EVSE 0 with non-zero connector id', () => { const result = TemplateSchema.safeParse( buildMinimalTemplate({ Evses: { 0: { Connectors: { 1: {} } } } }) diff --git a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-MasterPass.test.ts b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-MasterPass.test.ts index 710ac064..d6ddeb9c 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-MasterPass.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-MasterPass.test.ts @@ -20,6 +20,7 @@ import { OCPP20IdTokenEnumType, OCPP20OptionalVariableName, OCPPVersion, + ReasonCodeEnumType, RequestStartStopStatusEnumType, } from '../../../../src/types/index.js' import { Constants } from '../../../../src/utils/index.js' @@ -110,6 +111,8 @@ await describe('C12.FR.09 - MasterPassGroupId Check', async () => { assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.transactionId, undefined) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.InvalidIdToken) }) await it('C12.FR.09 - should be no-op when MasterPassGroupId not configured', async () => { diff --git a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStartTransaction.test.ts b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStartTransaction.test.ts index d82bd18c..22c3c0c9 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStartTransaction.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStartTransaction.test.ts @@ -38,6 +38,7 @@ import { OCPP20TransactionEventEnumType, OCPP20TriggerReasonEnumType, OCPPVersion, + ReasonCodeEnumType, RequestStartStopStatusEnumType, } from '../../../../src/types/index.js' import { Constants } from '../../../../src/utils/index.js' @@ -137,9 +138,10 @@ await describe('F01 & F02 - Remote Start Transaction', async () => { // Assert assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.transactionId, undefined) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.InvalidIdToken) }) - // G03.FR.03 — Reject when auth returns BLOCKED for groupIdToken await it('should reject RequestStartTransaction when groupIdToken auth returns BLOCKED', async () => { // Arrange: Primary token accepted, group token blocked let callCount = 0 @@ -186,9 +188,10 @@ await describe('F01 & F02 - Remote Start Transaction', async () => { // Assert assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.transactionId, undefined) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.InvalidIdToken) }) - // FR: F01.FR.17, F02.FR.05 - Verify remoteStartId and idToken are stored for later TransactionEvent await it('should store remoteStartId and idToken in connector status for TransactionEvent', async () => { const { station: spyChargingStation } = createMockChargingStation({ baseName: TEST_CHARGING_STATION_BASE_NAME, @@ -342,6 +345,8 @@ await describe('F01 & F02 - Remote Start Transaction', async () => { assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.transactionId, undefined) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.InvalidProfile) }) // OCPP 2.0.1 §2.10: transactionId MUST NOT be present at RequestStartTransaction time @@ -383,6 +388,8 @@ await describe('F01 & F02 - Remote Start Transaction', async () => { assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.transactionId, undefined) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.InvalidProfile) }) // FR: F01.FR.07 @@ -416,6 +423,8 @@ await describe('F01 & F02 - Remote Start Transaction', async () => { } await testableService.handleRequestStartTransaction(mockStation, firstRequest) + const connectorStatus = mockStation.getConnectorStatus(1) + const pendingTransactionId = connectorStatus?.transactionId // Now try to start another transaction on the same EVSE const secondRequest: OCPP20RequestStartTransactionRequest = { @@ -431,7 +440,33 @@ await describe('F01 & F02 - Remote Start Transaction', async () => { assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) - assert.notStrictEqual(response.transactionId, undefined) + // §F01.FR.13: echo pending transactionId + assert.strictEqual(response.transactionId, pendingTransactionId) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.TxStarted) + }) + + await it('should reject RequestStartTransaction with TxInProgress (no transactionId echo) when connector is locked without pending transaction', async () => { + const connectorStatus = mockStation.getConnectorStatus(1) + if (connectorStatus == null) { + assert.fail('Connector 1 missing on mock station') + } + connectorStatus.locked = true + + const request: OCPP20RequestStartTransactionRequest = { + evseId: 1, + idToken: { + idToken: 'LOCKED_CONNECTOR_TOKEN', + type: OCPP20IdTokenEnumType.ISO14443, + }, + remoteStartId: 102, + } + + const response = await testableService.handleRequestStartTransaction(mockStation, request) + + assert.notStrictEqual(response, undefined) + assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.transactionId, undefined) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.TxInProgress) }) await it('should reject RequestStartTransaction when authorization throws an error', async () => { @@ -457,6 +492,8 @@ await describe('F01 & F02 - Remote Start Transaction', async () => { // Assert assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.transactionId, undefined) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.InternalError) }) // FR: F01.FR.02 — TC_F_03_CS @@ -564,6 +601,8 @@ await describe('F01 & F02 - Remote Start Transaction', async () => { // Assert assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.transactionId, undefined) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.NotFound) }) await describe('REQUEST_START_TRANSACTION event listener', async () => { diff --git a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStopTransaction.test.ts b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStopTransaction.test.ts index ab0e9086..61105e01 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStopTransaction.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStopTransaction.test.ts @@ -28,6 +28,7 @@ import { OCPP20TransactionEventEnumType, OCPP20TriggerReasonEnumType, OCPPVersion, + ReasonCodeEnumType, RequestStartStopStatusEnumType, } from '../../../../src/types/index.js' import { Constants } from '../../../../src/utils/index.js' @@ -129,13 +130,25 @@ await describe('F03 - Remote Stop Transaction', async () => { }) // FR: F03.FR.08 - await it('should reject stop transaction for non-existent transaction ID', () => { + await it('should reject stop transaction for invalid transaction ID format - non-UUID string', () => { const response = testableService.handleRequestStopTransaction(mockStation, { transactionId: 'non-existent-transaction-id' as UUIDv4, }) assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.InvalidValue) + }) + + // FR: F03.FR.08 + await it('should reject stop transaction for valid UUID with no matching transaction', () => { + const response = testableService.handleRequestStopTransaction(mockStation, { + transactionId: '00000000-0000-4000-8000-000000000000', + }) + + assert.notStrictEqual(response, undefined) + assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.TxNotFound) }) // FR: F03.FR.08 @@ -146,6 +159,7 @@ await describe('F03 - Remote Stop Transaction', async () => { assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.InvalidValue) }) // FR: F03.FR.08 @@ -156,6 +170,7 @@ await describe('F03 - Remote Stop Transaction', async () => { assert.notStrictEqual(response, undefined) assert.strictEqual(response.status, RequestStartStopStatusEnumType.Rejected) + assert.strictEqual(response.statusInfo?.reasonCode, ReasonCodeEnumType.InvalidValue) }) // FR: F03.FR.02 diff --git a/ui/common/src/errors.ts b/ui/common/src/errors.ts index 6aebfe90..9665cb9c 100644 --- a/ui/common/src/errors.ts +++ b/ui/common/src/errors.ts @@ -1,12 +1,12 @@ import type { ResponsePayload } from './types/UIProtocol.js' export class ConnectionError extends Error { + public override readonly name = 'ConnectionError' as const public readonly url: string public constructor (url: string, cause?: unknown) { const causeMsg = cause instanceof Error && cause.message.length > 0 ? `: ${cause.message}` : '' super(`Failed to connect to ${url}${causeMsg}`) - this.name = 'ConnectionError' this.url = url if (cause != null) { this.cause = cause @@ -15,6 +15,7 @@ export class ConnectionError extends Error { } export class ServerFailureError extends Error { + public override readonly name = 'ServerFailureError' as const public readonly payload: ResponsePayload public constructor (payload: ResponsePayload) { @@ -23,7 +24,6 @@ export class ServerFailureError extends Error { ? `: ${payload.hashIdsFailed.length.toString()} station(s) failed` : '' super(`Server returned failure status${details}`) - this.name = 'ServerFailureError' this.payload = payload } } -- 2.53.0