diff --git a/src/task.ts b/src/task.ts index 0344a4c3fe..99290ae63d 100644 --- a/src/task.ts +++ b/src/task.ts @@ -13,6 +13,8 @@ export async function runTask( taskEvent: TaskEvent, opts?: TaskRunnerOptions ): Promise<{ result: unknown }> { + // No default timeout: the socket stays idle while the task runs, so any + // default would cap how long a task may take. Opt in via `opts.timeout`. const ctx = await _getTasksContext(opts); const result = await ctx.devFetch(`/_nitro/tasks/${taskEvent.name}`, { method: "POST", @@ -23,7 +25,9 @@ export async function runTask( /** @experimental */ export async function listTasks(opts?: TaskRunnerOptions) { - const ctx = await _getTasksContext(opts); + // Listing is metadata the dev server answers immediately, so a default is + // safe here and stops a stalled socket from hanging the caller forever. + const ctx = await _getTasksContext(opts, 30_000); const res = (await ctx.devFetch("/_nitro/tasks")) as { tasks: Record; }; @@ -34,9 +38,10 @@ export async function listTasks(opts?: TaskRunnerOptions) { const _devHint = `(is dev server running?)`; -async function _getTasksContext(opts?: TaskRunnerOptions) { +async function _getTasksContext(opts?: TaskRunnerOptions, defaultTimeout?: number) { const cwd = resolve(process.cwd(), opts?.cwd || "."); const buildDir = resolve(cwd, opts?.buildDir || "node_modules/.nitro"); + const timeout = opts?.timeout ?? defaultTimeout; const buildInfoPath = resolve(buildDir, "nitro.dev.json"); if (!existsSync(buildInfoPath)) { @@ -75,6 +80,10 @@ async function _getTasksContext(opts?: TaskRunnerOptions) { { socketPath, method: options?.method, + // Only set it when asked. Passing `timeout: undefined` still leaves + // the socket reporting the peer's own idle close as a `timeout` + // event, which would reject a request that was never capped. + ...(timeout ? { timeout } : {}), headers: { Accept: "application/json", "Content-Type": "application/json", @@ -95,6 +104,13 @@ async function _getTasksContext(opts?: TaskRunnerOptions) { } ); + if (timeout) { + request.on("timeout", () => { + // destroy with an error so the `error` handler rejects instead of + // leaving the promise pending forever on a stalled socket + request.destroy(new Error(`Request timed out after ${timeout}ms ${_devHint}`)); + }); + } request.on("error", (e) => reject(e)); if (options?.body) { diff --git a/src/types/runtime/task.ts b/src/types/runtime/task.ts index 2d997f746b..9b5cd34ba0 100644 --- a/src/types/runtime/task.ts +++ b/src/types/runtime/task.ts @@ -36,4 +36,12 @@ export interface Task { export interface TaskRunnerOptions { cwd?: string; buildDir?: string; + /** + * Socket inactivity timeout in milliseconds for requests to the dev server. + * + * `listTasks()` defaults to 30 seconds, since the dev server answers it + * immediately. `runTask()` has no default: the socket stays idle for as long + * as the task runs, so a default would cap the task's own duration. + */ + timeout?: number; } diff --git a/test/unit/task-runner.test.ts b/test/unit/task-runner.test.ts new file mode 100644 index 0000000000..41d3829c82 --- /dev/null +++ b/test/unit/task-runner.test.ts @@ -0,0 +1,77 @@ +import http from "node:http"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "pathe"; +import { afterAll, describe, expect, it, vi } from "vitest"; +import { listTasks, runTask } from "../../src/task.ts"; + +describe("task runner devFetch", () => { + const cleanups: Array<() => Promise | void> = []; + + afterAll(async () => { + for (const cleanup of cleanups) { + await cleanup(); + } + }); + + // A worker socket that accepts connections but never responds, like a + // stalled dev server whose pid is still alive. + async function stalledDevServer() { + const cwd = await mkdtemp(join(tmpdir(), "nitro-task-test-")); + cleanups.push(() => rm(cwd, { recursive: true, force: true })); + + const socketPath = join(cwd, "worker.sock"); + const server = http.createServer(() => {}); + cleanups.push(() => new Promise((resolve) => server.close(() => resolve()))); + await new Promise((resolve) => server.listen(socketPath, resolve)); + + const buildDir = join(cwd, "node_modules/.nitro"); + await mkdir(buildDir, { recursive: true }); + await writeFile( + join(buildDir, "nitro.dev.json"), + JSON.stringify({ + dev: { pid: process.pid, workerAddress: { socketPath } }, + }) + ); + + return cwd; + } + + // https://github.com/unjs/nitro/issues/4292 + it("rejects instead of hanging when the dev server socket stalls", async () => { + const cwd = await stalledDevServer(); + await expect(listTasks({ cwd, timeout: 200 })).rejects.toThrow(/timed out/i); + }, 5000); + + it("honours an opt-in timeout for runTask", async () => { + const cwd = await stalledDevServer(); + await expect(runTask({ name: "db:migrate" }, { cwd, timeout: 200 })).rejects.toThrow( + /timed out/i + ); + }, 5000); + + // The socket stays idle for as long as the task runs, so only listTasks may + // carry a default. Read the request options rather than waiting 30s for it. + it("defaults the timeout for listTasks but not for runTask", async () => { + const cwd = await stalledDevServer(); + const request = vi.spyOn(http, "request"); + + const requested = async (call: Promise) => { + call.catch(() => {}); + await vi.waitFor(() => expect(request).toHaveBeenCalled()); + const [, options] = request.mock.calls[0] as [unknown, { timeout?: number }]; + (request.mock.results[0].value as http.ClientRequest).destroy(); + request.mockClear(); + return options; + }; + + expect(await requested(listTasks({ cwd }))).toHaveProperty("timeout", 30_000); + expect(await requested(runTask({ name: "db:migrate" }, { cwd }))).not.toHaveProperty("timeout"); + expect(await requested(runTask({ name: "db:migrate" }, { cwd, timeout: 200 }))).toHaveProperty( + "timeout", + 200 + ); + + request.mockRestore(); + }, 5000); +});