-
-
Notifications
You must be signed in to change notification settings - Fork 199
Fix: support configurable Electron ports and free port fallback #1460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
f280bd7
63fd30b
0947472
04bd581
73a4d71
b6c5292
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,23 +120,88 @@ function delay(ms) { | |
| return new Promise((resolve) => setTimeout(resolve, ms, true)); | ||
| } | ||
|
|
||
| function isPortAvailable(port) { | ||
| function isPortAvailable(port, host = "127.0.0.1") { | ||
| return new Promise((resolve) => { | ||
| const server = createServer(); | ||
| let settled = false; | ||
| const timeout = setTimeout(() => { | ||
| if (settled) { | ||
| return; | ||
| } | ||
| settled = true; | ||
| cleanup(); | ||
| server.close(); | ||
| resolve(false); | ||
| }, 1000); | ||
|
|
||
| const cleanup = () => { | ||
| clearTimeout(timeout); | ||
| server.removeListener("error", onError); | ||
| server.removeListener("listening", onListening); | ||
| }; | ||
|
|
||
| server.listen(port, "127.0.0.1", () => { | ||
| const onListening = () => { | ||
| if (settled) { | ||
| return; | ||
| } | ||
| settled = true; | ||
| cleanup(); | ||
| server.close(() => { | ||
| resolve(true); | ||
| }); | ||
| }); | ||
| }; | ||
|
|
||
| server.on("error", () => { | ||
| // Port is not available | ||
| const onError = () => { | ||
| if (settled) { | ||
| return; | ||
| } | ||
| settled = true; | ||
| cleanup(); | ||
| resolve(false); | ||
| }); | ||
| }; | ||
|
|
||
| server.once("error", onError); | ||
| server.once("listening", onListening); | ||
| server.listen(port, host); | ||
| }); | ||
| } | ||
|
|
||
| function parsePort(value) { | ||
| const parsed = Number.parseInt(value, 10); | ||
| if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed <= 0 || parsed > 65535) { | ||
| throw new Error(`Invalid port value: ${value}`); | ||
| } | ||
| return parsed; | ||
| } | ||
|
|
||
| async function findAvailablePort( | ||
| startPort, | ||
| host, | ||
| maxPortOffset = 100, | ||
| verbose = false, | ||
| ) { | ||
| const maxPort = Math.min(startPort + maxPortOffset, 65535); | ||
|
|
||
| if (maxPort <= startPort) { | ||
| throw new Error( | ||
| `No available port found in range ${startPort}-${maxPort}`, | ||
| ); | ||
| } | ||
|
|
||
| for (let port = startPort; port <= maxPort; port++) { | ||
| const isAvailable = await isPortAvailable(port, host); | ||
| if (isAvailable) { | ||
| return port; | ||
| } | ||
|
|
||
| if (verbose) { | ||
| logWarning(`Port ${port} unavailable; checking next port`); | ||
| } | ||
| } | ||
|
|
||
| throw new Error(`No available port found in range ${startPort}-${maxPort}`); | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| function spawnPromise(command, args, options) { | ||
| return new Promise((resolve, reject) => { | ||
| const child = spawn(command, args, { | ||
|
|
@@ -357,11 +422,6 @@ async function main() { | |
| if (parsingFlags && arg === "--port" && i + 1 < args.length) { | ||
| const port = args[++i]; | ||
| envVars.PORT = port; | ||
| // Default: localhost in development, 127.0.0.1 in production | ||
| const defaultHost = | ||
| process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1"; | ||
| const baseHost = process.env.HOST || defaultHost; | ||
| envVars.BASE_URL = `http://${baseHost}:${port}`; | ||
| continue; | ||
| } | ||
|
|
||
|
|
@@ -623,17 +683,26 @@ async function main() { | |
| // Apply parsed environment variables to process.env first | ||
| Object.assign(process.env, envVars); | ||
|
|
||
| // Port configuration (fixed default to 6274) | ||
| const requestedPort = 6274; | ||
| const requestedPortCandidate = | ||
| process.env.SERVER_PORT !== undefined | ||
| ? process.env.SERVER_PORT | ||
| : envVars.PORT; | ||
| const requestedPort = requestedPortCandidate | ||
| ? parsePort(requestedPortCandidate) | ||
| : 6274; | ||
| let PORT; | ||
| const defaultHost = | ||
| process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1"; | ||
| const baseHost = process.env.HOST || defaultHost; | ||
| const host = baseHost; | ||
|
|
||
| try { | ||
| // Check if user explicitly set a port via --port flag | ||
| const hasExplicitPort = envVars.PORT !== undefined; | ||
| const hasExplicitPort = requestedPortCandidate !== undefined; | ||
|
Comment on lines
+695
to
+710
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Truthiness vs. identity: Line 699 uses a truthiness test — falsy strings like Align the two checks so Proposed fix const requestedPortCandidate =
process.env.SERVER_PORT !== undefined
? process.env.SERVER_PORT
: envVars.PORT;
- const requestedPort = requestedPortCandidate
- ? parsePort(requestedPortCandidate)
- : 6274;
+ const requestedPort =
+ requestedPortCandidate !== undefined && requestedPortCandidate !== ""
+ ? parsePort(requestedPortCandidate)
+ : 6274;
let PORT;
const defaultHost =
process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1";
const baseHost = process.env.HOST || defaultHost;
const host = baseHost;
try {
// Check if user explicitly set a port via --port flag
- const hasExplicitPort = requestedPortCandidate !== undefined;
+ const hasExplicitPort =
+ requestedPortCandidate !== undefined && requestedPortCandidate !== "";This way 🤖 Prompt for AI Agents |
||
|
|
||
| if (hasExplicitPort) { | ||
| if (await isPortAvailable(requestedPort)) { | ||
| PORT = requestedPort.toString(); | ||
| if (await isPortAvailable(requestedPort, host)) { | ||
| PORT = String(requestedPort); | ||
| } else { | ||
| logError(`Explicitly requested port ${requestedPort} is not available`); | ||
| logInfo( | ||
|
|
@@ -642,23 +711,17 @@ async function main() { | |
| throw new Error(`Port ${requestedPort} is already in use`); | ||
| } | ||
| } else { | ||
| // Fixed port policy: use default port 6274 and fail fast if unavailable | ||
| if (await isPortAvailable(requestedPort)) { | ||
| PORT = requestedPort.toString(); | ||
| } else { | ||
| logError( | ||
| `Default port ${requestedPort} is already in use. Please free the port`, | ||
| const resolvedPort = await findAvailablePort(requestedPort, host, 100, true); | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (resolvedPort !== requestedPort) { | ||
| logInfo( | ||
| `Default port ${requestedPort} is busy. Using next available port ${resolvedPort}.`, | ||
| ); | ||
| throw new Error(`Port ${requestedPort} is already in use`); | ||
| } | ||
| PORT = String(resolvedPort); | ||
| } | ||
|
|
||
| // Update environment variables with the final port | ||
| envVars.PORT = PORT; | ||
| // Default: localhost in development, 127.0.0.1 in production | ||
| const defaultHost = | ||
| process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1"; | ||
| const baseHost = process.env.HOST || defaultHost; | ||
| envVars.BASE_URL = `http://${baseHost}:${PORT}`; | ||
| Object.assign(process.env, envVars); | ||
| } catch (error) { | ||
|
|
@@ -744,9 +807,6 @@ async function main() { | |
| // Open the browser automatically | ||
| // Use BASE_URL if set, otherwise construct from HOST and PORT | ||
| // Default: localhost in development, 127.0.0.1 in production | ||
| const defaultHost = | ||
| process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1"; | ||
| const host = process.env.HOST || defaultHost; | ||
| let url = process.env.BASE_URL || `http://${host}:${PORT}`; | ||
|
|
||
| // Append initial tab hash if specified | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.