]> Piment Noir Git Repositories - e-mobility-charging-stations-simulator.git/commit
fix(utils): make file persistence atomic across writers (#1871)
authorJérôme Benoit <jerome.benoit@piment-noir.org>
Tue, 26 May 2026 00:19:25 +0000 (02:19 +0200)
committerGitHub <noreply@github.com>
Tue, 26 May 2026 00:19:25 +0000 (02:19 +0200)
commit550824fa7da7dd829fb3155321aa924bf9681116
tree1aecff9f4916ecc88a0e87a7591045aaa3d02582
parent2de4de3b6ba9ac729aaec7fe22514d175ea8be92
fix(utils): make file persistence atomic across writers (#1871)

* feat(utils): add atomic file write primitive and migrate call sites

Add `atomicWriteFile` and `atomicWriteFileSync` to `src/utils/FileUtils.ts`,
implementing the canonical write-then-rename pattern with optional fsync
durability and best-effort temp file cleanup on failure. Errors are funnelled
through the existing `handleFileException` helper so callers stay aligned
with the project-wide file error reporting contract.

Migrate the five non-atomic disk writes uncovered by the audit:

- `BootstrapStateUtils.writeStateFile` replaces its inline tmp+rename with
  the new primitive (single source of truth, gains fsync durability).
- `ChargingStation.saveConfiguration` replaces `writeFileSync` so charging
  station OCPP configuration JSON cannot be torn by a crash mid-write.
- `JsonFileStorage.storePerformanceStatistics` drops the persistent
  `openSync('w')` file descriptor design (which truncated the file at byte
  zero on every sample) and uses the atomic primitive instead. Also fixes
  the previous fire-and-forget `runExclusive(...).catch()` pattern by
  awaiting the lock.
- `OCPP20CertificateManager` writes installed PEM certificates atomically.

Add `FileType.Certificate` so PEM writes can flow through
`handleFileException` with an accurate file type label.

Concurrent writers to the same path must still be serialized externally
(typically via `AsyncLock`); the primitive does not implement an internal
queue, matching how every existing call site already locks. The
`createWriteStream` diagnostics archive in OCPP 1.6 `GetDiagnostics` is
intentionally left as-is since the file is ephemeral (FTP-uploaded then
discarded).

Targets Node >= 22 so `writeFile`/`writeFileSync` natively expose the
`flush: true` option used for the fsync step.

* fix(utils): align atomic write call sites on a single error path

Address review feedback on PR #1871. The atomic write primitive logs and
re-throws by default via `handleFileException`. Three call sites kept
their pre-migration outer error wrappers, producing double handling:

- `ChargingStation.saveConfiguration`: the `.catch` handler attached to
  the fire-and-forget `AsyncLock.runExclusive(...).catch(...)` chain
  re-threw inside the catch callback, leaking an unhandled promise
  rejection on every config write failure (disk full, EACCES, EROFS,
  ...). Pass `{ throwError: false }` to that `handleFileException`
  call so the rejection is fully absorbed. The retry semantics are
  preserved: when `atomicWriteFileSync` throws, the `endMeasure` and
  `configurationFileHash` updates inside the lock callback are skipped,
  so the next `saveConfiguration` invocation will retry the write.

- `JsonFileStorage.storePerformanceStatistics`: drop the redundant
  outer `try/catch` and pass `{ throwError: false }` to the primitive,
  matching the `BootstrapStateUtils.writeStateFile` template. Failures
  now produce a single error log instead of one error log followed by
  a second warn-level log.

- `OCPP20CertificateManager.storeCertificate`: replace the empty
  `logPrefix` with a string carrying `stationHashId` and the module
  origin so the new error log carries actionable context. The outer
  `try/catch` in `storeCertificate` only stringifies the error into
  the structured `{ success: false, error }` result and does not call
  `handleFileException`, so there is no double handling here — only
  the missing context to fix.

* refactor(utils): consolidate atomic write options and address audit findings

- FileUtils: include threadId in temp filename for cross-worker uniqueness;
  fold errorParams into AtomicWriteOptions (single trailing options bag);
  document SIGKILL leak, durability scope, and per-field defaults; drop
  redundant 'utf8' as BufferEncoding cast.
- BootstrapStateUtils.writeStateFile: drop the undefined placeholder.
- ChargingStation.saveConfiguration: route fs failures (already logged at
  error by handleFileException) to debug, and surface non-fs failures from
  the lock body at error level via a typeof-guarded ErrnoException check.
- OCPP20CertificateManager.storeCertificate: take a required logPrefix from
  the caller (replaces the synthetic prefix); drop the redundant manual
  mkdir+pathExists block (atomicWriteFile's default ensureDir handles it);
  write certificates with mode 0o600; document the per-path serialization
  rationale (paths keyed by certificate serial number). Both
  OCPP20IncomingRequestService callers updated to pass
  chargingStation.logPrefix(); the manager unit tests are migrated.
- JsonFileStorage.storePerformanceStatistics: migrate to AtomicWriteOptions
  with errorParams; set flush: false on this hot telemetry path (durability
  across crashes is not required for performance records); add 5 focused
  unit tests covering happy path, overwrite, Map serialization, runtime
  storage directory removal, and parallel writes.
- FileUtils tests: tighten assert.throws/rejects with { code: 'ENOENT' } and
  add a deterministic test that asserts the destination remains intact and
  the temp file is cleaned up when the rename step fails.

* fix(performance-storage): decode file URIs into native filesystem paths

JsonFileStorage parsed the storage URI via new URL(uri) and used
.pathname directly as a filesystem path. On Windows, file: URLs yield
WHATWG-formatted pathnames such as '/C:/Users/...' which mkdirSync
interprets relative to the current drive, producing corrupt paths like
'D:\\C:\\Users\\...' and breaking JsonFileStorage on windows-latest CI.

Use fileURLToPath() (the standard Node.js inverse of pathToFileURL) to
decode file: URIs into the native path on every platform. Non-file
schemes (typically jsonfile:./relative-path) keep .pathname semantics
for backward compatibility with existing user configurations.

The new tests in tests/performance/storage/JsonFileStorage.test.ts
exercise this code path with pathToFileURL(absolutePath), which now
round-trips correctly across POSIX and Windows.
src/charging-station/BootstrapStateUtils.ts
src/charging-station/ChargingStation.ts
src/charging-station/ocpp/2.0/OCPP20CertificateManager.ts
src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts
src/performance/storage/JsonFileStorage.ts
src/types/FileType.ts
src/utils/FileUtils.ts
src/utils/index.ts
tests/charging-station/ocpp/2.0/OCPP20CertificateManager.test.ts
tests/performance/storage/JsonFileStorage.test.ts [new file with mode: 0644]
tests/utils/FileUtils.test.ts