fix(charging-station): throw BaseError on invalid supervision URL and revert state on set failure by reginaldalfret · Pull Request #2136 · SAP/e-mobility-charging-stations-simulator

reginaldalfret · 2026-10-04T03:59:34Z

Summary

Report supervision URL parsing failures as a BaseError naming the configured field and charging station, rather than a bare TypeError.
Restore stationInfo.supervisionUrls on validation failure. In the OCPP-key setter branch, store the parsed URL's normalized href before applying credentials and retained options, so an accepted whitespace-bearing base does not fail later while building the worker update.
Preserve existing reset/reload behavior and independent optional credentials.
Reuse existing test factory defaults, shared identity/network constants, the worker event constant/envelope, and payload/configuration types.

Testing

Regression coverage is in tests/charging-station/ChargingStation-SupervisionUrl.test.ts, including a real worker with persistent configuration. It verifies the consumer update URL, normalization, credentials (including an empty password), retained options, and unchanged memory and persisted bytes after a malformed update. ChargingStation-ResetIdentity.test.ts retains reset/reload coverage.

Local verification: root mise exec -- pnpm format, typecheck, lint, build, and test pass (4,003 tests passed, 6 skipped). The two affected suites pass 13/13 on Node 24; URL validation also passes 6/6 on Node 22.14.0. A standalone real-worker smoke confirmed the normalized update URL and persisted key, and unchanged memory and file contents after rejection.

The unrelated ChangeConfiguration and performanceStorage.uri defects remain tracked in #2141 and #2142.

PR Checklist

Changes described; title follows Conventional Commits.
Regression tests updated; no new user-facing configuration or API.
Related issue linked above.

Does this PR introduce a breaking change?

Yes
No

… revert state on set failure Signed-off-by: reginaldalfret <[email protected]>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

cla-assistant · 2026-10-04T03:59:48Z

All committers have signed the CLA.

hyperspace-pr-bot

The change is focused and mostly consistent with the existing error-handling pattern, but there is a type-level issue that should be addressed before merge. After fixing that, the state rollback behavior looks reasonable for the non-OCPP-configuration path.

PR Bot Information

Version: 1.31.69

Agent Instructions:
File Content Strategy: Full file content
Correlation ID: ada6c4c0-bff0-11f1-8fb9-5d16ced29fa3
Event Trigger: issue_comment.created
LLM: gpt-5.5

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The invalid-supervision-URL fix only covered the stationInfo branch of setSupervisionUrl and passed a cause option BaseError does not accept, so the file did not typecheck. Route every URL construction through a single parseSupervisionUrl helper that raises a BaseError naming the offending field, covering the template/config path, the OCPP-configuration key branch and the wsConnectionUrl getter, and add the regression tests the issue requires. Signed-off-by: Jérôme Benoit <[email protected]>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…error

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

… the suite The getter guard added by the previous commit had no test: reverting it left the suite green. The three supervision URL tests also leaked a temp template directory each, because a station built with the default persistence writes its configuration after the synchronous afterEach ran cleanup. Move them into a dedicated ChargingStation-SupervisionUrl suite with an async teardown that drains the pending write, disable persistence on every station they build, add the missing getter regression, drop two inert creation options and pin the exact BaseError message and name. Name the charging station in the diagnostic and report the configured base URL instead of the derived one, so the error matches the issue acceptance criteria and no longer quotes a value the operator never configured. Signed-off-by: Jérôme Benoit <[email protected]>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

jerome-benoit · 2026-10-07T02:44:42Z

Review follow-up in fbad081ab. Each finding from the initial review, with its verdict:

Fixed in this PR

Finding	Fix	Proof
Getter guard `wsConnectionUrl` had no test	Added the missing regression (template-seeded OCPP key)	Reverting the guard makes exactly that test fail; suite otherwise green
The 3 supervision URL tests leaked a `/tmp/cs-test-*` dir each	Moved into a dedicated `ChargingStation-SupervisionUrl` suite with an async `afterEach` draining the pending configuration write, and `persistentConfiguration: false` on every station	`/tmp/cs-test-*` count 149→152 before, 0 after
The propagated error did not name the charging station	Diagnostic prefixed with the station identity	Message: `CS-URL: Invalid supervision url 'not a url' configured in 'supervisionUrls'`
The getter quoted a derived URL the operator never configured, and could name the OCPP key while the value came from `supervisionUrls`	Reports the configured base URL; the OCPP key is named only when it actually supplied the value	Pinned by the new getter test
Two inert creation options in the OCPP branch test	Removed (only the template override drives that branch)	Test still passes
Weak assertions (`includes` on a hardcoded literal)	Pinned `{ name: 'BaseError', message: <exact> }`	`assert.throws` object form compares the message by exact equality
File header/`describe` did not describe the moved tests	Dedicated file with its own `@file`/`describe`; the identity suite is back to its 7 original tests	—

Confirmed but out of scope, tracked separately

The OCPP 1.6 ChangeConfiguration path can still write a non-URL into the supervision key (the guard turns it into a BaseError, but the value is persisted): [BUG] A non-URL value can be written into the supervision OCPP configuration key via OCPP 1.6 ChangeConfiguration #2141.
performanceStorage.uri still fails with a bare TypeError at module load: [BUG] A non-URL performanceStorage.uri crashes the simulator at module load with a bare TypeError #2142.

Verification: root typecheck and eslint pass; the affected suites pass 174/174 with no skipped tests and no temp-dir leak; three mutations (getter guard, OCPP-branch guard, state revert) each fail exactly one test.

…error

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Signed-off-by: Jérôme Benoit <[email protected]>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Signed-off-by: Jérôme Benoit <[email protected]>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

hyperspace-pr-bot

No new substantive issues found. The previous BaseError constructor issue has been resolved, and the added regression coverage addresses the relevant validation and rollback paths.

PR Bot Information

Version: 1.32.6

LLM: gpt-5.6-sol
Correlation ID: 68bce330-c28e-11f1-80c9-9a7cca6687a4
Agent Instructions:
Event Trigger: issue_comment.created
File Content Strategy: Full file content

fix(charging-station): throw BaseError on invalid supervision URL and…

2fae5da

… revert state on set failure Signed-off-by: reginaldalfret <[email protected]>

Copilot AI balanced review requested due to automatic review settings October 4, 2026 03:59

reginaldalfret requested review from jerome-benoit and olivierbagot as code owners October 4, 2026 03:59

Copilot AI reviewed Oct 4, 2026

hyperspace-pr-bot Bot reviewed Oct 4, 2026

View reviewed changes

Comment thread src/charging-station/ChargingStation.ts Resolved

hyperspace-pr-bot Bot deleted a comment from jerome-benoit Oct 4, 2026

[autofix.ci] apply automated fixes

1fbb41c

Copilot AI balanced review requested due to automatic review settings October 4, 2026 12:45

Copilot AI reviewed Oct 4, 2026

Copilot AI balanced review requested due to automatic review settings October 6, 2026 23:21

Copilot AI reviewed Oct 6, 2026

Merge branch 'main' into sap/issue-2130-invalid-supervision-url-base-…

a316ecb

…error

Copilot AI balanced review requested due to automatic review settings October 7, 2026 00:07

Copilot AI reviewed Oct 7, 2026

Copilot AI balanced review requested due to automatic review settings October 7, 2026 02:32

Copilot AI reviewed Oct 7, 2026

This was referenced Oct 7, 2026

[BUG] A non-URL value can be written into the supervision OCPP configuration key via OCPP 1.6 ChangeConfiguration #2141

Closed

[BUG] A non-URL performanceStorage.uri crashes the simulator at module load with a bare TypeError #2142

Closed

Merge branch 'main' into sap/issue-2130-invalid-supervision-url-base-…

52a3bfe

…error

Copilot AI balanced review requested due to automatic review settings October 7, 2026 15:50

Copilot AI reviewed Oct 7, 2026

fix(charging-station): normalize supervision URL before persisting

b3e4393

Signed-off-by: Jérôme Benoit <[email protected]>

Copilot AI balanced review requested due to automatic review settings October 7, 2026 17:46

Copilot AI reviewed Oct 7, 2026

refactor: reuse test helpers and keep README user-focused

22e505f

Signed-off-by: Jérôme Benoit <[email protected]>

Copilot AI balanced review requested due to automatic review settings October 7, 2026 18:36

Copilot AI reviewed Oct 7, 2026

hyperspace-pr-bot Bot reviewed Oct 7, 2026

View reviewed changes

hyperspace-pr-bot Bot deleted a comment from jerome-benoit Oct 7, 2026

jerome-benoit merged commit cb8a959 into SAP:main Oct 7, 2026
41 checks passed

jerome-benoit mentioned this pull request Oct 7, 2026

chore: release main #2098

Merged

Conversation

reginaldalfret commented Oct 4, 2026 • edited by jerome-benoit Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

PR Checklist

Does this PR introduce a breaking change?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

cla-assistant Bot commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hyperspace-pr-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

jerome-benoit commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

hyperspace-pr-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reginaldalfret commented Oct 4, 2026 •

edited by jerome-benoit

Loading

cla-assistant Bot commented Oct 4, 2026 •

edited

Loading