From 1047a3f52f34f334e0e10a57c6f4f33aca0628ad Mon Sep 17 00:00:00 2001 From: Felix Schneider <99918022+trueberryless@users.noreply.github.com> Date: Sun, 27 Sep 2026 22:39:55 +0200 Subject: [PATCH 1/2] feat: preview takeover other server --- docs/preview.md | 20 +- packages/nuxt-cli/src/commands/dev.ts | 4 +- packages/nuxt-cli/src/commands/preview.ts | 84 +++++++- packages/nuxt-cli/src/dev/listen.ts | 2 +- packages/nuxt-cli/src/dev/takeover.ts | 89 +++++--- packages/nuxt-cli/src/utils/lockfile.ts | 13 +- .../test/unit/commands/dev-run.spec.ts | 16 +- .../test/unit/commands/preview.spec.ts | 201 +++++++++++++++++- packages/nuxt-cli/test/unit/help.spec.ts | 6 + packages/nuxt-cli/test/unit/lockfile.spec.ts | 24 ++- packages/nuxt-cli/test/unit/takeover.spec.ts | 129 ++++++++--- .../test/unit/utils/untrusted-lock.spec.ts | 9 +- 12 files changed, 519 insertions(+), 78 deletions(-) diff --git a/docs/preview.md b/docs/preview.md index bf2fd5424..83b1c3a28 100644 --- a/docs/preview.md +++ b/docs/preview.md @@ -10,7 +10,7 @@ links: ```bash [Terminal] -npx nuxt preview [ROOTDIR] [--cwd=] [--logLevel=] [--envName=] [-e, --extends=...] [-p, --port=] [-h, --host=] [--dotenv=...] +npx nuxt preview [ROOTDIR] [--cwd=] [--logLevel=] [--envName=] [-e, --extends=...] [-p, --port=] [-h, --host=] [--takeover] [--strictPort] [--dotenv=...] ``` @@ -39,9 +39,27 @@ Some Nitro presets do not produce a server that can be run locally. For those, t | `-e, --extends=...` | | Extend from a Nuxt layer | | `-p, --port=` | | Port to listen on (default: `NUXT_PORT \|\| NITRO_PORT \|\| PORT`) | | `-h, --host=` | | Host to listen on (default: `NUXT_HOST \|\| NITRO_HOST \|\| HOST`) | +| `--takeover` | | Stop a preview server already running on this project and take its place | +| `--no-takeover` | | Never stop a preview server already running on this project | +| `--strictPort` | `false` | Exit if the requested port is unavailable instead of using another one | | `--dotenv=...` | | Path to `.env` file to load, relative to the root directory. Can be repeated, with later files taking precedence. | +## Taking over a running preview + +A preview records itself in `node_modules/.cache/nuxt/preview`, so a second `nuxt preview` for the same project deals with the one already running the way [`nuxt dev`](/docs/api/commands/dev#taking-over-a-running-dev-server) deals with a running dev server: + +| Running preview was started | New preview is started | What happens | +|-----------------------------|------------------------|--------------| +| without a terminal | without a terminal | It is stopped, and the new one takes its port. | +| without a terminal | in a terminal | You are asked, defaulting to taking it over. | +| in a terminal | without a terminal | The new one exits, saying where the running one is. | +| in a terminal | in a terminal | You are asked, defaulting to not starting. | + +Pass `--takeover` to always stop it and start in its place, or `--no-takeover` to never do so. Choosing "Start anyway" at the prompt, or passing a different `--port`, runs a second preview alongside it instead. + +When anything else is using the port (`3000` unless you pass `--port`), the preview moves to another free port as `nuxt dev` does, or exits if you pass `--strictPort`. This only applies to presets whose server runs directly with Node.js, Bun or Deno; other presets' preview commands choose their own port. + This command sets `process.env.NODE_ENV` to `production`. To override, define `NODE_ENV` in a `.env` file or as command-line argument. ::note diff --git a/packages/nuxt-cli/src/commands/dev.ts b/packages/nuxt-cli/src/commands/dev.ts index 4fff5400f..b92955c49 100644 --- a/packages/nuxt-cli/src/commands/dev.ts +++ b/packages/nuxt-cli/src/commands/dev.ts @@ -20,7 +20,7 @@ import { preflight } from '../dev/preflight' import { formatRestartReason } from '../dev/reason' import { devShortcutContext } from '../dev/shortcut-context' import { SUPERVISOR_SHUTDOWN_TIMEOUT_MS } from '../dev/shutdown' -import { formatTakeoverRefusal, takeOverDevServer } from '../dev/takeover' +import { formatTakeoverRefusal, takeOverServer } from '../dev/takeover' import { beginDevUI, setupDevUI, teardownDevUI } from '../dev/tui/controller' import { replaceCwdArg } from '../utils/args' import { resolveLockDir } from '../utils/dev-server' @@ -185,7 +185,7 @@ const command = defineCommand({ const buildDir = await beforeServing(() => resolveLockDir(cwd)) - const takeover = await beforeServing(() => takeOverDevServer(buildDir, { + const takeover = await beforeServing(() => takeOverServer(buildDir, { requestedPort: parsePort(listenOverrides.port), takeover: ctx.args.takeover, })) diff --git a/packages/nuxt-cli/src/commands/preview.ts b/packages/nuxt-cli/src/commands/preview.ts index 5c157a540..902c94aa7 100644 --- a/packages/nuxt-cli/src/commands/preview.ts +++ b/packages/nuxt-cli/src/commands/preview.ts @@ -6,11 +6,16 @@ import { styleText } from 'node:util' import { box } from '@clack/prompts' import { tokenizeArgs } from 'args-tokenizer' import { defineCommand } from 'citty' +import { checkPort } from 'get-port-please' import { resolve } from 'pathe' import { x } from 'tinyexec' +import { parsePort, resolvePort } from '../dev/listen' +import { formatTakeoverRefusal, takeOverServer } from '../dev/takeover' import { resolveDotenvFileNames } from '../utils/args' +import { ActionableError } from '../utils/errors' import { loadKit } from '../utils/kit' +import { acquireLock, previewLockDir, updateLock } from '../utils/lockfile' import { logger, outro } from '../utils/logger' import { withPrependedPath } from '../utils/path-env' import { relativeToProcess, resolveRootDir } from '../utils/paths' @@ -18,6 +23,10 @@ import { resolveServerBuild } from '../utils/server-build' import { findStaticEntry, formatServerURL, previewStaticOutput } from '../utils/static-preview' import { dotEnvArgs, envNameArgs, extendsArgs, logLevelArgs, rootDirArgs } from './_shared' +// Other preview commands (such as `wrangler dev`) choose their own port. +const PORT_AWARE_RUNTIMES = new Set(['node', 'bun', 'deno']) +const DEFAULT_PORT = 3000 + const command = defineCommand({ meta: { name: 'preview', @@ -40,6 +49,16 @@ const command = defineCommand({ valueHint: 'host', alias: ['h'], }, + takeover: { + type: 'boolean', + description: 'Stop a preview server already running on this project and take its place', + negativeDescription: 'Never stop a preview server already running on this project', + }, + strictPort: { + type: 'boolean', + description: 'Exit if the requested port is unavailable instead of using another one', + default: false, + }, ...dotEnvArgs, }, async run(ctx) { @@ -106,6 +125,31 @@ const command = defineCommand({ || process.env.NITRO_HOST || process.env.HOST + async function claimPort(): Promise { + const requestedPort = parsePort(port) + const takeover = await takeOverServer(previewLockDir(cwd), { + command: 'preview', + requestedPort, + takeover: ctx.args.takeover, + }) + if (takeover.action === 'refused') { + logger.error(formatTakeoverRefusal(takeover.existing, takeover.reason)) + process.exit(1) + } + if (takeover.action === 'taken') { + return takeover.port + } + + const listenPort = requestedPort ?? DEFAULT_PORT + if (!ctx.args.strictPort) { + return resolvePort(listenPort, host || '') + } + if (listenPort !== 0 && await checkPort(listenPort, host || undefined) === false) { + throw new ActionableError(`Port ${listenPort} is already in use (\`--strictPort\` is enabled).`) + } + return listenPort + } + let previewCommand: string | undefined let outputPath: string | undefined let target: readonly [label: string, value: string] | undefined @@ -154,7 +198,9 @@ const command = defineCommand({ const entry = findStaticEntry(dir) if (entry) { logger.info(`This build has no server, so ${styleText('cyan', relativeToProcess(dir))} is being served statically.`) - const server = await previewStaticOutput({ dir, entry, port, hostname: host }) + const listenPort = await claimPort() + const server = await previewStaticOutput({ dir, entry, port: String(listenPort), hostname: host }) + recordPreview(cwd, listenPort, host) outro(`Previewing ${styleText('cyan', relativeToProcess(dir))} at ${styleText('cyan', formatServerURL(server.url))}`) return } @@ -174,6 +220,10 @@ const command = defineCommand({ // `outputPath` is set whenever a preview command was found. const previewDir = outputPath! + const [command, ...commandArgs] = tokenizeArgs(previewCommand) as [string, ...string[]] + const listenPort = PORT_AWARE_RUNTIMES.has(command) ? await claimPort() : undefined + const serverPort = listenPort === undefined ? port : String(listenPort) + const info = [ ['Node.js:', `v${process.versions.node}`], ...(target ? [target] : []), @@ -227,8 +277,8 @@ const command = defineCommand({ outro(`Running ${styleText('cyan', previewCommand)} in ${styleText('cyan', relativeToProcess(previewDir))}`) - const [command, ...commandArgs] = tokenizeArgs(previewCommand) as [string, ...string[]] - await x(command, commandArgs, { + const recordServer = listenPort === undefined ? undefined : recordPreview(cwd, listenPort, host) + const server = x(command, commandArgs, { throwOnError: true, nodeOptions: { stdio: 'inherit', @@ -238,14 +288,38 @@ const command = defineCommand({ resolve(previewDir, 'node_modules/.bin'), resolve(cwd, 'node_modules/.bin'), ]), - NUXT_PORT: port, - NITRO_PORT: port, + NUXT_PORT: serverPort, + NITRO_PORT: serverPort, NUXT_HOST: host, NITRO_HOST: host, }, }, }) + if (recordServer && server.pid) { + recordServer(server.pid) + } + await server }, }) export default command + +function recordPreview(rootDir: string, port: number, hostname: string | undefined): (serverPid: number) => void { + if (port === 0) { + return () => {} + } + const lockDir = previewLockDir(rootDir) + const host = hostname || 'localhost' + const info = { + command: 'preview' as const, + cwd: rootDir, + port, + hostname, + url: `http://${host.includes(':') && !host.startsWith('[') ? `[${host}]` : host}:${port}`, + } + const { release } = acquireLock(lockDir, info) + if (!release) { + return () => {} + } + return serverPid => updateLock(lockDir, { ...info, serverPid }) +} diff --git a/packages/nuxt-cli/src/dev/listen.ts b/packages/nuxt-cli/src/dev/listen.ts index 8f09cdd6b..b8e4f3190 100644 --- a/packages/nuxt-cli/src/dev/listen.ts +++ b/packages/nuxt-cli/src/dev/listen.ts @@ -488,7 +488,7 @@ export function parsePort(value: string | number | undefined): number | undefine return port } -async function resolvePort(requestedPort: number | undefined, hostname: string, strictPort?: boolean): Promise { +export async function resolvePort(requestedPort: number | undefined, hostname: string, strictPort?: boolean): Promise { if (requestedPort === 0) { return getPort({ random: true, host: hostname || undefined }) } diff --git a/packages/nuxt-cli/src/dev/takeover.ts b/packages/nuxt-cli/src/dev/takeover.ts index 99ffc444e..044c39e03 100644 --- a/packages/nuxt-cli/src/dev/takeover.ts +++ b/packages/nuxt-cli/src/dev/takeover.ts @@ -42,7 +42,24 @@ export type TakeoverResult export type TakeoverChoice = 'takeover' | 'abort' | 'start-anyway' +type ServerCommand = 'dev' | 'preview' + +const SERVER_KINDS: Record = { + dev: { + label: 'dev server', + startAnywayHint: 'unsupported: both servers share the build directory', + secondServer: '`NUXT_IGNORE_LOCK=1` to run a second server (unsupported)', + }, + preview: { + label: 'preview server', + startAnywayHint: 'on another free port', + secondServer: '`--port` to run a second one alongside it', + }, +} + export interface TakeoverOptions { + /** Which kind of server to take over. Defaults to `dev`. */ + command?: ServerCommand /** Port this invocation was explicitly asked to use, if any. */ requestedPort?: number /** `--takeover` / `--no-takeover`; either skips the prompt. */ @@ -56,31 +73,31 @@ export interface TakeoverOptions { } /** - * Decide what to do about an existing dev server on this build directory, and - * carry out a takeover if that is the answer. + * Decide what to do about an existing server of the same kind recorded in + * `lockDir`, and carry out a takeover if that is the answer. * * Must be called before anything binds a port or writes to the build directory, * because a takeover adopts the port the outgoing server was using. */ -export function takeOverDevServer(buildDir: string, options: TakeoverOptions = {}): Promise { +export function takeOverServer(lockDir: string, options: TakeoverOptions = {}): Promise { // The prompt and the spinner below both redraw by moving the cursor, so they // need stdout back from consola for the duration. - return withDirectStdout(() => resolveTakeover(buildDir, options)) + return withDirectStdout(() => resolveTakeover(lockDir, options)) } -async function resolveTakeover(buildDir: string, options: TakeoverOptions): Promise { +async function resolveTakeover(lockDir: string, options: TakeoverOptions): Promise { if (!isLockEnabled()) { return { action: 'none' } } - const existing = readLock(buildDir) + const existing = readLock(lockDir) if (!existing || existing.pid === process.pid) { return { action: 'none' } } - // Signalling a `build` is never on the table, and a dev server that has not + // Signalling a `build` is never on the table, and a server that has not // bound a port yet cannot be identified well enough to touch. - if (existing.command !== 'dev' || !existing.port) { + if (existing.command !== (options.command ?? 'dev') || !existing.port) { return { action: 'none' } } @@ -94,9 +111,9 @@ async function resolveTakeover(buildDir: string, options: TakeoverOptions): Prom // and would otherwise refuse to start on behalf of a server that is not there. if (!alive || portFree) { if (!alive && !portFree) { - logger.warn(`The dev server that was using port ${existing.port} is gone, but something is still listening there.`) + logger.warn(`The ${describeServer(existing)} that was using port ${existing.port} is gone, but something is still listening there.`) } - clearStaleLock(buildDir, existing) + clearStaleLock(lockDir, existing) return { action: 'stale' } } @@ -132,43 +149,45 @@ async function resolveTakeover(buildDir: string, options: TakeoverOptions): Prom return { action: 'refused', existing, reason: 'declined' } } if (choice === 'start-anyway') { - logger.warn(`Starting a second dev server: both will write to ${styleText('cyan', buildDir)}, which is unsupported and may corrupt the build.`) + if (existing.command === 'dev') { + logger.warn(`Starting a second dev server: both will write to ${styleText('cyan', lockDir)}, which is unsupported and may corrupt the build.`) + } return { action: 'start-anyway', existing } } } } - return withUserAttention(() => performTakeover(buildDir, existing, options.timeouts)) + return withUserAttention(() => performTakeover(lockDir, existing, options.timeouts)) } -async function performTakeover(buildDir: string, existing: LockInfo, timeouts: TakeoverOptions['timeouts'] = {}): Promise { +async function performTakeover(lockDir: string, existing: LockInfo, timeouts: TakeoverOptions['timeouts'] = {}): Promise { const port = existing.port! + const label = describeServer(existing) const startedAt = new Date(existing.startedAt).toLocaleTimeString() - const progress = startProgress(`Taking over the dev server on port ${port} (PID ${existing.pid}, started ${startedAt})`) + const progress = startProgress(`Taking over the ${label} on port ${port} (PID ${existing.pid}, started ${startedAt})`) - markTakenOver(buildDir, process.pid) + markTakenOver(lockDir, process.pid) - const pids = existing.parentPid && existing.parentPid !== existing.pid - ? [existing.pid, existing.parentPid] - : [existing.pid] + const pids = [...new Set([existing.pid, existing.parentPid, existing.serverPid])] + .filter((pid): pid is number => !!pid) // On Windows `SIGTERM` is not delivered as a signal and terminates the process // outright, so the graceful window below simply passes quickly there. signalAll(pids, 'SIGTERM') if (await waitForRelease(pids, port, existing.hostname, timeouts.graceful ?? DEV_SHUTDOWN_TIMEOUT_MS)) { - progress.stop(`Stopped the dev server on port ${port} (PID ${existing.pid})`) + progress.stop(`Stopped the ${label} on port ${port} (PID ${existing.pid})`) return { action: 'taken', port, pid: existing.pid } } - progress.update(`Waiting for the dev server on port ${port} to exit`) + progress.update(`Waiting for the ${label} on port ${port} to exit`) signalAll(pids, 'SIGKILL') if (await waitForRelease(pids, port, existing.hostname, timeouts.force ?? TAKEOVER_KILL_TIMEOUT_MS)) { - progress.stop(`Stopped the dev server on port ${port} (PID ${existing.pid})`) + progress.stop(`Stopped the ${label} on port ${port} (PID ${existing.pid})`) return { action: 'taken', port, pid: existing.pid } } - progress.fail(`Could not stop the dev server on port ${port}`) - clearTakeover(buildDir, process.pid) + progress.fail(`Could not stop the ${label} on port ${port}`) + clearTakeover(lockDir, process.pid) return { action: 'refused', existing, reason: 'timeout' } } @@ -211,8 +230,8 @@ export function formatTakeoverRefusal(existing: LockInfo, reason: TakeoverRefusa const lines = [ '', reason === 'timeout' - ? `The dev server on port ${existing.port} did not exit, so this one will not start.` - : 'Another Nuxt dev server is already running:', + ? `The ${describeServer(existing)} on port ${existing.port} did not exit, so this one will not start.` + : `Another Nuxt ${describeServer(existing)} is already running:`, '', ` URL: ${location}`, ` PID: ${existing.pid}`, @@ -225,32 +244,40 @@ export function formatTakeoverRefusal(existing: LockInfo, reason: TakeoverRefusa lines.push('It was started interactively, so it is not stopped automatically.') } if (reason !== 'timeout') { - lines.push('Pass `--takeover` to stop it and start this server in its place, or `NUXT_IGNORE_LOCK=1` to run a second server (unsupported).') + lines.push(`Pass \`--takeover\` to stop it and start this server in its place, or ${serverKind(existing).secondServer}.`) } lines.push('') return lines.join('\n') } +function serverKind(existing: LockInfo) { + return SERVER_KINDS[existing.command === 'preview' ? 'preview' : 'dev'] +} + +function describeServer(existing: LockInfo): string { + return serverKind(existing).label +} + function describeLocation(existing: LockInfo): string { return existing.url || 'starting up (no URL yet)' } async function promptForTakeover(existing: LockInfo, defaultChoice: TakeoverChoice): Promise { - logger.info(`A Nuxt dev server is already running here (PID ${existing.pid}, ${describeLocation(existing)}).`) + logger.info(`A Nuxt ${describeServer(existing)} is already running here (PID ${existing.pid}, ${describeLocation(existing)}).`) const choice = await select({ message: 'What would you like to do?', initialValue: defaultChoice, options: [ { value: 'takeover', label: `Take over port ${existing.port}`, hint: 'stops the running server (--takeover)' }, { value: 'abort', label: 'Do not start', hint: '--no-takeover' }, - { value: 'start-anyway', label: 'Start anyway', hint: 'unsupported: both servers share the build directory' }, + { value: 'start-anyway', label: 'Start anyway', hint: serverKind(existing).startAnywayHint }, ], }) restoreRawMode() // Ctrl-C must never be the thing that stops the other server. if (isCancel(choice)) { - cancel('Not starting a second dev server.') + cancel(`Not starting a second ${describeServer(existing)}.`) return 'abort' } return choice @@ -265,8 +292,10 @@ function signalAll(pids: number[], signal: NodeJS.Signals): void { } } +// A lock without a hostname was bound to every interface, and on macOS a +// `localhost` probe succeeds next to such a listener, so check them all. async function isPortFree(port: number, hostname?: string): Promise { - return await checkPort(port, hostname || 'localhost') !== false + return await checkPort(port, hostname || undefined) !== false } async function waitForRelease(pids: number[], port: number, hostname: string | undefined, timeout: number): Promise { diff --git a/packages/nuxt-cli/src/utils/lockfile.ts b/packages/nuxt-cli/src/utils/lockfile.ts index 628d43626..74d0a51a1 100644 --- a/packages/nuxt-cli/src/utils/lockfile.ts +++ b/packages/nuxt-cli/src/utils/lockfile.ts @@ -9,7 +9,7 @@ import { isInteractiveSession } from './stdout' export interface LockInfo { pid: number startedAt: number - command: 'dev' | 'build' | 'analyze' + command: 'dev' | 'build' | 'analyze' | 'preview' cwd: string /** * Whether the holder was started from a terminal a user is sitting at. Only @@ -25,6 +25,8 @@ export interface LockInfo { * Signalling the holder alone would leave its supervisor running. */ parentPid?: number + /** PID of the child process serving, for a holder that spawns one (`nuxt preview`). */ + serverPid?: number /** PID of the process that claimed this lock, written before it signals us. */ takenOverBy?: number } @@ -130,7 +132,7 @@ function readLockFile(lockPath: string): LockInfo | undefined { } } -const LOCK_COMMANDS = new Set(['dev', 'build', 'analyze']) +const LOCK_COMMANDS = new Set(['dev', 'build', 'analyze', 'preview']) const MAX_LOCK_STRING_LENGTH = 1024 // C0 and C1 control characters, which would otherwise reach the terminal when a // lock is described to the user. @@ -192,6 +194,7 @@ export function parseLockInfo(raw: unknown): LockInfo | undefined { const port = input.port const hostname = lockText(input.hostname) const parentPid = lockPid(input.parentPid) + const serverPid = lockPid(input.serverPid) const takenOverBy = lockPid(input.takenOverBy) return { @@ -204,6 +207,7 @@ export function parseLockInfo(raw: unknown): LockInfo | undefined { ...hostname && LOCK_HOSTNAME_RE.test(hostname) ? { hostname } : {}, ...lockURL(input.url) ? { url: lockURL(input.url) } : {}, ...parentPid ? { parentPid } : {}, + ...serverPid ? { serverPid } : {}, ...takenOverBy ? { takenOverBy } : {}, } } @@ -302,6 +306,11 @@ export function acquireOutputLock( return acquireLockAt(join(dir, `output-${key}.lock`), dir, info) } +/** Kept apart from the build directory, which `nuxt preview` never writes to. */ +export function previewLockDir(rootDir: string): string { + return join(rootDir, OUTPUT_LOCK_DIRNAME, 'preview') +} + function acquireLockAt( lockPath: string, dir: string, diff --git a/packages/nuxt-cli/test/unit/commands/dev-run.spec.ts b/packages/nuxt-cli/test/unit/commands/dev-run.spec.ts index a2c94a73c..3c2742f94 100644 --- a/packages/nuxt-cli/test/unit/commands/dev-run.spec.ts +++ b/packages/nuxt-cli/test/unit/commands/dev-run.spec.ts @@ -18,7 +18,7 @@ const { preflight, setupShortcuts, startWarming, - takeOverDevServer, + takeOverServer, } = vi.hoisted(() => ({ close: vi.fn(() => Promise.resolve()), createFork: vi.fn(), @@ -31,7 +31,7 @@ const { preflight: vi.fn((options: { cwd: string }) => Promise.resolve(options.cwd)), setupShortcuts: vi.fn(), startWarming: vi.fn(), - takeOverDevServer: vi.fn<(buildDir: string, options?: TakeoverOptions) => Promise>(() => Promise.resolve({ action: 'none' })), + takeOverServer: vi.fn<(buildDir: string, options?: TakeoverOptions) => Promise>(() => Promise.resolve({ action: 'none' })), })) vi.mock('../../../src/dev/index', () => ({ initialize })) @@ -50,7 +50,7 @@ vi.mock('../../../src/dev/shortcuts', async importOriginal => ({ vi.mock('../../../src/utils/dev-server', () => ({ resolveLockDir: (cwd: string) => Promise.resolve(`${cwd}/.nuxt`) })) vi.mock('../../../src/dev/takeover', async importOriginal => ({ ...await importOriginal(), - takeOverDevServer, + takeOverServer, })) vi.mock('../../../src/dev/pool', () => ({ ForkPool: class { @@ -93,7 +93,7 @@ let exit: ReturnType beforeEach(() => { vi.clearAllMocks() - takeOverDevServer.mockResolvedValue({ action: 'none' }) + takeOverServer.mockResolvedValue({ action: 'none' }) isReusePortSupported.mockResolvedValue(true) preflight.mockImplementation((options: { cwd: string }) => Promise.resolve(options.cwd)) initialize.mockImplementation(() => Promise.resolve({ @@ -176,7 +176,7 @@ describe('dev command startup', () => { describe('dev command takeover', () => { it('should not start when a takeover is refused', async () => { - takeOverDevServer.mockResolvedValue({ action: 'refused', existing: existingLock(), reason: 'declined' }) + takeOverServer.mockResolvedValue({ action: 'refused', existing: existingLock(), reason: 'declined' }) await expect(runDev(['--no-fork'])).rejects.toThrow('process.exit') @@ -185,7 +185,7 @@ describe('dev command takeover', () => { }) it('should adopt the port of the server it took over', async () => { - takeOverDevServer.mockResolvedValue({ action: 'taken', port: 3210, pid: 4321 }) + takeOverServer.mockResolvedValue({ action: 'taken', port: 3210, pid: 4321 }) await runDev(['--no-fork']) @@ -197,11 +197,11 @@ describe('dev command takeover', () => { it('should ask the takeover for the port it was given', async () => { await runDev(['--no-fork', '--port=4001']) - expect(vi.mocked(takeOverDevServer).mock.calls[0]![1]).toMatchObject({ requestedPort: 4001 }) + expect(vi.mocked(takeOverServer).mock.calls[0]![1]).toMatchObject({ requestedPort: 4001 }) }) it('should bypass the lock when the user starts a second server anyway', async () => { - takeOverDevServer.mockResolvedValue({ action: 'start-anyway', existing: existingLock() }) + takeOverServer.mockResolvedValue({ action: 'start-anyway', existing: existingLock() }) vi.stubEnv('NUXT_IGNORE_LOCK', '') await runDev(['--no-fork']) diff --git a/packages/nuxt-cli/test/unit/commands/preview.spec.ts b/packages/nuxt-cli/test/unit/commands/preview.spec.ts index a171e9204..7f4940c7b 100644 --- a/packages/nuxt-cli/test/unit/commands/preview.spec.ts +++ b/packages/nuxt-cli/test/unit/commands/preview.spec.ts @@ -1,13 +1,20 @@ +import type { LockInfo } from '../../../src/utils/lockfile' + import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' +import process from 'node:process' import { runCommand } from 'citty' import { join } from 'pathe' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import preview from '../../../src/commands/preview' +import { previewLockDir, readLock } from '../../../src/utils/lockfile' +import { logger } from '../../../src/utils/logger' -const { loadKit, loadNuxt, x } = vi.hoisted(() => ({ +const { checkPort, getPort, loadKit, loadNuxt, x } = vi.hoisted(() => ({ + checkPort: vi.fn<(port: number, host?: string) => Promise>(), + getPort: vi.fn<(options: { port?: number }) => Promise>(), loadKit: vi.fn(), loadNuxt: vi.fn(), x: vi.fn(), @@ -15,9 +22,53 @@ const { loadKit, loadNuxt, x } = vi.hoisted(() => ({ vi.mock('../../../src/utils/kit', () => ({ loadKit })) vi.mock('tinyexec', () => ({ x })) +vi.mock('get-port-please', () => ({ checkPort, getPort })) let cwd: string +const HOLDER_PID = 424242 +const SERVER_PID = 434343 + +async function writePreviewLock(info: Partial = {}) { + const lockDir = previewLockDir(cwd) + await mkdir(lockDir, { recursive: true }) + await writeFile(join(lockDir, 'nuxt.lock'), JSON.stringify({ + pid: HOLDER_PID, + startedAt: Date.now(), + command: 'preview', + cwd, + interactive: false, + port: 4500, + url: 'http://localhost:4500', + serverPid: SERVER_PID, + ...info, + })) +} + +/** Every process is alive until it is sent `SIGTERM`, which also frees its port. */ +function mockProcesses() { + const signals: Array<[number, string | number | undefined]> = [] + vi.spyOn(process, 'kill').mockImplementation((pid, signal) => { + if (signal !== 0 && signal !== undefined) { + signals.push([pid as number, signal]) + checkPort.mockImplementation(async port => port) + } + else if (pid !== process.pid && signals.some(([signalled]) => signalled === pid)) { + throw Object.assign(new Error('no such process'), { code: 'ESRCH' }) + } + return true as never + }) + return signals +} + +function expectServerPort(port: string | undefined) { + expect(x).toHaveBeenCalledWith(expect.any(String), expect.any(Array), expect.objectContaining({ + nodeOptions: expect.objectContaining({ + env: expect.objectContaining({ NUXT_PORT: port, NITRO_PORT: port }), + }), + })) +} + async function writeNitroJSON(outputDir: string, data: Record = {}) { await mkdir(outputDir, { recursive: true }) await writeFile(join(outputDir, 'nitro.json'), JSON.stringify({ @@ -45,10 +96,13 @@ describe('preview', () => { return nuxt }) x.mockResolvedValue({ exitCode: 0 }) + checkPort.mockImplementation(async port => port) + getPort.mockImplementation(async ({ port }) => port!) }) afterEach(async () => { vi.unstubAllEnvs() + vi.restoreAllMocks() vi.clearAllMocks() await rm(cwd, { recursive: true, force: true }) }) @@ -129,4 +183,149 @@ describe('preview', () => { }), })) }) + + it('listens on port 3000 when none is given', async () => { + await writeNitroJSON(join(cwd, '.output')) + + await runCommand(preview, { rawArgs: [cwd] }) + + expect(getPort).toHaveBeenCalledWith(expect.objectContaining({ port: 3000 })) + expectServerPort('3000') + }) + + it('leaves the port to preview commands that choose their own', async () => { + await writeNitroJSON(join(cwd, '.output'), { + preset: 'cloudflare-module', + commands: { preview: 'npx wrangler dev' }, + }) + + await runCommand(preview, { rawArgs: [cwd] }) + + expect(getPort).not.toHaveBeenCalled() + expect(x).toHaveBeenCalledWith('npx', ['wrangler', 'dev'], expect.anything()) + expectServerPort(undefined) + }) + + it('moves to another free port when something else holds the one asked for', async () => { + getPort.mockResolvedValue(4322) + const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + await writeNitroJSON(join(cwd, '.output')) + + await runCommand(preview, { rawArgs: [cwd, '--port=4321'] }) + + expect(warn).toHaveBeenCalledWith('Port 4321 is in use, using port 4322 instead.') + expectServerPort('4322') + }) + + it('refuses a taken port with `--strictPort`', async () => { + checkPort.mockResolvedValue(false) + await writeNitroJSON(join(cwd, '.output')) + + await expect(runCommand(preview, { rawArgs: [cwd, '--port=4321', '--strictPort'] })) + .rejects + .toThrow('Port 4321 is already in use (`--strictPort` is enabled).') + expect(x).not.toHaveBeenCalled() + }) + + it('records the server it starts so a later preview can take it over', async () => { + x.mockReturnValue(Object.assign(Promise.resolve({ exitCode: 0 }), { pid: 4242 })) + await writeNitroJSON(join(cwd, '.output')) + + await runCommand(preview, { rawArgs: [cwd, '--port=4321'] }) + + expect(readLock(previewLockDir(cwd))).toMatchObject({ + pid: process.pid, + command: 'preview', + port: 4321, + url: 'http://localhost:4321', + serverPid: 4242, + }) + }) + + describe('with a preview of this project already running', () => { + beforeEach(async () => { + await writeNitroJSON(join(cwd, '.output')) + checkPort.mockResolvedValue(false) + }) + + it('takes it over when nobody is watching it, adopting its port', async () => { + await writePreviewLock() + const signals = mockProcesses() + + await runCommand(preview, { rawArgs: [cwd] }) + + expect(signals).toEqual([[HOLDER_PID, 'SIGTERM'], [SERVER_PID, 'SIGTERM']]) + expectServerPort('4500') + }) + + it('refuses to stop one that someone is watching, saying where it is', async () => { + await writePreviewLock({ interactive: true }) + const signals = mockProcesses() + const error = vi.spyOn(logger, 'error').mockImplementation(() => {}) + vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`exit ${code}`) + }) + + await expect(runCommand(preview, { rawArgs: [cwd] })).rejects.toThrow('exit 1') + + expect(signals).toHaveLength(0) + expect(x).not.toHaveBeenCalled() + expect(error).toHaveBeenCalledWith(expect.stringContaining('Another Nuxt preview server is already running')) + expect(error).toHaveBeenCalledWith(expect.stringContaining('`--port` to run a second one alongside it')) + }) + + it('takes over one that someone is watching with `--takeover`', async () => { + await writePreviewLock({ interactive: true }) + const signals = mockProcesses() + + await runCommand(preview, { rawArgs: [cwd, '--takeover'] }) + + expect(signals).toEqual([[HOLDER_PID, 'SIGTERM'], [SERVER_PID, 'SIGTERM']]) + expectServerPort('4500') + }) + + it('never stops it with `--no-takeover`', async () => { + await writePreviewLock() + const signals = mockProcesses() + vi.spyOn(logger, 'error').mockImplementation(() => {}) + vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`exit ${code}`) + }) + + await expect(runCommand(preview, { rawArgs: [cwd, '--no-takeover'] })).rejects.toThrow('exit 1') + expect(signals).toHaveLength(0) + }) + + it('starts alongside it on another port asked for', async () => { + await writePreviewLock() + const signals = mockProcesses() + checkPort.mockImplementation(async port => port === 4500 ? false : port) + + await runCommand(preview, { rawArgs: [cwd, '--port=4600'] }) + + expect(signals).toHaveLength(0) + expectServerPort('4600') + expect(readLock(previewLockDir(cwd))).toMatchObject({ pid: HOLDER_PID }) + }) + + it('ignores a dev server of this project', async () => { + await mkdir(join(cwd, '.nuxt'), { recursive: true }) + await writeFile(join(cwd, '.nuxt', 'nuxt.lock'), JSON.stringify({ + pid: HOLDER_PID, + startedAt: Date.now(), + command: 'dev', + cwd, + interactive: false, + port: 3000, + })) + const signals = mockProcesses() + getPort.mockResolvedValue(3001) + vi.spyOn(logger, 'warn').mockImplementation(() => {}) + + await runCommand(preview, { rawArgs: [cwd] }) + + expect(signals).toHaveLength(0) + expectServerPort('3001') + }) + }) }) diff --git a/packages/nuxt-cli/test/unit/help.spec.ts b/packages/nuxt-cli/test/unit/help.spec.ts index d0ac8131f..12d08d20f 100644 --- a/packages/nuxt-cli/test/unit/help.spec.ts +++ b/packages/nuxt-cli/test/unit/help.spec.ts @@ -380,6 +380,9 @@ describe('help', () => { -e, --extends=... Extend from a Nuxt layer -p, --port= Port to listen on (default: \`NUXT_PORT || NITRO_PORT || PORT\`) -h, --host= Host to listen on (default: \`NUXT_HOST || NITRO_HOST || HOST\`) + --takeover Stop a preview server already running on this project and take its place + --no-takeover Never stop a preview server already running on this project + --strictPort Exit if the requested port is unavailable instead of using another one (Default: false) --dotenv=... Path to \`.env\` file to load, relative to the root directory. Can be repeated, with later files taking precedence. " `) @@ -402,6 +405,9 @@ describe('help', () => { -e, --extends=... Extend from a Nuxt layer -p, --port= Port to listen on (default: \`NUXT_PORT || NITRO_PORT || PORT\`) -h, --host= Host to listen on (default: \`NUXT_HOST || NITRO_HOST || HOST\`) + --takeover Stop a preview server already running on this project and take its place + --no-takeover Never stop a preview server already running on this project + --strictPort Exit if the requested port is unavailable instead of using another one (Default: false) --dotenv=... Path to \`.env\` file to load, relative to the root directory. Can be repeated, with later files taking precedence. " `) diff --git a/packages/nuxt-cli/test/unit/lockfile.spec.ts b/packages/nuxt-cli/test/unit/lockfile.spec.ts index f786b3601..7cfa69926 100644 --- a/packages/nuxt-cli/test/unit/lockfile.spec.ts +++ b/packages/nuxt-cli/test/unit/lockfile.spec.ts @@ -12,7 +12,7 @@ vi.mock('std-env', async (importOriginal) => { return { ...original, isCI: false } }) -const { acquireLock, acquireOutputLock, formatLockError, isLockEnabled, readActiveLock, updateLock } = await import('../../src/utils/lockfile') +const { acquireLock, acquireOutputLock, formatLockError, isLockEnabled, previewLockDir, readActiveLock, readLock, updateLock } = await import('../../src/utils/lockfile') describe('lockfile', () => { let tempDir: string @@ -275,6 +275,28 @@ describe('lockfile', () => { }) }) + describe('previewLockDir', () => { + it('keeps a preview apart from the dev server of the same project', () => { + const buildDir = join(tempDir, '.nuxt') + const dev = acquireLock(buildDir, { command: 'dev', cwd: tempDir, port: 3000 }) + const preview = acquireLock(previewLockDir(tempDir), { command: 'preview', cwd: tempDir, port: 3001 }) + expect(dev.release).toBeDefined() + expect(preview.release).toBeDefined() + expect(readLock(buildDir)?.command).toBe('dev') + expect(readLock(previewLockDir(tempDir))?.command).toBe('preview') + dev.release?.() + preview.release?.() + }) + + it('records the pid of the server a preview runs', () => { + const lockDir = previewLockDir(tempDir) + const { release } = acquireLock(lockDir, { command: 'preview', cwd: tempDir, port: 3000 }) + updateLock(lockDir, { command: 'preview', cwd: tempDir, port: 3000, serverPid: 4242 }) + expect(readLock(lockDir)).toMatchObject({ pid: process.pid, serverPid: 4242 }) + release?.() + }) + }) + describe('acquireOutputLock', () => { const outputDir = '/project/.output' diff --git a/packages/nuxt-cli/test/unit/takeover.spec.ts b/packages/nuxt-cli/test/unit/takeover.spec.ts index fc3b7d1ff..53623de4f 100644 --- a/packages/nuxt-cli/test/unit/takeover.spec.ts +++ b/packages/nuxt-cli/test/unit/takeover.spec.ts @@ -12,7 +12,7 @@ const checkPort = vi.hoisted(() => vi.fn<(port: number, host?: string) => Promis vi.mock('get-port-please', () => ({ checkPort })) -const { formatTakeoverRefusal, takeOverDevServer } = await import('../../src/dev/takeover') +const { formatTakeoverRefusal, takeOverServer } = await import('../../src/dev/takeover') const { logger } = await import('../../src/utils/logger') const { getTakeoverPid, markTakenOver, readLock, updateLock } = await import('../../src/utils/lockfile') @@ -59,7 +59,7 @@ function mockProcess({ alive = true, diesOn }: { alive?: boolean, diesOn?: NodeJ return { signals, kill } } -describe('takeOverDevServer', () => { +describe('takeOverServer', () => { let buildDir: string beforeEach(async () => { @@ -76,25 +76,25 @@ describe('takeOverDevServer', () => { }) it('does nothing when there is no lock', async () => { - expect(await takeOverDevServer(buildDir)).toEqual({ action: 'none' }) + expect(await takeOverServer(buildDir)).toEqual({ action: 'none' }) }) it('does nothing when locking is disabled', async () => { process.env.NUXT_IGNORE_LOCK = '1' writeLock(buildDir) - expect(await takeOverDevServer(buildDir)).toEqual({ action: 'none' }) + expect(await takeOverServer(buildDir)).toEqual({ action: 'none' }) }) it('never takes over a build lock', async () => { writeLock(buildDir, { command: 'build', port: undefined, url: undefined }) mockProcess() - expect(await takeOverDevServer(buildDir, { interactive: false })).toEqual({ action: 'none' }) + expect(await takeOverServer(buildDir, { interactive: false })).toEqual({ action: 'none' }) }) it('never takes over when an explicit port differs from the holder\'s', async () => { writeLock(buildDir) const proc = mockProcess() - expect(await takeOverDevServer(buildDir, { requestedPort: 4000, interactive: false })).toEqual({ action: 'none' }) + expect(await takeOverServer(buildDir, { requestedPort: 4000, interactive: false })).toEqual({ action: 'none' }) expect(proc.signals).toHaveLength(0) expect(readLock(buildDir)).toBeDefined() }) @@ -103,7 +103,7 @@ describe('takeOverDevServer', () => { writeLock(buildDir) checkPort.mockResolvedValue(3000) mockProcess() - expect(await takeOverDevServer(buildDir, { requestedPort: 4000, interactive: false })).toEqual({ action: 'stale' }) + expect(await takeOverServer(buildDir, { requestedPort: 4000, interactive: false })).toEqual({ action: 'stale' }) expect(readLock(buildDir)).toBeUndefined() }) @@ -115,21 +115,21 @@ describe('takeOverDevServer', () => { writeLock(buildDir, { pid: 555555, startedAt: Date.now() + 1 }) return 3000 }) - expect(await takeOverDevServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) + expect(await takeOverServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) expect(readLock(buildDir)).toMatchObject({ pid: 555555 }) }) it('takes over when the explicit port matches the holder\'s', async () => { writeLock(buildDir) mockProcess() - expect(await takeOverDevServer(buildDir, { requestedPort: 3000, interactive: false })) + expect(await takeOverServer(buildDir, { requestedPort: 3000, interactive: false })) .toEqual({ action: 'taken', port: 3000, pid: HOLDER_PID }) }) it('reports a stale lock when the holder is dead', async () => { writeLock(buildDir) const proc = mockProcess({ alive: false }) - expect(await takeOverDevServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) + expect(await takeOverServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) expect(proc.signals).toHaveLength(0) expect(readLock(buildDir)).toBeUndefined() }) @@ -138,7 +138,7 @@ describe('takeOverDevServer', () => { writeLock(buildDir) mockProcess({ alive: false }) const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {}) - expect(await takeOverDevServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) + expect(await takeOverServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) expect(warn).toHaveBeenCalledWith(expect.stringContaining('still listening')) }) @@ -147,7 +147,7 @@ describe('takeOverDevServer', () => { checkPort.mockResolvedValue(3000) mockProcess({ alive: false }) const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {}) - expect(await takeOverDevServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) + expect(await takeOverServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) expect(warn).not.toHaveBeenCalled() }) @@ -155,7 +155,7 @@ describe('takeOverDevServer', () => { writeLock(buildDir) checkPort.mockResolvedValue(3000) const proc = mockProcess() - expect(await takeOverDevServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) + expect(await takeOverServer(buildDir, { interactive: false })).toEqual({ action: 'stale' }) expect(proc.signals).toHaveLength(0) expect(readLock(buildDir)).toBeUndefined() }) @@ -164,7 +164,7 @@ describe('takeOverDevServer', () => { it('non-interactive holder, non-interactive caller: takes over', async () => { writeLock(buildDir, { interactive: false }) const proc = mockProcess() - expect(await takeOverDevServer(buildDir, { interactive: false })) + expect(await takeOverServer(buildDir, { interactive: false })) .toEqual({ action: 'taken', port: 3000, pid: HOLDER_PID }) expect(proc.signals).toEqual([[HOLDER_PID, 'SIGTERM']]) }) @@ -173,7 +173,7 @@ describe('takeOverDevServer', () => { writeLock(buildDir, { interactive: false }) mockProcess() const prompt = vi.fn(async (_lock: LockInfo, fallback: TakeoverChoice) => fallback) - const result = await takeOverDevServer(buildDir, { interactive: true, prompt }) + const result = await takeOverServer(buildDir, { interactive: true, prompt }) expect(prompt).toHaveBeenCalledWith(expect.objectContaining({ pid: HOLDER_PID }), 'takeover') expect(result).toEqual({ action: 'taken', port: 3000, pid: HOLDER_PID }) }) @@ -181,7 +181,7 @@ describe('takeOverDevServer', () => { it('interactive holder, non-interactive caller: refuses', async () => { writeLock(buildDir, { interactive: true }) const proc = mockProcess() - const result = await takeOverDevServer(buildDir, { interactive: false }) + const result = await takeOverServer(buildDir, { interactive: false }) expect(result).toMatchObject({ action: 'refused', reason: 'holder-interactive' }) expect(proc.signals).toHaveLength(0) }) @@ -190,7 +190,7 @@ describe('takeOverDevServer', () => { writeLock(buildDir, { interactive: true }) const proc = mockProcess() const prompt = vi.fn(async (_lock: LockInfo, fallback: TakeoverChoice) => fallback) - const result = await takeOverDevServer(buildDir, { interactive: true, prompt }) + const result = await takeOverServer(buildDir, { interactive: true, prompt }) expect(prompt).toHaveBeenCalledWith(expect.objectContaining({ pid: HOLDER_PID }), 'abort') expect(result).toMatchObject({ action: 'refused', reason: 'declined' }) expect(proc.signals).toHaveLength(0) @@ -202,7 +202,7 @@ describe('takeOverDevServer', () => { writeLock(buildDir, { interactive: true }) const proc = mockProcess() const prompt = vi.fn(async () => 'abort' as const) - expect(await takeOverDevServer(buildDir, { takeover: true, interactive: true, prompt })) + expect(await takeOverServer(buildDir, { takeover: true, interactive: true, prompt })) .toEqual({ action: 'taken', port: 3000, pid: HOLDER_PID }) expect(prompt).not.toHaveBeenCalled() expect(proc.signals).toEqual([[HOLDER_PID, 'SIGTERM']]) @@ -212,7 +212,7 @@ describe('takeOverDevServer', () => { writeLock(buildDir, { interactive: false }) const proc = mockProcess() const prompt = vi.fn(async () => 'takeover' as const) - expect(await takeOverDevServer(buildDir, { takeover: false, interactive: true, prompt })) + expect(await takeOverServer(buildDir, { takeover: false, interactive: true, prompt })) .toMatchObject({ action: 'refused', reason: 'disabled' }) expect(prompt).not.toHaveBeenCalled() expect(proc.signals).toHaveLength(0) @@ -221,7 +221,7 @@ describe('takeOverDevServer', () => { it('`start anyway` proceeds without a takeover', async () => { writeLock(buildDir, { interactive: true }) const proc = mockProcess() - const result = await takeOverDevServer(buildDir, { interactive: true, prompt: async () => 'start-anyway' }) + const result = await takeOverServer(buildDir, { interactive: true, prompt: async () => 'start-anyway' }) expect(result).toMatchObject({ action: 'start-anyway' }) expect(proc.signals).toHaveLength(0) expect(readLock(buildDir)?.pid).toBe(HOLDER_PID) @@ -232,14 +232,14 @@ describe('takeOverDevServer', () => { it('marks the lock so the outgoing process can explain itself', async () => { writeLock(buildDir) mockProcess() - await takeOverDevServer(buildDir, { interactive: false }) + await takeOverServer(buildDir, { interactive: false }) expect(readLock(buildDir)?.takenOverBy).toBe(process.pid) }) it('escalates to SIGKILL when SIGTERM is ignored', async () => { writeLock(buildDir) const proc = mockProcess({ diesOn: 'SIGKILL' }) - expect(await takeOverDevServer(buildDir, { interactive: false, timeouts: { graceful: 200, force: 200 } })) + expect(await takeOverServer(buildDir, { interactive: false, timeouts: { graceful: 200, force: 200 } })) .toEqual({ action: 'taken', port: 3000, pid: HOLDER_PID }) expect(proc.signals).toEqual([[HOLDER_PID, 'SIGTERM'], [HOLDER_PID, 'SIGKILL']]) }) @@ -247,7 +247,7 @@ describe('takeOverDevServer', () => { it('refuses to start when the port is still held after the deadline', async () => { writeLock(buildDir) const proc = mockProcess({ diesOn: 'never' }) - expect(await takeOverDevServer(buildDir, { interactive: false, timeouts: { graceful: 200, force: 200 } })) + expect(await takeOverServer(buildDir, { interactive: false, timeouts: { graceful: 200, force: 200 } })) .toMatchObject({ action: 'refused', reason: 'timeout' }) expect(proc.signals).toEqual([[HOLDER_PID, 'SIGTERM'], [HOLDER_PID, 'SIGKILL']]) }) @@ -255,7 +255,7 @@ describe('takeOverDevServer', () => { it('leaves the holder identifiable after giving up', async () => { writeLock(buildDir) mockProcess({ diesOn: 'never' }) - await takeOverDevServer(buildDir, { interactive: false, timeouts: { graceful: 200, force: 200 } }) + await takeOverServer(buildDir, { interactive: false, timeouts: { graceful: 200, force: 200 } }) const lock = readLock(buildDir) expect(lock?.pid).toBe(HOLDER_PID) expect(lock?.takenOverBy).toBeUndefined() @@ -264,7 +264,7 @@ describe('takeOverDevServer', () => { it('also signals the supervising process of a dev fork', async () => { writeLock(buildDir, { parentPid: 424243 }) const proc = mockProcess() - await takeOverDevServer(buildDir, { interactive: false }) + await takeOverServer(buildDir, { interactive: false }) expect(proc.signals).toEqual([[HOLDER_PID, 'SIGTERM'], [424243, 'SIGTERM']]) }) }) @@ -300,6 +300,85 @@ describe('takeOverDevServer', () => { expect(message).toContain('did not exit') expect(message).not.toContain('--takeover') }) + + it('names a preview server, and runs a second one on another port rather than ignoring the lock', () => { + const message = formatTakeoverRefusal({ ...lock, command: 'preview' }, 'holder-interactive') + expect(message).toContain('Another Nuxt preview server is already running') + expect(message).toContain('`--port` to run a second one alongside it') + expect(message).not.toContain('NUXT_IGNORE_LOCK') + }) + + it('names a preview server that would not exit', () => { + expect(formatTakeoverRefusal({ ...lock, command: 'preview' }, 'timeout')) + .toContain('The preview server on port 3000 did not exit') + }) + }) + + describe('preview servers', () => { + it('check every interface for a server that recorded no hostname', async () => { + writeLock(buildDir, { command: 'preview', hostname: undefined }) + mockProcess() + await takeOverServer(buildDir, { command: 'preview', interactive: false }) + expect(checkPort).toHaveBeenCalledWith(3000, undefined) + }) + + it('check only the recorded hostname for a server that bound one', async () => { + writeLock(buildDir, { command: 'preview', hostname: '127.0.0.1' }) + mockProcess() + checkPort.mockClear() + await takeOverServer(buildDir, { command: 'preview', interactive: false }) + expect(checkPort).toHaveBeenCalledWith(3000, '127.0.0.1') + expect(checkPort).not.toHaveBeenCalledWith(3000, undefined) + }) + + it('are only taken over by a preview', async () => { + writeLock(buildDir, { command: 'preview' }) + const proc = mockProcess() + expect(await takeOverServer(buildDir, { interactive: false })).toEqual({ action: 'none' }) + expect(proc.signals).toHaveLength(0) + }) + + it('never let a preview take over a dev server', async () => { + writeLock(buildDir, { command: 'dev' }) + const proc = mockProcess() + expect(await takeOverServer(buildDir, { command: 'preview', interactive: false })).toEqual({ action: 'none' }) + expect(proc.signals).toHaveLength(0) + }) + + it('follow the same decision matrix as dev servers', async () => { + writeLock(buildDir, { command: 'preview', interactive: true }) + mockProcess() + expect(await takeOverServer(buildDir, { command: 'preview', interactive: false })) + .toMatchObject({ action: 'refused', reason: 'holder-interactive' }) + + writeLock(buildDir, { command: 'preview', interactive: false }) + expect(await takeOverServer(buildDir, { command: 'preview', interactive: false })) + .toEqual({ action: 'taken', port: 3000, pid: HOLDER_PID }) + }) + + it('stop the server process a preview runs as well as the preview itself', async () => { + writeLock(buildDir, { command: 'preview', serverPid: 434343 }) + const proc = mockProcess() + await takeOverServer(buildDir, { command: 'preview', interactive: false }) + expect(proc.signals).toEqual([[HOLDER_PID, 'SIGTERM'], [434343, 'SIGTERM']]) + }) + + it('can be started alongside without a warning about the build directory', async () => { + writeLock(buildDir, { command: 'preview', interactive: true }) + mockProcess() + const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + expect(await takeOverServer(buildDir, { command: 'preview', interactive: true, prompt: async () => 'start-anyway' })) + .toMatchObject({ action: 'start-anyway' }) + expect(warn).not.toHaveBeenCalled() + }) + + it('are named when stale', async () => { + writeLock(buildDir, { command: 'preview' }) + mockProcess({ alive: false }) + const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {}) + expect(await takeOverServer(buildDir, { command: 'preview', interactive: false })).toEqual({ action: 'stale' }) + expect(warn).toHaveBeenCalledWith('The preview server that was using port 3000 is gone, but something is still listening there.') + }) }) describe('outgoing side', () => { diff --git a/packages/nuxt-cli/test/unit/utils/untrusted-lock.spec.ts b/packages/nuxt-cli/test/unit/utils/untrusted-lock.spec.ts index 23ec2658e..1217757e0 100644 --- a/packages/nuxt-cli/test/unit/utils/untrusted-lock.spec.ts +++ b/packages/nuxt-cli/test/unit/utils/untrusted-lock.spec.ts @@ -5,7 +5,7 @@ import process from 'node:process' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { takeOverDevServer } from '../../../src/dev/takeover' +import { takeOverServer } from '../../../src/dev/takeover' import { findDevServer, findNitroDevWorker } from '../../../src/utils/dev-server' import { parseLockInfo, readLock } from '../../../src/utils/lockfile' @@ -58,9 +58,14 @@ describe('parseLockInfo', () => { it('should drop a negative parent pid rather than the whole lock', () => { expect(parseLockInfo(baseLock({ parentPid: -1 }))?.parentPid).toBeUndefined() + expect(parseLockInfo(baseLock({ serverPid: -1 }))?.serverPid).toBeUndefined() expect(parseLockInfo(baseLock({ takenOverBy: -1 }))?.takenOverBy).toBeUndefined() }) + it('should accept a preview lock and the pid of the server it runs', () => { + expect(parseLockInfo(baseLock({ command: 'preview', serverPid: 4242 }))).toMatchObject({ command: 'preview', serverPid: 4242 }) + }) + it('should drop an out-of-range port', () => { expect(parseLockInfo(baseLock({ port: 0 }))?.port).toBeUndefined() expect(parseLockInfo(baseLock({ port: 70_000 }))?.port).toBeUndefined() @@ -100,7 +105,7 @@ describe('reading an untrusted lock', () => { it('should never signal a process group during takeover', async () => { const kill = vi.spyOn(process, 'kill') - const result = await takeOverDevServer( + const result = await takeOverServer( await writeLock(baseLock({ pid: -1, port: 3000, url: 'http://localhost:3000' })), { takeover: true }, ) From f959619ea0fb2bb1d6398a45e30f5722a1542be5 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 5 Oct 2026 13:02:26 +0000 Subject: [PATCH 2/2] fix: release preview locks and revalidate takeover targets --- docs/preview.md | 4 +- packages/nuxt-cli/src/commands/preview.ts | 56 ++++++++++------- packages/nuxt-cli/src/dev/takeover.ts | 26 +++++++- .../test/unit/commands/preview.spec.ts | 55 ++++++++++++++-- packages/nuxt-cli/test/unit/takeover.spec.ts | 63 ++++++++++++++++++- 5 files changed, 172 insertions(+), 32 deletions(-) diff --git a/docs/preview.md b/docs/preview.md index 83b1c3a28..0d5c56bb9 100644 --- a/docs/preview.md +++ b/docs/preview.md @@ -58,7 +58,9 @@ A preview records itself in `node_modules/.cache/nuxt/preview`, so a second `nux Pass `--takeover` to always stop it and start in its place, or `--no-takeover` to never do so. Choosing "Start anyway" at the prompt, or passing a different `--port`, runs a second preview alongside it instead. -When anything else is using the port (`3000` unless you pass `--port`), the preview moves to another free port as `nuxt dev` does, or exits if you pass `--strictPort`. This only applies to presets whose server runs directly with Node.js, Bun or Deno; other presets' preview commands choose their own port. +When another server, including `nuxt dev`, is using the requested port, the preview automatically uses another free port without stopping that server. Pass `--strictPort` to exit instead. The requested port comes from `--port`, `NUXT_PORT`, `NITRO_PORT`, or `PORT`, defaulting to `3000`. + +Port selection applies to static previews and preview commands that run directly with Node.js, Bun, or Deno. Other preview commands choose their own port. This command sets `process.env.NODE_ENV` to `production`. To override, define `NODE_ENV` in a `.env` file or as command-line argument. diff --git a/packages/nuxt-cli/src/commands/preview.ts b/packages/nuxt-cli/src/commands/preview.ts index 902c94aa7..d1b3a0214 100644 --- a/packages/nuxt-cli/src/commands/preview.ts +++ b/packages/nuxt-cli/src/commands/preview.ts @@ -277,36 +277,41 @@ const command = defineCommand({ outro(`Running ${styleText('cyan', previewCommand)} in ${styleText('cyan', relativeToProcess(previewDir))}`) - const recordServer = listenPort === undefined ? undefined : recordPreview(cwd, listenPort, host) - const server = x(command, commandArgs, { - throwOnError: true, - nodeOptions: { - stdio: 'inherit', - cwd: previewDir, - env: { - ...withPrependedPath(process.env, [ - resolve(previewDir, 'node_modules/.bin'), - resolve(cwd, 'node_modules/.bin'), - ]), - NUXT_PORT: serverPort, - NITRO_PORT: serverPort, - NUXT_HOST: host, - NITRO_HOST: host, + const previewLock = listenPort === undefined ? undefined : recordPreview(cwd, listenPort, host) + try { + const server = x(command, commandArgs, { + throwOnError: true, + nodeOptions: { + stdio: 'inherit', + cwd: previewDir, + env: { + ...withPrependedPath(process.env, [ + resolve(previewDir, 'node_modules/.bin'), + resolve(cwd, 'node_modules/.bin'), + ]), + NUXT_PORT: serverPort, + NITRO_PORT: serverPort, + NUXT_HOST: host, + NITRO_HOST: host, + }, }, - }, - }) - if (recordServer && server.pid) { - recordServer(server.pid) + }) + if (server.pid) { + previewLock?.update(server.pid) + } + await server + } + finally { + previewLock?.release() } - await server }, }) export default command -function recordPreview(rootDir: string, port: number, hostname: string | undefined): (serverPid: number) => void { +function recordPreview(rootDir: string, port: number, hostname: string | undefined): { update: (serverPid: number) => void, release: () => void } | undefined { if (port === 0) { - return () => {} + return } const lockDir = previewLockDir(rootDir) const host = hostname || 'localhost' @@ -319,7 +324,10 @@ function recordPreview(rootDir: string, port: number, hostname: string | undefin } const { release } = acquireLock(lockDir, info) if (!release) { - return () => {} + return + } + return { + release, + update: serverPid => updateLock(lockDir, { ...info, serverPid }), } - return serverPid => updateLock(lockDir, { ...info, serverPid }) } diff --git a/packages/nuxt-cli/src/dev/takeover.ts b/packages/nuxt-cli/src/dev/takeover.ts index 39ba74830..118739874 100644 --- a/packages/nuxt-cli/src/dev/takeover.ts +++ b/packages/nuxt-cli/src/dev/takeover.ts @@ -162,13 +162,24 @@ async function resolveTakeover(lockDir: string, options: TakeoverOptions): Promi async function performTakeover(lockDir: string, existing: LockInfo, timeouts: TakeoverOptions['timeouts'] = {}): Promise { const port = existing.port! + const portFree = await isPortFree(port, existing.hostname) + if (!matchesLock(readLock(lockDir), existing)) { + return { action: 'none' } + } + if (!isProcessAlive(existing.pid) || portFree) { + clearStaleLock(lockDir, existing) + return { action: 'stale' } + } + const label = describeServer(existing) const startedAt = new Date(existing.startedAt).toLocaleTimeString() const progress = startProgress(`Taking over the ${label} on port ${port} (PID ${existing.pid}, started ${startedAt})`) markTakenOver(lockDir, process.pid) - const pids = [...new Set([existing.pid, existing.parentPid, existing.serverPid])] + const pids = [...new Set(existing.command === 'preview' + ? [existing.serverPid, existing.pid] + : [existing.pid, existing.parentPid])] .filter((pid): pid is number => !!pid) // On Windows `SIGTERM` terminates outright. @@ -181,6 +192,11 @@ async function performTakeover(lockDir: string, existing: LockInfo, timeouts: Ta progress.update(`Waiting for the ${label} on port ${port} to exit`) } for (const pid of pids) { + const current = readLock(lockDir) + if (!matchesLock(current, existing) + || (pid !== existing.pid && pid !== current?.parentPid && pid !== current?.serverPid)) { + continue + } try { process.kill(pid, signal) } @@ -197,6 +213,14 @@ async function performTakeover(lockDir: string, existing: LockInfo, timeouts: Ta return { action: 'refused', existing, reason: 'timeout' } } +function matchesLock(current: LockInfo | undefined, existing: LockInfo): boolean { + return current?.pid === existing.pid + && current.startedAt === existing.startedAt + && current.command === existing.command + && current.port === existing.port + && current.hostname === existing.hostname +} + interface TakeoverProgress { update: (message: string) => void stop: (message: string) => void diff --git a/packages/nuxt-cli/test/unit/commands/preview.spec.ts b/packages/nuxt-cli/test/unit/commands/preview.spec.ts index 7f4940c7b..ff8bd0168 100644 --- a/packages/nuxt-cli/test/unit/commands/preview.spec.ts +++ b/packages/nuxt-cli/test/unit/commands/preview.spec.ts @@ -228,18 +228,65 @@ describe('preview', () => { }) it('records the server it starts so a later preview can take it over', async () => { - x.mockReturnValue(Object.assign(Promise.resolve({ exitCode: 0 }), { pid: 4242 })) + let recorded: LockInfo | undefined + x.mockReturnValue({ + pid: 4242, + then(resolve: (value: { exitCode: number }) => void) { + recorded = readLock(previewLockDir(cwd)) + resolve({ exitCode: 0 }) + }, + }) await writeNitroJSON(join(cwd, '.output')) await runCommand(preview, { rawArgs: [cwd, '--port=4321'] }) - expect(readLock(previewLockDir(cwd))).toMatchObject({ + expect(recorded).toMatchObject({ pid: process.pid, command: 'preview', port: 4321, url: 'http://localhost:4321', serverPid: 4242, }) + expect(readLock(previewLockDir(cwd))).toBeUndefined() + }) + + it.each(['spawn', 'exit'])('releases the lock after a child %s error', async (failure) => { + const error = new Error('preview failed') + if (failure === 'spawn') { + x.mockImplementation(() => { + throw error + }) + } + else { + x.mockRejectedValue(error) + } + await writeNitroJSON(join(cwd, '.output')) + const listeners = process.listenerCount('exit') + + await expect(runCommand(preview, { rawArgs: [cwd] })).rejects.toThrow(error) + + expect(readLock(previewLockDir(cwd))).toBeUndefined() + expect(process.listenerCount('exit')).toBe(listeners) + }) + + it('allocates a random port when port zero is requested', async () => { + getPort.mockResolvedValue(4321) + await writeNitroJSON(join(cwd, '.output')) + + await runCommand(preview, { rawArgs: [cwd, '--port=0'] }) + + expect(getPort).toHaveBeenCalledWith({ random: true, host: undefined }) + expectServerPort('4321') + }) + + it('rejects an invalid port before starting a server', async () => { + await writeNitroJSON(join(cwd, '.output')) + + await expect(runCommand(preview, { rawArgs: [cwd, '--port=invalid'] })) + .rejects + .toThrow('Invalid port') + + expect(x).not.toHaveBeenCalled() }) describe('with a preview of this project already running', () => { @@ -254,7 +301,7 @@ describe('preview', () => { await runCommand(preview, { rawArgs: [cwd] }) - expect(signals).toEqual([[HOLDER_PID, 'SIGTERM'], [SERVER_PID, 'SIGTERM']]) + expect(signals).toEqual([[SERVER_PID, 'SIGTERM'], [HOLDER_PID, 'SIGTERM']]) expectServerPort('4500') }) @@ -280,7 +327,7 @@ describe('preview', () => { await runCommand(preview, { rawArgs: [cwd, '--takeover'] }) - expect(signals).toEqual([[HOLDER_PID, 'SIGTERM'], [SERVER_PID, 'SIGTERM']]) + expect(signals).toEqual([[SERVER_PID, 'SIGTERM'], [HOLDER_PID, 'SIGTERM']]) expectServerPort('4500') }) diff --git a/packages/nuxt-cli/test/unit/takeover.spec.ts b/packages/nuxt-cli/test/unit/takeover.spec.ts index 53623de4f..7c924f85b 100644 --- a/packages/nuxt-cli/test/unit/takeover.spec.ts +++ b/packages/nuxt-cli/test/unit/takeover.spec.ts @@ -1,7 +1,7 @@ import type { TakeoverChoice } from '../../src/dev/takeover' import type { LockInfo } from '../../src/utils/lockfile' -import { mkdirSync, writeFileSync } from 'node:fs' +import { mkdirSync, unlinkSync, writeFileSync } from 'node:fs' import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -360,7 +360,66 @@ describe('takeOverServer', () => { writeLock(buildDir, { command: 'preview', serverPid: 434343 }) const proc = mockProcess() await takeOverServer(buildDir, { command: 'preview', interactive: false }) - expect(proc.signals).toEqual([[HOLDER_PID, 'SIGTERM'], [434343, 'SIGTERM']]) + expect(proc.signals).toEqual([[434343, 'SIGTERM'], [HOLDER_PID, 'SIGTERM']]) + }) + + it.each(['removed', 'replaced', 'free'])('does not signal a preview whose lock is %s while prompting', async (change) => { + writeLock(buildDir, { command: 'preview', serverPid: 434343 }) + const proc = mockProcess() + const result = await takeOverServer(buildDir, { + command: 'preview', + interactive: true, + prompt: async () => { + if (change === 'removed') { + unlinkSync(join(buildDir, 'nuxt.lock')) + } + else if (change === 'replaced') { + writeLock(buildDir, { command: 'preview', pid: 555555 }) + } + else { + checkPort.mockResolvedValue(3000) + } + return 'takeover' + }, + }) + expect(result.action).toBe(change === 'free' ? 'stale' : 'none') + expect(proc.signals).toHaveLength(0) + if (change === 'replaced') { + expect(readLock(buildDir)).toMatchObject({ pid: 555555 }) + expect(readLock(buildDir)?.takenOverBy).toBeUndefined() + } + }) + + it('does not signal a child removed from the lock while prompting', async () => { + const lock = writeLock(buildDir, { command: 'preview', serverPid: 434343 }) + const proc = mockProcess() + await takeOverServer(buildDir, { + command: 'preview', + interactive: true, + prompt: async () => { + writeLock(buildDir, { ...lock, serverPid: undefined }) + return 'takeover' + }, + }) + expect(proc.signals).toEqual([[HOLDER_PID, 'SIGTERM']]) + }) + + it('does not escalate against a preview whose lock was released', async () => { + writeLock(buildDir, { command: 'preview', serverPid: 434343 }) + const proc = mockProcess({ diesOn: 'never' }) + proc.kill.mockImplementation((pid, signal) => { + if (signal === 'SIGTERM') { + proc.signals.push([pid as number, signal]) + unlinkSync(join(buildDir, 'nuxt.lock')) + } + return true as never + }) + expect(await takeOverServer(buildDir, { + command: 'preview', + takeover: true, + timeouts: { graceful: 1, force: 1 }, + })).toMatchObject({ action: 'refused', reason: 'timeout' }) + expect(proc.signals).toEqual([[434343, 'SIGTERM']]) }) it('can be started alongside without a warning about the build directory', async () => {