JS-2355 Expose Standalone Parser Port in public Configuration

[quentin-jaquier-sonarsource]

---

Summary by Gitar

• Configuration:
  • Added existingNodeProcessPort field to BridgeServerConfig and exposed it in StandaloneParser builder
• Bridge Server:
  • Updated BridgeServerImpl to support overriding the node process port via configuration rather than only environment variables
• Testing:
  • Added unit tests in StandaloneParserTest to validate out-of-range and invalid node port configurations

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

JS-2355

[github-actions]

Ruling Report

No changes to ruling expected issues in this PR

[gitar-bot]

That's fair — a full mock/spy setup to exercise startServerLazily end-to-end just to check a field assignment would be overkill given how heavy the existing scaffolding already is in that test class.

That said, a narrow unit test wouldn't need any of that scaffolding. getExistingNodeProcessPort() and the existingNodeProcessPortOverride field don't require spawning a server or spying on startServerLazily — you can call startServerLazily(serverConfig) on a plain (non-spied) createUnitBridgeServer() instance with a BridgeServerConfig that has a non-null existingNodeProcessPort, expect it to throw (since there's no real process listening), and then assert getExistingNodeProcessPort() returns the config value rather than the env var — no deploy/startServer/waitChannelReady stubbing needed since the override is assigned before any of that logic runs. That would cover precedence without pulling in the existing convoluted spy setup.

If you'd still rather skip it, that's your call — the validation-focused tests you added are the more valuable ones either way.

[gitar-bot]

Code Review ✅ Approved 4 resolved / 4 findings

Exposes the standalone parser port in public configuration via existingNodeProcessPort field on BridgeServerConfig, allowing callers to override the node process port through the API instead of only environment variables. Port validation errors still reference the env var rather than the new API, existingNodeProcessPort("0") is silently ignored, and invalid-but-syntactically-valid ports surface as a message-less ServerAlreadyFailedException.

✅ 4 resolved

✅ Quality: Port validation errors still blame the env var, not the new API

📄 sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java:767-781 📄 sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java:820-824 📄 sonar-plugin/standalone/src/main/java/org/sonar/plugins/javascript/standalone/StandaloneParser.java:140-143 📄 sonar-plugin/standalone/src/test/java/org/sonar/plugins/javascript/standalone/StandaloneParserTest.java:105-119
The port is now settable programmatically via StandaloneParser.builder().existingNodeProcessPort(...), but validation still happens deep inside nodeAlreadyRunningPort() and reports "Node.js process port set in $SONARJS_EXISTING_NODE_PROCESS_PORT ..." / "Error parsing number in environment variable SONARJS_EXISTING_NODE_PROCESS_PORT". An embedder passing "not-a-port" to the builder gets an error pointing at an environment variable it never set (the two new tests even assert that wording), and it only surfaces after the parser constructor has already created temp directories and the BridgeServerImpl. Validate/parse the port where it is supplied (builder setter or build()) and word the message according to the source of the value.

✅ Edge Case: existingNodeProcessPort("0") is silently ignored

📄 sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java:767-781 📄 sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java:412-425
nodeAlreadyRunningPort() uses 0 as the sentinel for "not set" and only rejects values < 0 or > 65535, so a caller that passes existingNodeProcessPort("0") through the new public builder gets no error: the bridge silently spawns its own Node process instead of attaching, contradicting the error message that claims the port must be "between 1 and 65535". Reject 0 (and blank strings) explicitly when the value comes from the builder rather than treating it as absent.

✅ Quality: Wrong-but-valid port surfaces as a message-less ServerAlreadyFailedException

📄 sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java:412-425 📄 sonar-plugin/standalone/src/main/java/org/sonar/plugins/javascript/standalone/StandaloneParser.java:90-101
With existingNodeProcessPort("54321") and nothing listening there, waitChannelReady fails and startServerLazily throws a bare ServerAlreadyFailedException (a RuntimeException with no message and no cause), which propagates out of the StandaloneParser constructor since only IOException is caught. An embedder using the newly exposed option therefore gets an exception that says nothing about the port it supplied. Throw an IllegalStateException naming the host/port when the channel to a user-supplied existing process cannot be established.

✅ Quality: No test covers the port actually being plumbed through

📄 sonar-plugin/standalone/src/test/java/org/sonar/plugins/javascript/standalone/StandaloneParserTest.java:105-119 📄 sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java:820-824
The two added tests only exercise the two validation failures, which would also pass if builder.existingNodeProcessPort were routed nowhere except the validation call. Nothing asserts the happy path (the builder value is used as the connection port and takes precedence over SONARJS_EXISTING_NODE_PROCESS_PORT) nor the fallback (null in BridgeServerConfig keeps reading the env var, which is what the sensor path relies on via fromSensorContext). Add a BridgeServerImplTest case asserting getExistingNodeProcessPort() returns the config value after startServerLazily and falls back to the env var when it is null.

Review coverage

Functional validation No results

Rules No rules evaluated

Auto-approval Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
78.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[quentin-jaquier-sonarsource]

We changed the strategy to reach the objective, this is not required anymore.