diff --git a/src/js/internal/html.ts b/src/js/internal/html.ts index 93e958fa3b5d..4f13b75bd676 100644 --- a/src/js/internal/html.ts +++ b/src/js/internal/html.ts @@ -219,69 +219,84 @@ yourself with Bun.serve(). }, {} as Record, ); - var server: Server; - getServer: { + const enableANSIColors = Bun.enableANSIColors; + + // A port somebody asked for stays fixed. Only a default (3000, bunfig `serve.port`) may move. + function portIsPinned(refusedPort: number) { + return ( + // --port, --host=: after the entry point + port !== undefined || + // bun --port= ./index.html + process.execArgv.some(arg => arg === "--port" || arg.startsWith("--port=")) || + // the variables Bun.serve() reads, when one of them is where the port came from + [env.BUN_PORT, env.PORT, env.NODE_PORT].some(value => value !== undefined && Number(value) === refusedPort) + ); + } + + function serve(port: number | undefined) { + return Bun.serve({ + static: staticRoutes, + development: + env.NODE_ENV !== "production" + ? { + console: enableConsoleLog, + hmr: undefined, + } + : false, + + hostname, + + // undefined: Bun.serve() resolves BUN_PORT/PORT/NODE_PORT, `bun --port`, bunfig `serve.port`, else 3000 + port, + + fetch(_req: Request) { + return new Response("Not found", { status: 404 }); + }, + }); + } + + let portInUse: number | undefined; + function listen(): Server | undefined { try { - server = Bun.serve({ - static: staticRoutes, - development: - env.NODE_ENV !== "production" - ? { - console: enableConsoleLog, - hmr: undefined, - } - : false, - - hostname, - port, - - // use the default port via existing port detection code. - // port: 3000, - - fetch(_req: Request) { - return new Response("Not found", { status: 404 }); - }, - }); - break getServer; + return serve(port); } catch (error: any) { - if (error?.code === "EADDRINUSE") { - let defaultPort = port || parseInt(env.PORT || env.BUN_PORT || env.NODE_PORT || "3000", 10); - for (let remainingTries = 5; remainingTries > 0; remainingTries--) { + // Anything but a refused bind is a bug worth a stack trace. + if (error?.syscall !== "listen") throw error; + + let message: string = error.message; + const { code, port: refusedPort } = error; + if (code === "EADDRINUSE" && typeof refusedPort === "number" && !portIsPinned(refusedPort)) { + const lastCandidate = Math.min(refusedPort + 10, 65535); + portInUse = refusedPort; + message = `Ports ${refusedPort} to ${lastCandidate} are all in use.`; + for (let candidate = refusedPort + 1; candidate <= lastCandidate; candidate++) { try { - server = Bun.serve({ - static: staticRoutes, - development: - env.NODE_ENV !== "production" - ? { - console: enableConsoleLog, - hmr: undefined, - } - : false, - - hostname, - - // Retry with a different port up to 4 times. - port: defaultPort++, - - fetch(_req: Request) { - return new Response("Not found", { status: 404 }); - }, - }); - break getServer; + return serve(candidate); } catch (error: any) { - if (error?.code === "EADDRINUSE") { - continue; + if (error?.syscall !== "listen") throw error; + if (error.code !== "EADDRINUSE") { + message = error.message; + break; } - throw error; } } } - throw error; + console.error(enableANSIColors ? `\x1b[31merror\x1b[0m\x1b[2m:\x1b[0m ${message}` : `error: ${message}`); + // No process.exit(): --watch/--hot keep the process up for the next reload. + process.exitCode = 1; + return undefined; } } + const server = listen(); + if (!server) return; + + if (portInUse !== undefined) { + const message = `Port ${portInUse} is in use, using port ${server.port} instead.`; + console.warn(enableANSIColors ? `\x1b[33mwarn\x1b[0m\x1b[2m:\x1b[0m ${message}` : `warn: ${message}`); + } + const elapsed = (performance.now() - initial).toFixed(2); - const enableANSIColors = Bun.enableANSIColors; function printInitialMessage(isFirst: boolean) { let pathnameToPrint; if (servePaths.length === 1) { diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index d29af0bf5146..14e727ed4a4e 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -1991,6 +1991,17 @@ impl NewServer { port, hostname: _hostname, } => { + // `err.port`, as on node's listen errors. + let with_port = |error_instance: JSValue| { + if *port != 0 { + error_instance.put( + global, + b"port", + JSValue::js_number_from_int32(i32::from(*port)), + ); + } + error_instance + }; // Rust's `target_os = "linux"` excludes // Android, so match both explicitly. #[cfg(any(target_os = "linux", target_os = "android"))] @@ -2011,7 +2022,7 @@ impl NewServer { syscall: bun_core::String::static_("listen"), ..Default::default() }; - let _ = global.throw_value(err.to_error_instance(global)); + let _ = global.throw_value(with_port(err.to_error_instance(global))); return; } // e.g. ENOSPC from epoll_ctl(EPOLL_CTL_ADD). Linux-only because @@ -2022,20 +2033,22 @@ impl NewServer { bun_sys::Error::from_code(errno, bun_sys::Tag::listen) .to_system_error(), ); - let _ = global.throw_value(err.to_error_instance(global)); + let _ = global.throw_value(with_port(err.to_error_instance(global))); return; } } - jsc::SystemError { - message: bun_core::String::create_format(format_args!( - "Failed to start server. Is port {} in use?", - port - )), - code: bun_core::String::static_("EADDRINUSE"), - syscall: bun_core::String::static_("listen"), - ..Default::default() - } - .to_error_instance(global) + with_port( + jsc::SystemError { + message: bun_core::String::create_format(format_args!( + "Failed to start server. Is port {} in use?", + port + )), + code: bun_core::String::static_("EADDRINUSE"), + syscall: bun_core::String::static_("listen"), + ..Default::default() + } + .to_error_instance(global), + ) } server_config::Address::Unix(unix) => { let unix = unix.as_bytes(); diff --git a/test/js/bun/http/bun-serve-html-entry.test.ts b/test/js/bun/http/bun-serve-html-entry.test.ts index 0b4b31ad4b0d..92dd2146caf5 100644 --- a/test/js/bun/http/bun-serve-html-entry.test.ts +++ b/test/js/bun/http/bun-serve-html-entry.test.ts @@ -1,6 +1,6 @@ import type { Subprocess } from "bun"; -import { expect, test } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; import { join } from "node:path"; async function getServerUrl(process: Subprocess) { @@ -715,3 +715,114 @@ test.concurrent("subdirectory routes use forward slashes on Windows", async () = expect(buttons.status).toBe(200); expect(await buttons.text()).toContain("Buttons"); }); + +describe("when the port is in use", () => { + // No PORT-like variable may leak in from the environment: each test picks + // exactly one way to name (or not name) the port. + const { PORT: _1, BUN_PORT: _2, NODE_PORT: _3, ...cleanEnv } = bunEnv; + const env = { ...cleanEnv, NODE_ENV: "production", NO_COLOR: "1" }; + const files = { "index.html": `

Hello

` }; + + // Holds a port that still has room above it: the fallback walks upward and + // stops at 65535. + function takePort() { + for (;;) { + const server = Bun.serve({ port: 0, hostname: "127.0.0.1", fetch: () => new Response("taken") }); + if (server.port <= 65500) return server; + server.stop(true); + } + } + + // Reads stdout until the dev server prints its URL or exits. The unfixed + // behaviour (serving on another port) shows up as a URL here instead of + // as a test timeout. + async function urlOrExit(proc: Subprocess<"ignore", "pipe", "pipe">) { + const decoder = new TextDecoder(); + let stdout = ""; + for await (const chunk of proc.stdout) { + stdout += decoder.decode(chunk, { stream: true }); + // only a whole line, so a chunk boundary cannot cut the port short + const match = stdout.match(/http:\/\/\S+(?=\n)/); + if (match && URL.canParse(match[0])) return { stdout, url: new URL(match[0]) }; + } + return { stdout, url: undefined }; + } + + test.concurrent("an explicit --port fails instead of moving to another port", async () => { + using taken = takePort(); + await using dir = tempDir("html-port-taken-flag", files); + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.html", `--port=${taken.port}`, "--hostname=127.0.0.1"], + env, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const { url } = await urlOrExit(proc); + if (url) proc.kill(); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(url?.href).toBeUndefined(); + expect(stderr.trim()).toBe(`error: Failed to start server. Is port ${taken.port} in use?`); + expect(exitCode).toBe(1); + }); + + test.concurrent("an explicit PORT fails instead of moving to another port", async () => { + using taken = takePort(); + await using dir = tempDir("html-port-taken-env", files); + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.html", "--hostname=127.0.0.1"], + env: { ...env, PORT: String(taken.port) }, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const { url } = await urlOrExit(proc); + if (url) proc.kill(); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(url?.href).toBeUndefined(); + expect(stderr.trim()).toBe(`error: Failed to start server. Is port ${taken.port} in use?`); + expect(exitCode).toBe(1); + }); + + test.concurrent("a default port moves to the next free one and says so", async () => { + using taken = takePort(); + // `serve.port` replaces the built-in default of 3000, so this exercises the + // same path as a taken port 3000 without depending on that port in CI. + await using dir = tempDir("html-port-taken-default", { + ...files, + "bunfig.toml": `[serve]\nport = ${taken.port}\n`, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.html", "--hostname=127.0.0.1"], + env, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const { url, stdout } = await urlOrExit(proc); + expect(url?.href, stdout).toBeDefined(); + try { + // Not on Windows: `taken` sits in the dynamic port range there, and Windows + // can hand the child's explicitly bound fallback port to a concurrent + // `port: 0` listener in another process as well (netstat then shows two + // LISTENING rows for 127.0.0.1:), so a fetch may reach the wrong + // server. Real default ports (3000) are outside that range. + if (!isWindows) { + const response = await fetch(url!); + expect(response.status).toBe(200); + expect(await response.text()).toContain("

Hello

"); + } + } finally { + proc.kill(); + } + const stderr = await proc.stderr.text(); + + expect(Number(url!.port)).not.toBe(taken.port); + expect(stderr.trim()).toBe(`warn: Port ${taken.port} is in use, using port ${url!.port} instead.`); + }); +}); diff --git a/test/js/bun/http/serve.test.ts b/test/js/bun/http/serve.test.ts index e4c5aef97553..9ed02c78bdb2 100644 --- a/test/js/bun/http/serve.test.ts +++ b/test/js/bun/http/serve.test.ts @@ -2673,6 +2673,29 @@ it("should support promise returned from error", async () => { subprocess.kill(); }); +it("EADDRINUSE error names the port that was refused", () => { + using first = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch: () => new Response("first"), + }); + let error: any; + try { + using second = Bun.serve({ + port: first.port, + hostname: "127.0.0.1", + fetch: () => new Response("second"), + }); + } catch (e) { + error = e; + } + expect({ code: error?.code, syscall: error?.syscall, port: error?.port }).toEqual({ + code: "EADDRINUSE", + syscall: "listen", + port: first.port, + }); +}); + if (process.platform === "linux") it("should use correct error when using a root range port(#7187)", () => { expect(() => {