refactor: Replace portscanner in server with a native port scan

packages/server/lib/serve/httpListener.js

@@ -1,7 +1,7 @@
 import os from "node:os";
+import net from "node:net";
 import http from "node:http";
 import https from "node:https";
-import portscanner from "portscanner";
 
 /**
  * HTTP-listener helpers used by the {@link Supervisor}, which binds the port once and
@@ -40,45 +40,103 @@ export function createServer({https: useHttps, key, cert}, requestHandler) {
  * @returns {Promise<object>} Resolves with the bound <code>port</code> and the <code>server</code> instance
  * @private
  */
-export function listen(server, port, changePortIfInUse, acceptRemoteConnections) {
-	return new Promise(function(resolve, reject) {
-		const options = {};
+// Timeout (ms) for a single port probe. On localhost a port either accepts or refuses the
+// connection immediately, so this only guards against a probe that hangs indefinitely.
+const PORT_PROBE_TIMEOUT = 400;
 
-		if (!acceptRemoteConnections) {
-			// Unless remote connections are allowed, bind to the IPv4 loopback address
-			options.host = "127.0.0.1";
-		} // If remote connections are allowed, do not set host so the server listens on all supported interfaces
+/**
+ * Probes whether something is accepting TCP connections on the given host/port.
+ *
+ * Mirrors the connect-probe semantics of the previously used <code>portscanner</code> dependency:
+ * a successful connection means the port is in use; a refused connection or a timeout means it is
+ * free. Any other socket error (e.g. an unreachable host) is treated as a scan failure and rejects,
+ * so unexpected problems surface to the caller instead of being silently reported as "free".
+ *
+ * @param {string} host Host to probe
+ * @param {number} port Port to probe
+ * @returns {Promise<boolean>} Resolves <code>true</code> if the port is in use, <code>false</code> if free
+ * @private
+ */
+function isPortInUse(host, port) {
+	return new Promise(function(resolve, reject) {
+		const socket = new net.Socket();
+		const finish = function(settle, value) {
+			socket.removeAllListeners();
+			socket.destroy();
+			settle(value);
+		};
+		socket.setTimeout(PORT_PROBE_TIMEOUT);
+		socket.once("connect", () => finish(resolve, true));
+		socket.once("timeout", () => finish(resolve, false));
+		socket.once("error", function(err) {
+			if (err.code === "ECONNREFUSED") {
+				finish(resolve, false);
+			} else {
+				finish(reject, err);
+			}
+		});
+		socket.connect(port, host);
+	});
+}
 
-		const portScanHost = options.host || "127.0.0.1";
-		const portMax = changePortIfInUse ? port + 30 : port;
+/**
+ * Scans the inclusive port range <code>[port, portMax]</code> on the given host and returns the
+ * first port not in use, or <code>null</code> if every port in the range is taken.
+ *
+ * @param {number} port First port of the range
+ * @param {number} portMax Last port of the range (inclusive)
+ * @param {string} host Host to scan
+ * @returns {Promise<number|null>} The first free port, or <code>null</code> if none is available
+ * @private
+ */
+async function findAPortNotInUse(port, portMax, host) {
+	for (let candidate = port; candidate <= portMax; candidate++) {
+		if (!await isPortInUse(host, candidate)) {
+			return candidate;
+		}
+	}
+	return null;
+}
 
-		portscanner.findAPortNotInUse(port, portMax, portScanHost, function(error, foundPort) {
-			if (error) {
-				reject(error);
-				return;
-			}
+export async function listen(server, port, changePortIfInUse, acceptRemoteConnections) {
+	// Unless remote connections are allowed, bind to the IPv4 loopback address. Otherwise leave
+	// host unset so the server listens on all supported interfaces.
+	const host = acceptRemoteConnections ? undefined : "127.0.0.1";
+	const portScanHost = host ?? "127.0.0.1";
+	const portMax = changePortIfInUse ? port + 30 : port;
 
-			if (!foundPort) {
-				const err = new Error(changePortIfInUse ?
-					`EADDRINUSE: Could not find available ports between ${port} and ${portMax}.` :
-					`EADDRINUSE: Port ${port} is already in use.`);
-				err.code = "EADDRINUSE";
-				err.errno = "EADDRINUSE";
-				err.address = portScanHost;
-				err.port = portMax;
-				reject(err);
-				return;
-			}
+	const foundPort = await findAPortNotInUse(port, portMax, portScanHost);
+	if (foundPort === null) {
+		const err = new Error(changePortIfInUse ?
+			`EADDRINUSE: Could not find available ports between ${port} and ${portMax}.` :
+			`EADDRINUSE: Port ${port} is already in use.`);
+		err.code = "EADDRINUSE";
+		err.errno = "EADDRINUSE";
+		err.address = portScanHost;
+		err.port = portMax;
+		throw err;
+	}
 
-			options.port = foundPort;
-			server.listen(options, function() {
-				resolve({port: options.port, server});
-			});
+	await listenOnce(server, {host, port: foundPort});
+	return {port: foundPort, server};
+}
 
-			server.on("error", function(err) {
-				reject(err);
-			});
-		});
+// server.listen signals success via a 'listening' event and failure via an 'error' event.
+// Bridge both into a single promise, detaching the losing listener once one fires (the old code
+// left the error listener attached on every successful bind).
+function listenOnce(server, options) {
+	return new Promise(function(resolve, reject) {
+		const onError = function(err) {
+			server.removeListener("listening", onListening);
+			reject(err);
+		};
+		const onListening = function() {
+			server.removeListener("error", onError);
+			resolve();
+		};
+		server.once("error", onError);
+		server.once("listening", onListening);
+		server.listen(options);
 	});
 }
 

packages/server/package.json

@@ -99,7 +99,6 @@
 		"graceful-fs": "^4.2.11",
 		"mime-types": "^3.0.2",
 		"parseurl": "^1.3.3",
-		"portscanner": "^2.2.0",
 		"router": "^2.2.0",
 		"ws": "^8.21.3"
 	},

packages/server/test/lib/server/ports.js

@@ -2,19 +2,10 @@ import test from "ava";
 import supertest from "supertest";
 import {serve} from "../../../lib/server.js";
 import http from "node:http";
-import portscanner from "portscanner";
-import sinonGlobal from "sinon";
+import esmock from "esmock";
 import {graphFromPackageDependencies} from "@ui5/project/graph";
 import {isolatedUi5DataDir} from "../../utils/buildCacheIsolation.js";
 
-test.beforeEach((t) => {
-	t.context.sinon = sinonGlobal.createSandbox();
-});
-
-test.afterEach.always((t) => {
-	t.context.sinon.restore();
-});
-
 test.serial("Start server - Port is already taken and an error occurs", async (t) => {
 	t.plan(6);
 	const port = 3360;
@@ -102,31 +93,32 @@ test.serial("Start server together with node server - Port is already taken and
 	server.close();
 });
 
-test.serial("Start server - Port can not be determined and an error occurs", async (t) => {
-	const {sinon} = t.context;
-
-	t.plan(2);
-	const portscannerFake = function(port, portMax, host, callback) {
-		return new Promise((resolve) => {
-			callback(new Error("testError"), false);
-			resolve();
-		});
-	};
-	const portScannerStub = sinon.stub(portscanner, "findAPortNotInUse").callsFake(portscannerFake);
-
-	const graph = await graphFromPackageDependencies({
-		cwd: "./test/fixtures/application.a"
-	});
-	const startServer = serve(graph, {
-		port: 3990,
-		changePortIfInUse: true,
-		ui5DataDir: isolatedUi5DataDir(t),
+test.serial("listen - Port scan fails with a generic error", async (t) => {
+	t.plan(3);
+	// Simulate a non-ECONNREFUSED socket error during the port probe (e.g. an unreachable host):
+	// it must be treated as a scan failure and propagate unchanged rather than be reported as "free".
+	class FakeSocket {
+		setTimeout() {}
+		once(event, cb) {
+			this._handlers = this._handlers || {};
+			this._handlers[event] = cb;
+		}
+		connect() {
+			const err = new Error("testError");
+			err.code = "EHOSTUNREACH";
+			queueMicrotask(() => this._handlers.error(err));
+		}
+		removeAllListeners() {}
+		destroy() {}
+	}
+	const httpListener = await esmock("../../../lib/serve/httpListener.js", {
+		"node:net": {default: {Socket: FakeSocket}, Socket: FakeSocket},
 	});
 
-	const error = await t.throwsAsync(startServer);
-	t.is(error.message, "testError",
-		"Server could not start, port is already taken and no other port is used.");
-	portScannerStub.restore();
+	const server = httpListener.createServer({https: false}, (req, res) => res.end());
+	const error = await t.throwsAsync(httpListener.listen(server, 3990, true, false));
+	t.is(error.message, "testError", "Generic scan error is propagated unchanged");
+	t.is(error.code, "EHOSTUNREACH", "Original error code is preserved");
 });