fix(cli): preview --background reports a preview that cannot start instead of waiting it out (#5437)

* fix(cli): preview --background reports a child that exits instead of waiting it out

* fix(cli): a preview that cannot listen exits so its launcher reports why

* fix(cli): keep preview's comment share within budget

* test(cli): the start failure test uses a project path outside the temp dir
This commit is contained in:
Miguel Ángel
2026-10-10 22:17:47 +00:00
committed by GitHub
parent a9823f719d
commit 6c96ea5344
4 changed files with 114 additions and 7 deletions
@@ -0,0 +1,41 @@
import { describe, expect, it, vi } from "vitest";
import { runEmbeddedMode } from "./preview.js";
const ui = vi.hoisted(() => ({
spinner: { start: vi.fn(), stop: vi.fn(), message: vi.fn() },
error: vi.fn(),
}));
vi.mock("@clack/prompts", async (importOriginal) => {
const actual = await importOriginal<typeof import("@clack/prompts")>();
return {
...actual,
spinner: () => ui.spinner,
intro: () => {},
log: { ...actual.log, error: ui.error },
};
});
vi.mock("../server/studioServer.js", () => ({
resolveStudioBundle: () => ({ available: true, checkedPaths: [] }),
loadPreviewServerBuildSignature: async () => "test",
createStudioServer: () => ({
app: { fetch: () => new Response() },
watcher: { close: vi.fn() },
shutdown: vi.fn(),
}),
}));
vi.mock("../server/portUtils.js", async (importOriginal) => ({
...(await importOriginal<typeof import("../server/portUtils.js")>()),
findPortAndServe: async () => {
throw new Error("listen EPERM: operation not permitted 127.0.0.1:3056");
},
}));
describe("preview start failure", () => {
it("stops the startup spinner so a server that cannot listen lets the process exit", async () => {
await runEmbeddedMode("start-failure-project", 3056, { noOpen: true });
expect(ui.spinner.start).toHaveBeenCalledOnce();
expect(ui.spinner.stop).toHaveBeenCalledOnce();
expect(ui.error).toHaveBeenCalledWith(expect.stringContaining("listen EPERM"));
});
});
+2 -1
View File
@@ -1542,7 +1542,7 @@ export async function runLocalStudioMode(
* If an existing HyperFrames server for the same project is detected,
* reuses it instead of starting a new one (unless --force-new is set).
*/
async function runEmbeddedMode(
export async function runEmbeddedMode(
dir: string,
startPort: number,
options?: EmbeddedStudioOptions,
@@ -1601,6 +1601,7 @@ async function runEmbeddedMode(
);
} catch (err: unknown) {
watcher.close();
if (!options?.json) s.stop(c.error("Could not start studio"));
reportPreviewFailure(
Boolean(options?.json),
"start",
@@ -1,4 +1,4 @@
import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync } from "node:fs";
import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeSync } from "node:fs";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { describe, expect, it, onTestFinished, vi } from "vitest";
@@ -682,6 +682,32 @@ describe("background preview lifecycle", () => {
expect(result).toMatchObject({ type: "started", port: 3002, pid: 5432 });
});
it("stops waiting when the detached child exits and reports its exit and log", async () => {
const stateHome = tempDir("hf-preview-state-");
const kill = vi.fn();
const scan = vi.fn(async () => []);
let onExit: ((code: number | null, signal: string | null) => void) | undefined;
await expect(
startBackgroundPreview(projectDir, 3002, {
scan,
spawn: (_command, _args, options) => {
writeSync(
options.stdio[1],
"\u001b[?25hError: listen EPERM: operation not permitted 127.0.0.1:3002\n",
);
return { pid: 4321, unref: vi.fn(), once: (_event, listener) => (onExit = listener) };
},
sleep: async () => onExit?.(1, null),
kill,
stateHome,
}),
).rejects.toThrow(/exited \(exit code 1\) before it was ready[\s\S]*\nError: listen EPERM/);
expect(kill).not.toHaveBeenCalled();
expect(scan.mock.calls.length).toBeLessThan(5);
});
it("reaps a detached child that never becomes reachable without recording ownership", async () => {
const stateHome = tempDir("hf-preview-state-");
const kill = vi.fn();
+44 -5
View File
@@ -13,6 +13,7 @@ import {
} from "node:fs";
import { homedir } from "node:os";
import { dirname, join, resolve } from "node:path";
import { stripVTControlCharacters } from "node:util";
import {
activeServerOnPort,
MAX_PORT_SCAN,
@@ -46,7 +47,11 @@ export interface PreviewSession {
logPath: string;
}
type SpawnResult = { pid?: number; unref(): void };
type SpawnResult = {
pid?: number;
unref(): void;
once?(event: "exit", listener: (code: number | null, signal: string | null) => void): unknown;
};
type SpawnPreview = (
command: string,
args: string[],
@@ -226,7 +231,12 @@ function spawnDetachedPreview(
projectDir: string,
stateHome: string,
dependencies: LifecycleDependencies,
): { pid: number; wrapperIdentity: string | undefined; logPath: string } {
): {
pid: number;
wrapperIdentity: string | undefined;
logPath: string;
exitedWith: () => string | null;
} {
const logPath = previewLogPath(projectDir, stateHome);
mkdirSync(dirname(logPath), { recursive: true });
const logFd = openSync(logPath, "a", 0o600);
@@ -246,11 +256,16 @@ function spawnDetachedPreview(
closeSync(logFd);
}
if (!child.pid) throw new Error("background preview child did not report a PID");
let exit: string | null = null;
child.once?.("exit", (code, signal) => {
exit = signal ? `signal ${signal}` : `exit code ${code}`;
});
child.unref();
return {
pid: child.pid,
wrapperIdentity: (dependencies.identity ?? processIdentity)(child.pid) ?? undefined,
logPath,
exitedWith: () => exit,
};
}
@@ -547,15 +562,28 @@ export async function startBackgroundPreview(
// server, never a pre-existing unmanaged sibling.
const preLaunchPorts = sameProjectPorts(await scan(startPort), projectDir);
const { pid, wrapperIdentity, logPath } = spawnDetachedPreview(
const { pid, wrapperIdentity, logPath, exitedWith } = spawnDetachedPreview(
projectDir,
stateHome,
dependencies,
);
const kill = dependencies.kill ?? stopProcess;
const server = await awaitStartedServer(projectDir, startPort, preLaunchPorts, pid, dependencies);
const server = await awaitStartedServer(
projectDir,
startPort,
preLaunchPorts,
pid,
exitedWith,
dependencies,
);
if (!server) {
const exit = exitedWith();
if (exit) {
throw new Error(
`background preview exited (${exit}) before it was ready; see ${logPath}${logTail(logPath)}`,
);
}
await kill(pid);
throw new Error(`background preview did not become ready; see ${logPath}`);
}
@@ -587,16 +615,27 @@ export async function startBackgroundPreview(
};
}
function logTail(logPath: string): string {
try {
const text = stripVTControlCharacters(readFileSync(logPath, "utf8"));
const lines = text.trimEnd().split("\n").slice(-5);
return lines.join("").trim() ? `\n${lines.join("\n")}` : "";
} catch {
return "";
}
}
async function awaitStartedServer(
projectDir: string,
startPort: number,
preLaunchPorts: Set<number>,
launchedPid: number,
exitedWith: () => string | null,
dependencies: LifecycleDependencies,
): Promise<ActiveServer | null> {
const scan = dependencies.scan ?? scanActiveServers;
const sleep = dependencies.sleep ?? delay;
for (let attempt = 0; attempt < 50; attempt++) {
for (let attempt = 0; attempt < 50 && !exitedWith(); attempt++) {
const server = startedServer(
await scan(startPort),
projectDir,