diff --git a/packages/opencode/src/cli/cmd/serve.ts b/packages/opencode/src/cli/cmd/serve.ts index c0f62b3ca071..9ca8900a677a 100644 --- a/packages/opencode/src/cli/cmd/serve.ts +++ b/packages/opencode/src/cli/cmd/serve.ts @@ -1,7 +1,8 @@ import { Effect } from "effect" -import { effectCmd } from "../effect-cmd" +import { CliError, effectCmd } from "../effect-cmd" import { withNetworkOptions, resolveNetworkOptions } from "../network" import { Flag } from "@opencode-ai/core/flag/flag" +import { errorMessage } from "@/util/error" export const ServeCommand = effectCmd({ command: "serve", @@ -16,7 +17,13 @@ export const ServeCommand = effectCmd({ console.log("Warning: OPENCODE_SERVER_PASSWORD is not set; server is unsecured.") } const opts = yield* resolveNetworkOptions(args) - const server = yield* Effect.promise(() => Server.listen(opts)) + // A failed bind is a user-fixable mistake (busy port, wrong --hostname), not + // a crash — surface it as a CliError so the message prints on its own instead + // of behind "Unexpected error". + const server = yield* Effect.tryPromise({ + try: () => Server.listen(opts), + catch: (error) => new CliError({ message: errorMessage(error) }), + }) console.log(`opencode server listening on http://${server.hostname}:${server.port}`) yield* Effect.never diff --git a/packages/opencode/src/server/bind.ts b/packages/opencode/src/server/bind.ts new file mode 100644 index 000000000000..3ccea3e6cfda --- /dev/null +++ b/packages/opencode/src/server/bind.ts @@ -0,0 +1,96 @@ +// Bind-failure diagnostics for `Server.listen`. +// +// Two problems make a failed bind undiagnosable without help: +// +// 1. Effect's `HttpServerError.ServeError` is a `Data.TaggedError` whose +// `message` is empty — the real `listen` error is parked on `.cause`, so a +// naive rethrow prints the bare string "ServeError". +// 2. Bun's `node:http` shim reports *every* `listen()` failure as `EADDRINUSE` +// ("Failed to start server. Is port N in use?"), discarding the real errno. +// Node returns EADDRNOTAVAIL / EACCES for the same binds. So errno dispatch +// alone cannot tell a busy port from a hostname that isn't on this machine. +// +// `assertBindable` handles (2) by checking the address before we ever call +// `listen`, where an exact answer is available. `bindError` handles (1) by +// walking the cause chain and translating whatever errno survives. +import { isIP } from "node:net" +import { networkInterfaces } from "node:os" + +const WILDCARD_HOSTS = new Set(["0.0.0.0", "::", "localhost", "127.0.0.1", "::1"]) + +export type BindTarget = { + hostname: string + port: number +} + +export function localAddresses(interfaces = networkInterfaces()) { + return [ + ...new Set( + Object.values(interfaces) + .flatMap((entries) => entries ?? []) + .map((entry) => entry.address), + ), + ] +} + +/** + * Throw before binding when `hostname` is a literal IP that no local interface + * owns — the common case being a VPN/Tailscale address whose interface is down. + * Hostnames are left alone; `listen` resolves those. + */ +export function assertBindable(target: BindTarget, interfaces = networkInterfaces()) { + const { hostname } = target + if (WILDCARD_HOSTS.has(hostname)) return + if (!isIP(hostname)) return + const local = localAddresses(interfaces) + // Link-local IPv6 may be written with a zone id (fe80::1%en0); compare bare. + const bare = hostname.split("%")[0] + if (local.some((address) => address === hostname || address.split("%")[0] === bare)) return + throw new Error( + `Cannot bind to ${hostname}: it is not an address on any local network interface.` + + ` Available: ${local.join(", ")}.` + + ` Use 0.0.0.0 to listen on every interface, or bring the interface up first (e.g. start Tailscale/VPN).`, + ) +} + +/** + * Translate a failed bind into a message that names the address and the fix. + * Falls back to the deepest cause message so we never print nothing. + */ +export function bindError(error: unknown, target: BindTarget) { + const { hostname, port } = target + const label = `${hostname}:${port}` + const wrap = (message: string) => new Error(message, { cause: error }) + + let cause: unknown = error + const seen = new Set() + let deepest: unknown = error + while (cause && typeof cause === "object" && !seen.has(cause)) { + seen.add(cause) + deepest = cause + const code = (cause as { code?: unknown }).code + if (code === "EADDRINUSE") { + // Bun folds EACCES into EADDRINUSE, and a privileged port is the far more + // likely explanation below 1024 — say so rather than blaming a conflict. + if (port > 0 && port < 1024) + return wrap( + `Cannot bind ${label}: ports below 1024 are privileged. Use a port >= 1024, or run with elevated privileges.`, + ) + return wrap( + `Address ${label} is already in use. Choose a different --port, use --port 0 to pick a free one,` + + ` or stop the process holding it.`, + ) + } + if (code === "EADDRNOTAVAIL") + return wrap( + `Cannot bind to ${label}: "${hostname}" is not an address on this machine.` + + ` Use a local interface address, 127.0.0.1, or 0.0.0.0.`, + ) + if (code === "EACCES") + return wrap(`Permission denied binding ${label}. Ports below 1024 require elevated privileges.`) + cause = (cause as { cause?: unknown }).cause + } + + const detail = deepest instanceof Error && deepest.message ? `: ${deepest.message}` : "" + return wrap(`Failed to start server on ${label}${detail}`) +} diff --git a/packages/opencode/src/server/server.ts b/packages/opencode/src/server/server.ts index 440b992c1557..89c7ed66ca0c 100644 --- a/packages/opencode/src/server/server.ts +++ b/packages/opencode/src/server/server.ts @@ -6,6 +6,7 @@ import { ConfigProvider, Context, Effect, Exit, Layer, Scope } from "effect" import { HttpRouter, HttpServer } from "effect/unstable/http" import { OpenApi } from "effect/unstable/httpapi" import { createServer } from "node:http" +import { assertBindable, bindError } from "./bind" import { MDNS } from "./mdns" import { HttpApiApp } from "./routes/instance/httpapi/server" import { disposeMiddleware } from "./routes/instance/httpapi/lifecycle" @@ -71,7 +72,10 @@ export async function openapi() { export let url: URL | undefined export async function listen(opts: ListenOptions): Promise { - const listener = await Effect.runPromise(listenEffect(opts)) + assertBindable(opts) + const listener = await Effect.runPromise(listenEffect(opts)).catch((error) => { + throw bindError(error, opts) + }) return { hostname: listener.hostname, port: listener.port, diff --git a/packages/opencode/test/server/bind.test.ts b/packages/opencode/test/server/bind.test.ts new file mode 100644 index 000000000000..54ff7d3ef47a --- /dev/null +++ b/packages/opencode/test/server/bind.test.ts @@ -0,0 +1,118 @@ +// Unit tests for bind-failure diagnostics. These exist because the two failure +// modes they cover are invisible at runtime: Effect's `ServeError` has an empty +// `message`, and Bun's `node:http` reports every listen error as EADDRINUSE. A +// regression here degrades silently to "Unexpected error / ServeError". +import { describe, expect, test } from "bun:test" +import type { networkInterfaces } from "node:os" +import { assertBindable, bindError, localAddresses } from "../../src/server/bind" + +type Interfaces = ReturnType + +// Only `address` is read; the rest of NetworkInterfaceInfo is noise here. +// oxlint-disable-next-line typescript-eslint/no-unsafe-type-assertion -- partial fixture, `address` is the only field under test +const INTERFACES = { + lo0: [ + { address: "127.0.0.1", family: "IPv4", internal: true }, + { address: "::1", family: "IPv6", internal: true }, + ], + en0: [ + { address: "192.168.0.40", family: "IPv4", internal: false }, + { address: "fe80::1c52:8bff:fe2e:29fe", family: "IPv6", internal: false }, + ], +} as unknown as Interfaces + +// Mirrors Effect's `HttpServerError.ServeError`: a tagged error with an empty +// message that hides the real failure on `.cause`. +function serveError(cause: unknown) { + const error = new Error("") + error.name = "ServeError" + Object.assign(error, { _tag: "ServeError", cause }) + return error +} + +function errno(code: string, message: string) { + return Object.assign(new Error(message), { code }) +} + +describe("server.bind", () => { + describe("assertBindable", () => { + test("allows wildcard and loopback hosts", () => { + for (const hostname of ["0.0.0.0", "::", "localhost", "127.0.0.1", "::1"]) { + expect(() => assertBindable({ hostname, port: 4096 }, INTERFACES)).not.toThrow() + } + }) + + test("allows an address owned by a local interface", () => { + expect(() => assertBindable({ hostname: "192.168.0.40", port: 4096 }, INTERFACES)).not.toThrow() + }) + + test("rejects an IP no interface owns, and lists what is available", () => { + // A Tailscale CGNAT address with the interface down — the reported case. + const bind = () => assertBindable({ hostname: "100.68.120.26", port: 4096 }, INTERFACES) + expect(bind).toThrow(/not an address on any local network interface/) + // The message has to be actionable, not just accurate. + expect(bind).toThrow(/192\.168\.0\.40/) + expect(bind).toThrow(/0\.0\.0\.0/) + }) + + test("ignores DNS names — listen() resolves those", () => { + expect(() => assertBindable({ hostname: "example.internal", port: 4096 }, INTERFACES)).not.toThrow() + }) + + test("matches link-local IPv6 written with a zone id", () => { + expect(() => + assertBindable({ hostname: "fe80::1c52:8bff:fe2e:29fe%en0", port: 4096 }, INTERFACES), + ).not.toThrow() + }) + }) + + test("localAddresses dedupes and flattens", () => { + expect(localAddresses(INTERFACES)).toEqual(["127.0.0.1", "::1", "192.168.0.40", "fe80::1c52:8bff:fe2e:29fe"]) + }) + + describe("bindError", () => { + const target = { hostname: "127.0.0.1", port: 4096 } + + test("unwraps an errno buried under ServeError", () => { + const error = bindError(serveError(errno("EADDRINUSE", "address already in use")), target) + expect(error.message).toContain("127.0.0.1:4096 is already in use") + expect(error.message).not.toContain("ServeError") + }) + + test("blames privileges, not a conflict, for ports below 1024", () => { + // Bun folds EACCES into EADDRINUSE, so the errno alone would mislead here. + const error = bindError(serveError(errno("EADDRINUSE", "in use?")), { hostname: "127.0.0.1", port: 80 }) + expect(error.message).toContain("privileged") + }) + + test("maps EADDRNOTAVAIL to the hostname, not the port", () => { + const error = bindError(serveError(errno("EADDRNOTAVAIL", "address not available")), { + hostname: "100.68.120.26", + port: 4096, + }) + expect(error.message).toContain("not an address on this machine") + }) + + test("maps EACCES", () => { + const error = bindError(serveError(errno("EACCES", "permission denied")), { hostname: "127.0.0.1", port: 443 }) + expect(error.message).toContain("Permission denied") + }) + + test("falls back to the deepest cause message when the errno is unknown", () => { + const error = bindError(serveError(new Error("socket exploded")), target) + expect(error.message).toBe("Failed to start server on 127.0.0.1:4096: socket exploded") + }) + + test("never loses the original error", () => { + const original = serveError(errno("EADDRINUSE", "in use")) + expect(bindError(original, target).cause).toBe(original) + }) + + test("terminates on a self-referential cause chain", () => { + const looped: { cause?: unknown } = {} + looped.cause = looped + // Would hang without the `seen` guard; reaching the assertion is the test. + expect(bindError(looped, target).message).toBe("Failed to start server on 127.0.0.1:4096") + }) + }) +}) diff --git a/packages/tui/src/util/error.ts b/packages/tui/src/util/error.ts index 757758a2058d..a4997f93b5bb 100644 --- a/packages/tui/src/util/error.ts +++ b/packages/tui/src/util/error.ts @@ -125,6 +125,12 @@ export function errorFormat(error: unknown): string { export function errorMessage(error: unknown): string { if (error instanceof Error) { if (error.message) return error.message + // Tagged errors (e.g. Effect's `ServeError`) carry an empty `message` and hide + // the real failure in `cause`. Printing just the name tells the user nothing. + if (error.cause !== undefined && error.cause !== null) { + const inner = errorMessage(error.cause) + if (inner && inner !== "unknown error") return error.name ? `${error.name}: ${inner}` : inner + } if (error.name) return error.name } diff --git a/packages/tui/test/util/error.test.ts b/packages/tui/test/util/error.test.ts index 58d6e3c06a22..e1986e80e6f3 100644 --- a/packages/tui/test/util/error.test.ts +++ b/packages/tui/test/util/error.test.ts @@ -46,4 +46,29 @@ describe("util.error", () => { expect(data.message).toBe("ResolveMessage: Cannot resolve module") expect(String(data.formatted)).toContain("ResolveMessage") }) + + test("surfaces the cause when a tagged error has no message", () => { + // Effect's `Data.TaggedError` subclasses (e.g. `ServeError`) leave `message` + // empty and park the real failure on `cause`. Printing the bare name told + // the user nothing. + const inner = Object.assign(new Error("listen EADDRINUSE: address already in use"), { code: "EADDRINUSE" }) + const tagged = new Error("") + tagged.name = "ServeError" + Object.assign(tagged, { _tag: "ServeError", cause: inner }) + + expect(errorMessage(tagged)).toBe("ServeError: listen EADDRINUSE: address already in use") + }) + + test("still falls back to the name when the cause is empty too", () => { + const tagged = new Error("") + tagged.name = "ServeError" + Object.assign(tagged, { cause: null }) + + expect(errorMessage(tagged)).toBe("ServeError") + }) + + test("prefers an explicit message over the cause", () => { + const err = new Error("outer", { cause: new Error("inner") }) + expect(errorMessage(err)).toBe("outer") + }) })