diff --git a/packages/next/src/bin/next.ts b/packages/next/src/bin/next.ts index 061227d66f78e..b01debaebb4bc 100755 --- a/packages/next/src/bin/next.ts +++ b/packages/next/src/bin/next.ts @@ -108,12 +108,11 @@ if (process.env.NODE_ENV) { ;(process.env as any).NODE_ENV = process.env.NODE_ENV || defaultEnv ;(process.env as any).NEXT_RUNTIME = 'nodejs' -// Make sure commands gracefully respect termination signals (e.g. from Docker) -// Allow the graceful termination to be manually configurable -if (!process.env.NEXT_MANUAL_SIG_HANDLE && command !== 'dev') { +if (command === 'build') { process.on('SIGTERM', () => process.exit(0)) process.on('SIGINT', () => process.exit(0)) } + async function main() { const currentArgsSpec = commandArgs[command]() const validatedArgs = getValidatedArgs(currentArgsSpec, forwardedArgs) diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index a73962422f861..a114b48bb275e 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -2072,13 +2072,6 @@ const dir = path.join(__dirname) process.env.NODE_ENV = 'production' process.chdir(__dirname) -// Make sure commands gracefully respect termination signals (e.g. from Docker) -// Allow the graceful termination to be manually configurable -if (!process.env.NEXT_MANUAL_SIG_HANDLE) { - process.on('SIGTERM', () => process.exit(0)) - process.on('SIGINT', () => process.exit(0)) -} - const currentPort = parseInt(process.env.PORT, 10) || 3000 const hostname = process.env.HOSTNAME || '0.0.0.0' diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts index 23eb1a56af7bd..aba4e22d49c55 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -28,27 +28,31 @@ import type { SelfSignedCertificate } from '../lib/mkcert' import uploadTrace from '../trace/upload-trace' import { initialEnv } from '@next/env' import { fork } from 'child_process' +import type { ChildProcess } from 'child_process' import { getReservedPortExplanation, isPortIsReserved, } from '../lib/helpers/get-reserved-port' import os from 'os' +import { once } from 'node:events' let dir: string -let child: undefined | ReturnType +let child: undefined | ChildProcess let config: NextConfigComplete let isTurboSession = false let traceUploadUrl: string let sessionStopHandled = false let sessionStarted = Date.now() -const handleSessionStop = async (signal: string | null) => { - if (child) { - child.kill((signal as any) || 0) - } +const handleSessionStop = async (signal: NodeJS.Signals | number | null) => { + if (child?.pid) child.kill(signal ?? 0) if (sessionStopHandled) return sessionStopHandled = true + if (child?.pid && child.exitCode === null && child.signalCode === null) { + await once(child, 'exit').catch(() => {}) + } + try { const { eventCliSessionStopped } = require('../telemetry/events/session-stopped') as typeof import('../telemetry/events/session-stopped') @@ -107,8 +111,11 @@ const handleSessionStop = async (signal: string | null) => { process.exit(0) } -process.on('SIGINT', () => handleSessionStop('SIGINT')) -process.on('SIGTERM', () => handleSessionStop('SIGTERM')) +process.on('SIGINT', () => handleSessionStop('SIGKILL')) +process.on('SIGTERM', () => handleSessionStop('SIGKILL')) + +// exit event must be synchronous +process.on('exit', () => child?.kill('SIGKILL')) const nextDev: CliCommand = async (args) => { if (args['--help']) { @@ -335,16 +342,4 @@ const nextDev: CliCommand = async (args) => { await runDevServer(false) } -function cleanup() { - if (!child) { - return - } - - child.kill('SIGTERM') -} - -process.on('exit', cleanup) -process.on('SIGINT', cleanup) -process.on('SIGTERM', cleanup) - export { nextDev } diff --git a/packages/next/src/server/lib/start-server.ts b/packages/next/src/server/lib/start-server.ts index 480ed5c3f1863..cc82e116b839d 100644 --- a/packages/next/src/server/lib/start-server.ts +++ b/packages/next/src/server/lib/start-server.ts @@ -264,10 +264,9 @@ export async function startServer( }) try { - const cleanup = (code: number | null) => { + const cleanup = () => { debug('start-server process cleanup') - server.close() - process.exit(code ?? 0) + server.close(() => process.exit(0)) } const exception = (err: Error) => { if (isPostpone(err)) { @@ -279,11 +278,11 @@ export async function startServer( // This is the render worker, we keep the process alive console.error(err) } - process.on('exit', (code) => cleanup(code)) + // Make sure commands gracefully respect termination signals (e.g. from Docker) + // Allow the graceful termination to be manually configurable if (!process.env.NEXT_MANUAL_SIG_HANDLE) { - // callback value is signal string, exit with 0 - process.on('SIGINT', () => cleanup(0)) - process.on('SIGTERM', () => cleanup(0)) + process.on('SIGINT', cleanup) + process.on('SIGTERM', cleanup) } process.on('rejectionHandled', () => { // It is ok to await a Promise late in Next.js as it allows for better diff --git a/test/integration/telemetry/test/page-features.test.js b/test/integration/telemetry/test/page-features.test.js index 46b87827f0b78..5d3cad8c271d3 100644 --- a/test/integration/telemetry/test/page-features.test.js +++ b/test/integration/telemetry/test/page-features.test.js @@ -100,7 +100,7 @@ describe('page features telemetry', () => { await renderViaHTTP(port, '/hello') if (app) { - await killApp(app) + await killApp(app, 'SIGTERM') } await check(() => stderr, /NEXT_CLI_SESSION_STOPPED/) @@ -141,7 +141,7 @@ describe('page features telemetry', () => { await renderViaHTTP(port, '/hello') if (app) { - await killApp(app) + await killApp(app, 'SIGTERM') } await check(() => stderr, /NEXT_CLI_SESSION_STOPPED/) diff --git a/test/lib/next-modes/base.ts b/test/lib/next-modes/base.ts index 2ffd53e6e83ca..0f460c0b2491e 100644 --- a/test/lib/next-modes/base.ts +++ b/test/lib/next-modes/base.ts @@ -10,6 +10,7 @@ import { Span } from 'next/src/trace' import webdriver from '../next-webdriver' import { renderViaHTTP, fetchViaHTTP, waitFor } from 'next-test-utils' import cheerio from 'cheerio' +import { once } from 'events' import { BrowserInterface } from '../browsers/base' import escapeStringRegexp from 'escape-string-regexp' @@ -59,7 +60,7 @@ export class NextInstance { public testDir: string protected isStopping: boolean = false protected isDestroyed: boolean = false - protected childProcess: ChildProcess + protected childProcess?: ChildProcess protected _url: string protected _parsedUrl: URL protected packageJson?: PackageJson = {} @@ -331,13 +332,7 @@ export class NextInstance { public async stop(): Promise { this.isStopping = true if (this.childProcess) { - let exitResolve - const exitPromise = new Promise((resolve) => { - exitResolve = resolve - }) - this.childProcess.addListener('exit', () => { - exitResolve() - }) + const exitPromise = once(this.childProcess, 'exit') await new Promise((resolve) => { treeKill(this.childProcess.pid, 'SIGKILL', (err) => { if (err) { diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 5dc70b3b922ea..32460a90dbbb2 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -17,6 +17,7 @@ import { getRandomPort } from 'get-port-please' import fetch from 'node-fetch' import qs from 'querystring' import treeKill from 'tree-kill' +import { once } from 'events' import server from 'next/dist/server/next' import _pkg from 'next/package.json' @@ -497,7 +498,7 @@ export function buildTS( export async function killProcess( pid: number, - signal: string | number = 'SIGTERM' + signal: NodeJS.Signals | number = 'SIGTERM' ): Promise { return await new Promise((resolve, reject) => { treeKill(pid, signal, (err) => { @@ -524,9 +525,18 @@ export async function killProcess( } // Kill a launched app -export async function killApp(instance: ChildProcess) { - if (instance && instance.pid) { - await killProcess(instance.pid) +export async function killApp( + instance?: ChildProcess, + signal: NodeJS.Signals | number = 'SIGKILL' +) { + if ( + instance?.pid && + instance.exitCode === null && + instance.signalCode === null + ) { + const exitPromise = once(instance, 'exit') + await killProcess(instance.pid, signal) + await exitPromise } } diff --git a/test/production/graceful-shutdown/index.test.ts b/test/production/graceful-shutdown/index.test.ts new file mode 100644 index 0000000000000..1af4758170b78 --- /dev/null +++ b/test/production/graceful-shutdown/index.test.ts @@ -0,0 +1,236 @@ +import { join } from 'path' +import { NextInstance, createNext, FileRef } from 'e2e-utils' +import { + fetchViaHTTP, + findPort, + initNextServerScript, + killApp, + launchApp, + nextBuild, + nextStart, + waitFor, +} from 'next-test-utils' +import fs from 'fs-extra' +import glob from 'glob' +import { LONG_RUNNING_MS } from './src/pages/api/long-running' +import { once } from 'events' + +const appDir = join(__dirname, './src') +let appPort +let app + +function assertDefined(value: T | void): asserts value is T { + expect(value).toBeDefined() +} + +describe('Graceful Shutdown', () => { + describe('development (next dev)', () => { + beforeEach(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort) + }) + afterEach(() => killApp(app)) + + runTests(true) + }) + ;(process.env.TURBOPACK ? describe.skip : describe)( + 'production (next start)', + () => { + beforeAll(async () => { + await nextBuild(appDir) + }) + beforeEach(async () => { + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterEach(() => killApp(app)) + + runTests() + } + ) + ;(process.env.TURBOPACK ? describe.skip : describe)( + 'production (standalone mode)', + () => { + let next: NextInstance + let serverFile + + const projectFiles = { + 'next.config.mjs': `export default { output: 'standalone' }`, + } + + for (const file of glob.sync('*', { cwd: appDir, dot: false })) { + projectFiles[file] = new FileRef(join(appDir, file)) + } + + beforeAll(async () => { + next = await createNext({ + files: projectFiles, + dependencies: { + swr: 'latest', + }, + }) + + await next.stop() + + await fs.move( + join(next.testDir, '.next/standalone'), + join(next.testDir, 'standalone') + ) + + for (const file of await fs.readdir(next.testDir)) { + if (file !== 'standalone') { + await fs.remove(join(next.testDir, file)) + } + } + const files = glob.sync('**/*', { + cwd: join(next.testDir, 'standalone/.next/server/pages'), + dot: true, + }) + + for (const file of files) { + if (file.endsWith('.json') || file.endsWith('.html')) { + await fs.remove(join(next.testDir, '.next/server', file)) + } + } + + serverFile = join(next.testDir, 'standalone/server.js') + }) + + beforeEach(async () => { + appPort = await findPort() + app = await initNextServerScript( + serverFile, + /- Local:/, + { ...process.env, PORT: appPort.toString() }, + undefined, + { cwd: next.testDir } + ) + }) + afterEach(() => killApp(app)) + + afterAll(() => next.destroy()) + + runTests() + } + ) +}) + +function runTests(dev = false) { + if (dev) { + it('should shut down child immediately', async () => { + const appKilledPromise = once(app, 'exit') + + // let the dev server compile the route before running the test + await expect( + fetchViaHTTP(appPort, '/api/long-running') + ).resolves.toBeDefined() + + const resPromise = fetchViaHTTP(appPort, '/api/long-running') + + // yield event loop to kick off request before killing the app + await waitFor(20) + process.kill(app.pid, 'SIGTERM') + expect(app.exitCode).toBe(null) + + // `next dev` should kill the child immediately + let start = Date.now() + await expect(resPromise).rejects.toThrow() + expect(Date.now() - start).toBeLessThan(LONG_RUNNING_MS) + + // `next dev` parent process is still running cleanup + expect(app.exitCode).toBe(null) + + // App finally shuts down + await appKilledPromise + expect(app.exitCode).toBe(0) + }) + } else { + it('should wait for requests to complete before exiting', async () => { + const appKilledPromise = once(app, 'exit') + + let responseResolved = false + const resPromise = fetchViaHTTP(appPort, '/api/long-running') + .then((res) => { + responseResolved = true + return res + }) + .catch(() => {}) + + // yield event loop to kick off request before killing the app + await waitFor(20) + process.kill(app.pid, 'SIGTERM') + expect(app.exitCode).toBe(null) + + // Long running response should still be running after a bit + await waitFor(LONG_RUNNING_MS / 2) + expect(app.exitCode).toBe(null) + expect(responseResolved).toBe(false) + + // App responds as expected without being interrupted + const res = await resPromise + assertDefined(res) + expect(res.status).toBe(200) + expect(await res.json()).toStrictEqual({ hello: 'world' }) + + // App is still running briefly after response returns + expect(app.exitCode).toBe(null) + expect(responseResolved).toBe(true) + + // App finally shuts down + await appKilledPromise + expect(app.exitCode).toBe(0) + }) + + describe('should not accept new requests during shutdown cleanup', () => { + it('when request is made before shutdown', async () => { + const appKilledPromise = once(app, 'exit') + + const resPromise = fetchViaHTTP(appPort, '/api/long-running') + + // yield event loop to kick off request before killing the app + await waitFor(20) + process.kill(app.pid, 'SIGTERM') + expect(app.exitCode).toBe(null) + + // Long running response should still be running after a bit + await waitFor(LONG_RUNNING_MS / 2) + expect(app.exitCode).toBe(null) + + // Second request should be rejected + await expect( + fetchViaHTTP(appPort, '/api/long-running') + ).rejects.toThrow() + + // Original request responds as expected without being interrupted + await expect(resPromise).resolves.toBeDefined() + const res = await resPromise + expect(res.status).toBe(200) + expect(await res.json()).toStrictEqual({ hello: 'world' }) + + // App is still running briefly after response returns + expect(app.exitCode).toBe(null) + + // App finally shuts down + await appKilledPromise + expect(app.exitCode).toBe(0) + }) + + it('when there is no activity', async () => { + const appKilledPromise = once(app, 'exit') + + process.kill(app.pid, 'SIGTERM') + expect(app.exitCode).toBe(null) + + // yield event loop to allow server to start the shutdown process + await waitFor(20) + await expect( + fetchViaHTTP(appPort, '/api/long-running') + ).rejects.toThrow() + + // App finally shuts down + await appKilledPromise + expect(app.exitCode).toBe(0) + }) + }) + } +} diff --git a/test/production/graceful-shutdown/src/pages/api/long-running.ts b/test/production/graceful-shutdown/src/pages/api/long-running.ts new file mode 100644 index 0000000000000..749713e990036 --- /dev/null +++ b/test/production/graceful-shutdown/src/pages/api/long-running.ts @@ -0,0 +1,6 @@ +export const LONG_RUNNING_MS = 400 + +export default async (req, res) => { + await new Promise((resolve) => setTimeout(resolve, LONG_RUNNING_MS)) + res.json({ hello: 'world' }) +} diff --git a/test/production/graceful-shutdown/src/tsconfig.json b/test/production/graceful-shutdown/src/tsconfig.json new file mode 100644 index 0000000000000..1c4f693a991e7 --- /dev/null +++ b/test/production/graceful-shutdown/src/tsconfig.json @@ -0,0 +1,18 @@ +{ + "compilerOptions": { + "lib": ["dom", "dom.iterable", "esnext"], + "allowJs": true, + "skipLibCheck": true, + "strict": false, + "noEmit": true, + "incremental": true, + "esModuleInterop": true, + "module": "esnext", + "moduleResolution": "node", + "resolveJsonModule": true, + "isolatedModules": true, + "jsx": "preserve" + }, + "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"], + "exclude": ["node_modules"] +}