diff --git a/packages/devframe/src/adapters/__tests__/dev.test.ts b/packages/devframe/src/adapters/__tests__/dev.test.ts index 98c0da32..96434007 100644 --- a/packages/devframe/src/adapters/__tests__/dev.test.ts +++ b/packages/devframe/src/adapters/__tests__/dev.test.ts @@ -1,5 +1,6 @@ import type { DevframeNodeContext, DevframeRpcClientFunctions, DevframeRpcServerFunctions } from '../../types' import { mkdtempSync, writeFileSync } from 'node:fs' +import { createServer } from 'node:net' import { tmpdir } from 'node:os' import { join } from 'node:path' import { defineDevframe } from 'devframe' @@ -32,6 +33,15 @@ function makeTmpDist(): string { return dir } +// Holds a port on one address, like an unrelated server already running. +function listenOn(port: number, host: string): Promise> { + return new Promise((resolve, reject) => { + const server = createServer() + server.once('error', reject) + server.listen(port, host, () => resolve(server)) + }) +} + async function connectRaw(url: string, origin?: string): Promise<'open' | 'closed'> { return await new Promise((resolve) => { const ws = new WebSocket(url, origin ? { headers: { origin } } : undefined) @@ -763,6 +773,56 @@ describe('adapters/dev', () => { expect(port).toBe(override) }) + it('createDevServer on the default host does not share a port held on 127.0.0.1', async () => { + const taken = await getPort({ port: 19660, host: '127.0.0.1' }) + const blocker = await listenOn(taken, '127.0.0.1') + const devframe = defineDevframe({ + id: 'devframe-test-port-shared', + name: 'Port Shared', + version: '0.0.0', + packageName: 'devframe-test', + homepage: 'https://example.test', + description: 'Test devframe.', + setup: () => {}, + cli: { port: taken }, + }) + const handle = await createDevServer(devframe, { openBrowser: false, distDir: makeTmpDist() }) + try { + expect(handle.port).not.toBe(taken) + // A page on either loopback address reaches this dev server. + for (const host of ['127.0.0.1', '[::1]']) { + const res = await fetch(`http://${host}:${handle.port}/__connection.json`) + expect(res.status).toBe(200) + } + } + finally { + await handle.close() + blocker.close() + } + }) + + it('createDevServer fails on a pinned port held on 127.0.0.1', async () => { + const taken = await getPort({ port: 19670, host: '127.0.0.1' }) + const blocker = await listenOn(taken, '127.0.0.1') + const devframe = defineDevframe({ + id: 'devframe-test-port-pinned', + name: 'Port Pinned', + version: '0.0.0', + packageName: 'devframe-test', + homepage: 'https://example.test', + description: 'Test devframe.', + setup: () => {}, + }) + try { + await expect(createDevServer(devframe, { port: taken, openBrowser: false, distDir: makeTmpDist() })) + .rejects + .toMatchObject({ code: 'DF0052' }) + } + finally { + blocker.close() + } + }) + it('registers the instance in the registry and unregisters on close', async () => { const registryDir = mkdtempSync(join(tmpdir(), 'devframe-registry-')) vi.stubEnv('DEVFRAME_INSTANCES_DIR', registryDir) diff --git a/packages/devframe/src/adapters/cac.ts b/packages/devframe/src/adapters/cac.ts index 52d209bb..538324c8 100644 --- a/packages/devframe/src/adapters/cac.ts +++ b/packages/devframe/src/adapters/cac.ts @@ -14,7 +14,7 @@ import cac from 'cac' import { colors as c } from 'devframe/utils/colors' import { importAgenticMcp } from '../node/agentic' import { createBuild } from './build' -import { createDevServer, resolveDevServerPort } from './dev' +import { createDevServer } from './dev' import { flagKeyToOption, isBooleanFlag, parseCliFlags } from './flags' export { defineCliFlags, parseCliFlags } from './flags' @@ -103,7 +103,6 @@ export function createCac(d: DevframeDefinition, options: CreateCacOptions = {}) devCommand.action(async (_args: unknown, rawFlags: CliFlags) => { const flags = resolveTypedFlags(d, rawFlags) as CliFlags const host = (flags.host as string | undefined) ?? defaultHost - const port = (flags.port as number | undefined) ?? await resolveDevServerPort(d, { host, defaultPort }) // `--mcp` / `--no-mcp` map to a boolean override; when neither is passed // CAC leaves `mcp` undefined so we fall back to the assembly-stage // `options.mcp`, and `createDevServer` falls through to the `'auto'` @@ -111,7 +110,10 @@ export function createCac(d: DevframeDefinition, options: CreateCacOptions = {}) const mcp = (flags.mcp as boolean | undefined) ?? options.mcp await createDevServer(d, { host, - port, + // A `--port` flag pins the port; otherwise `createDevServer` resolves + // one and can move off a port it finds taken. + port: flags.port as number | undefined, + defaultPort, flags, mcp, onReady: options.onReady, diff --git a/packages/devframe/src/adapters/dev.ts b/packages/devframe/src/adapters/dev.ts index 2c5114cc..12d5eaf5 100644 --- a/packages/devframe/src/adapters/dev.ts +++ b/packages/devframe/src/adapters/dev.ts @@ -1,4 +1,5 @@ import type { DevframeRpcConnection, WsOriginRegistry } from 'devframe/rpc/transports/ws-server' +import type { Server } from 'node:http' import type { DevframeAuthHandler } from '../node/auth/handler' import type { StartedServer } from '../node/instance-shell' import type { DevframeDefinition, DevframeSseOptions, DevframeWsOptions, McpSetting } from '../types/devframe' @@ -9,6 +10,7 @@ import { open } from 'devframe/utils/open' import { withBase } from 'devframe/utils/url' import { H3, toNodeHandler } from 'h3' import { diagnostics } from '../node/diagnostics' +import { isAddressInUse, listenWithLoopbackTwin } from '../node/listen' import { normalizeHttpServerUrl } from '../node/utils' import { normalizeBasePath, resolveBasePath, resolveDevServerPort } from './_shared' import { getInstanceInternals, initDevframe } from './initiate' @@ -22,9 +24,15 @@ export interface CreateDevServerOptions { /** * Port to listen on. When omitted, falls back to * {@link resolveDevServerPort}, which respects `def.cli?.port` / - * `portRange` / `random`. + * `portRange` / `random`. A port set here is used as given: if it is + * taken, startup fails (`DF0052`) instead of moving to another port. */ port?: number + /** + * Preferred port when `port` is omitted, passed to + * {@link resolveDevServerPort}. Default: `def.cli?.port ?? 9999`. + */ + defaultPort?: number /** * Parsed flag bag forwarded to `setup(ctx, { flags })`. The dev * server itself only reads `flags.open` from this bag, and only when @@ -147,7 +155,6 @@ export async function createDevServer( throw diagnostics.DF0058({ id: def.id }) const host = options.host ?? def.cli?.host ?? 'localhost' - const requestedPort = options.port ?? await resolveDevServerPort(def, { host }) const flags = options.flags ?? {} const basePath = options.basePath ? normalizeBasePath(options.basePath) : resolveBasePath(def, 'standalone') const app = options.app ?? new H3() @@ -157,27 +164,7 @@ export async function createDevServer( // starts first so the shared WS tier and advertised origin reflect the real // bound port (`port: 0` binds an ephemeral one). const server = createServer(toNodeHandler(app)) - try { - await new Promise((resolveListen, rejectListen) => { - const onError = (error: Error): void => rejectListen(error) - // Without this listener a failed bind emits `error` with nobody - // attached (an uncaughtException) and the `listen` callback never - // fires, so this promise never settles. - server.once('error', onError) - server.listen(requestedPort, host, () => { - server.removeListener('error', onError) - resolveListen() - }) - }) - } - catch (error) { - throw diagnostics.DF0052({ - host, - port: requestedPort, - reason: error instanceof Error ? error.message : String(error), - cause: error, - }) - } + const { requestedPort, closeLoopbackTwin } = await listenDevServer(server, def, host, options) const address = server.address() const port = typeof address === 'object' && address ? address.port : requestedPort // A wildcard bind host (`0.0.0.0` / `::`) isn't dialable from a browser, so @@ -220,6 +207,7 @@ export async function createDevServer( await devframe.ready } catch (error) { + await closeLoopbackTwin() await new Promise(resolveClose => server.close(() => resolveClose())) throw error } @@ -244,11 +232,44 @@ export async function createDevServer( // transport (and closes any dedicated-port socket server), and disposes // MCP sessions; the HTTP server is this function's own to close. await devframe.close() + await closeLoopbackTwin() await new Promise(resolveClose => server.close(() => resolveClose())) }, } } +/** + * Bind the dev server. `getPort` probes only `host`, so it can call a port + * free while another process holds it on the other loopback address of + * `localhost`. The twin bind catches that, and a port that was not pinned + * moves to the next free one. + */ +async function listenDevServer( + server: Server, + def: DevframeDefinition, + host: string, + options: CreateDevServerOptions, +): Promise<{ requestedPort: number, closeLoopbackTwin: () => Promise }> { + let requestedPort = options.port ?? await resolveDevServerPort(def, { host, defaultPort: options.defaultPort }) + for (let attempt = 1; ; attempt++) { + try { + return { requestedPort, closeLoopbackTwin: await listenWithLoopbackTwin(server, requestedPort, host) } + } + catch (error) { + if (options.port == null && attempt < 3 && isAddressInUse(error)) { + requestedPort = await resolveDevServerPort(def, { host, defaultPort: requestedPort + 1 }) + continue + } + throw diagnostics.DF0052({ + host, + port: requestedPort, + reason: error instanceof Error ? error.message : String(error), + cause: error, + }) + } + } +} + async function maybeOpenBrowser( def: DevframeDefinition, flags: Record, diff --git a/packages/devframe/src/node/instance-shell.ts b/packages/devframe/src/node/instance-shell.ts index bc445db2..53b7c9db 100644 --- a/packages/devframe/src/node/instance-shell.ts +++ b/packages/devframe/src/node/instance-shell.ts @@ -22,6 +22,7 @@ import { DEVFRAME_SSE_ROUTE, DEVFRAME_WS_ROUTE } from '../constants' import { createInteractiveAuth } from '../recipes/interactive-auth' import { diagnostics } from './diagnostics' import { getInternalContext } from './hub-internals/context' +import { listenWithLoopbackTwin } from './listen' import { detectServerRuntime } from './runtime' import { formatHostForUrl, normalizeHttpServerUrl } from './utils' @@ -82,54 +83,6 @@ function websocketConnectionMeta(rpcHost: RpcFunctionsHostImpl, path: string | u return { backend: 'websocket', websocket: { path }, jsonSerializableMethods } } -function listen(server: NodeHttpServer, port: number, host: string): Promise { - return new Promise((resolve, reject) => { - const onError = (error: Error): void => reject(error) - // Without this listener a failed bind emits `error` with nobody - // attached (an uncaughtException) and the `listen` callback never - // fires, so this promise never settles. - server.once('error', onError) - server.listen(port, host, () => { - server.removeListener('error', onError) - resolve() - }) - }) -} - -/** Errors meaning the machine has no such loopback address to bind. */ -const NO_LOOPBACK_ADDRESS = new Set(['EADDRNOTAVAIL', 'EAFNOSUPPORT']) - -/** - * Listen on `host`, and when that is `localhost`, on the other loopback - * address too. `listen(port, 'localhost')` binds only the first address - * `localhost` resolves to (`::1` on most current Linux and macOS setups), - * while a client dials the side-car on its own page's hostname, so a page - * opened on `127.0.0.1` would be refused. The twin server hands its traffic - * to `server`. If another process holds the twin address, fail like a taken - * port, so pages on that address never reach a different server. A machine - * without that address family skips the twin. Resolves to a closer for the - * twin. - */ -async function listenWithLoopbackTwin(server: NodeHttpServer, port: number, host: string): Promise<() => Promise> { - await listen(server, port, host) - const bound = server.address() - if (host !== 'localhost' || !bound || typeof bound === 'string') - return async () => {} - const twin = createServer() - twin.on('request', (req, res) => server.emit('request', req, res)) - twin.on('upgrade', (req, socket, head) => server.emit('upgrade', req, socket, head)) - try { - await listen(twin, bound.port, bound.family === 'IPv6' ? '127.0.0.1' : '::1') - return () => new Promise(r => twin.close(() => r())) - } - catch (error) { - if (NO_LOOPBACK_ADDRESS.has((error as NodeJS.ErrnoException).code ?? '')) - return async () => {} - await new Promise(r => server.close(() => r())) - throw error - } -} - /** * Compose an h3 + WebSocket RPC server for a devframe context: the low-level * "listen on a port (or share one) + attach the WS transport" binding the diff --git a/packages/devframe/src/node/listen.ts b/packages/devframe/src/node/listen.ts new file mode 100644 index 00000000..ada5e4c6 --- /dev/null +++ b/packages/devframe/src/node/listen.ts @@ -0,0 +1,56 @@ +import type { Server as NodeHttpServer } from 'node:http' +import { createServer } from 'node:http' + +function listen(server: NodeHttpServer, port: number, host: string): Promise { + return new Promise((resolve, reject) => { + const onError = (error: Error): void => reject(error) + // Without this listener a failed bind emits `error` with nobody + // attached (an uncaughtException) and the `listen` callback never + // fires, so this promise never settles. + server.once('error', onError) + server.listen(port, host, () => { + server.removeListener('error', onError) + resolve() + }) + }) +} + +/** Errors meaning the machine has no such loopback address to bind. */ +const NO_LOOPBACK_ADDRESS = new Set(['EADDRNOTAVAIL', 'EAFNOSUPPORT']) + +/** Whether a failed bind means another process holds the port. */ +export function isAddressInUse(error: unknown): boolean { + return (error as NodeJS.ErrnoException | undefined)?.code === 'EADDRINUSE' +} + +/** + * Listen on `host`, and when that is `localhost`, on the other loopback + * address too. `listen(port, 'localhost')` binds only the first address + * `localhost` resolves to (`::1` on most current Linux and macOS setups), + * while a browser dials the page's own hostname, so a page opened on + * `127.0.0.1` would be refused, or reach another process that holds + * `127.0.0.1` on the same port. The twin server hands its traffic to + * `server`. If another process holds the twin address, fail like a taken + * port (`EADDRINUSE`), so pages on that address never reach a different + * server. A machine without that address family skips the twin. Resolves to + * a closer for the twin. + */ +export async function listenWithLoopbackTwin(server: NodeHttpServer, port: number, host: string): Promise<() => Promise> { + await listen(server, port, host) + const bound = server.address() + if (host !== 'localhost' || !bound || typeof bound === 'string') + return async () => {} + const twin = createServer() + twin.on('request', (req, res) => server.emit('request', req, res)) + twin.on('upgrade', (req, socket, head) => server.emit('upgrade', req, socket, head)) + try { + await listen(twin, bound.port, bound.family === 'IPv6' ? '127.0.0.1' : '::1') + return () => new Promise(r => twin.close(() => r())) + } + catch (error) { + if (NO_LOOPBACK_ADDRESS.has((error as NodeJS.ErrnoException).code ?? '')) + return async () => {} + await new Promise(r => server.close(() => r())) + throw error + } +} diff --git a/tests/__snapshots__/tsnapi/devframe/adapters/dev.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/adapters/dev.snapshot.d.ts index b6267668..a014b547 100644 --- a/tests/__snapshots__/tsnapi/devframe/adapters/dev.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/adapters/dev.snapshot.d.ts @@ -5,6 +5,7 @@ export interface CreateDevServerOptions { host?: string; port?: number; + defaultPort?: number; flags?: Record; distDir?: StaticAssetsSource; basePath?: string;