From 5b004f2021bae51ad653508ecfd77931e0a83eef Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Tue, 26 Dec 2023 22:22:17 -0800 Subject: [PATCH 01/15] fix: gracefully shutdown server This reverts commit 771705f961b1132455bed9103eebe9aeaefebaaa, which reverted 2c48b8796b8d963f8b90d40b9b54ef73dd911ea1 because it was breaking some test cases. Coming back to this as a starting point to try to fix the tests and add new tests for the graceful shutdown feature. --- packages/next/src/cli/next-dev.ts | 28 ++++++++------------ packages/next/src/server/lib/start-server.ts | 13 +++++---- test/lib/next-test-utils.ts | 2 +- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts index 02e920d6971cc..07e2c7a489c4a 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -34,17 +34,20 @@ import { } from '../lib/helpers/get-reserved-port' import os from 'os' +type Child = ReturnType +type ExitCode = Parameters[0] + let dir: string -let child: undefined | ReturnType +let child: undefined | Child let config: NextConfigComplete let isTurboSession = false let traceUploadUrl: string let sessionStopHandled = false let sessionStarted = Date.now() -const handleSessionStop = async (signal: string | null) => { +const handleSessionStop = async (signal: ExitCode | null) => { if (child) { - child.kill((signal as any) || 0) + child.kill(signal ?? 0) } if (sessionStopHandled) return sessionStopHandled = true @@ -108,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']) { @@ -333,16 +339,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 84610325b0632..59ef2362d3972 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/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index f764949059706..234a2d093b8e6 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -526,7 +526,7 @@ export async function killProcess( // Kill a launched app export async function killApp(instance: ChildProcess) { if (instance && instance.pid) { - await killProcess(instance.pid) + await killProcess(instance.pid, 'SIGKILL') } } From 57f880ae2d8583c1e408b50d8e7301f7464330b5 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Wed, 27 Dec 2023 23:57:11 -0800 Subject: [PATCH 02/15] fix: better support for graceful shutdown - remove immediate process.exit from `next start` - have handleSessionStop wait for the child to exit - add a test that verifies long-running api routes are not killed - send proper signal (e.g. SIGTERM or SIGINT) to child process from dev server - wait for app to shut down in tests, with a short default timeout --- packages/next/src/bin/next.ts | 6 --- packages/next/src/cli/next-dev.ts | 11 +++-- .../api-support/pages/api/long-running.js | 6 +++ .../api-support/test/index.test.js | 42 +++++++++++++++++++ test/lib/next-test-utils.ts | 34 ++++++++++++++- 5 files changed, 88 insertions(+), 11 deletions(-) create mode 100644 test/integration/api-support/pages/api/long-running.js diff --git a/packages/next/src/bin/next.ts b/packages/next/src/bin/next.ts index 061227d66f78e..de7064e7c060d 100755 --- a/packages/next/src/bin/next.ts +++ b/packages/next/src/bin/next.ts @@ -108,12 +108,6 @@ 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') { - 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/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts index 07e2c7a489c4a..aebc451bf2574 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -33,6 +33,7 @@ import { isPortIsReserved, } from '../lib/helpers/get-reserved-port' import os from 'os' +import { once } from 'node:events' type Child = ReturnType type ExitCode = Parameters[0] @@ -52,6 +53,10 @@ const handleSessionStop = async (signal: ExitCode | null) => { if (sessionStopHandled) return sessionStopHandled = true + if (child) { + await once(child, 'exit').catch(() => {}) + } + try { const { eventCliSessionStopped } = require('../telemetry/events/session-stopped') as typeof import('../telemetry/events/session-stopped') @@ -111,11 +116,11 @@ const handleSessionStop = async (signal: ExitCode | null) => { process.exit(0) } -process.on('SIGINT', () => handleSessionStop('SIGKILL')) -process.on('SIGTERM', () => handleSessionStop('SIGKILL')) +process.on('SIGINT', (signal) => handleSessionStop(signal)) +process.on('SIGTERM', (signal) => handleSessionStop(signal)) // exit event must be synchronous -process.on('exit', () => child?.kill('SIGKILL')) +process.on('exit', (code) => child?.kill(code)) const nextDev: CliCommand = async (args) => { if (args['--help']) { diff --git a/test/integration/api-support/pages/api/long-running.js b/test/integration/api-support/pages/api/long-running.js new file mode 100644 index 0000000000000..4187b207185c4 --- /dev/null +++ b/test/integration/api-support/pages/api/long-running.js @@ -0,0 +1,6 @@ +export const LONG_RUNNING_MS = 100 + +export default async (req, res) => { + await new Promise((resolve) => setTimeout(resolve, LONG_RUNNING_MS)) + res.json({ hello: 'world' }) +} diff --git a/test/integration/api-support/test/index.test.js b/test/integration/api-support/test/index.test.js index fb27d6103f1b1..d4264ca765de9 100644 --- a/test/integration/api-support/test/index.test.js +++ b/test/integration/api-support/test/index.test.js @@ -15,8 +15,11 @@ import { getPageFileFromBuildManifest, getPageFileFromPagesManifest, check, + isAppRunning, + waitFor, } from 'next-test-utils' import json from '../big.json' +import { LONG_RUNNING_MS } from '../pages/api/long-running' const appDir = join(__dirname, '../') let appPort @@ -536,6 +539,45 @@ function runTests(dev = false) { }, 'success') }) + it('should wait for requests to complete before exiting', async () => { + // let the dev server compile the route before running the test + if (dev) await fetchViaHTTP(appPort, '/api/long-running') + + let responseResolved = false + let killAppResolved = false + const resPromise = fetchViaHTTP(appPort, '/api/long-running') + .then((res) => { + responseResolved = true + return res + }) + .catch(() => {}) + const killAppPromise = killApp(app, LONG_RUNNING_MS * 2).then(() => { + killAppResolved = true + }) + expect(isAppRunning(app)).toBe(true) + + // Long running response should still be running after a bit + await waitFor(LONG_RUNNING_MS / 2) + expect(isAppRunning(app)).toBe(true) + expect(responseResolved).toBe(false) + expect(killAppResolved).toBe(false) + + // App responds as expected without being interrupted + 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(isAppRunning(app)).toBe(true) + expect(responseResolved).toBe(true) + expect(killAppResolved).toBe(false) + + // App finally shuts down + await killAppPromise + expect(isAppRunning(app)).toBe(false) + expect(killAppResolved).toBe(true) + }) + if (dev) { it('should compile only server code in development', async () => { await fetchViaHTTP(appPort, '/api/users') diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 234a2d093b8e6..139ab79b12740 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -523,10 +523,40 @@ export async function killProcess( }) } +export function isAppRunning(instance: ChildProcess) { + if (!instance?.pid) return false + + try { + // 0 is a special signal that tests the existence of a process + process.kill(instance.pid, 0) + return true + } catch { + return false + } +} + +async function waitForCondition( + condition: () => boolean | Promise, + maxWait: number, + pollInterval: number +) { + const start = Date.now() + while (Date.now() - start < maxWait) { + if (await condition()) { + return + } + await waitFor(pollInterval) + } + throw new Error(`Timed out after ${maxWait}ms`) +} + // Kill a launched app -export async function killApp(instance: ChildProcess) { +export async function killApp(instance: ChildProcess, maxWait: number = 100) { if (instance && instance.pid) { - await killProcess(instance.pid, 'SIGKILL') + // waits for the signal to be sent, but not for the process to exit + await killProcess(instance.pid) + // poll frequently to see if the process has exited + await waitForCondition(() => !isAppRunning(instance), maxWait, maxWait / 10) } } From e3b0427a4e74599af4e067d948c18f26ddfd5f13 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Thu, 28 Dec 2023 10:25:18 -0800 Subject: [PATCH 03/15] fix: shutdown next dev immediately - when a SIGINT or SIGTERM is sent to next dev, shutdown the server immediately - create new test app for graceful-shutdown features - add some more tests --- packages/next/src/cli/next-dev.ts | 6 +- .../api-support/test/index.test.js | 42 ----- .../pages/api/long-running.js | 0 .../graceful-shutdown/test/index.test.js | 144 ++++++++++++++++++ test/lib/next-test-utils.ts | 4 +- 5 files changed, 149 insertions(+), 47 deletions(-) rename test/integration/{api-support => graceful-shutdown}/pages/api/long-running.js (100%) create mode 100644 test/integration/graceful-shutdown/test/index.test.js diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts index aebc451bf2574..2eb26aa174c6b 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -116,11 +116,11 @@ const handleSessionStop = async (signal: ExitCode | null) => { process.exit(0) } -process.on('SIGINT', (signal) => handleSessionStop(signal)) -process.on('SIGTERM', (signal) => handleSessionStop(signal)) +process.on('SIGINT', () => handleSessionStop('SIGKILL')) +process.on('SIGTERM', () => handleSessionStop('SIGKILL')) // exit event must be synchronous -process.on('exit', (code) => child?.kill(code)) +process.on('exit', () => child?.kill('SIGKILL')) const nextDev: CliCommand = async (args) => { if (args['--help']) { diff --git a/test/integration/api-support/test/index.test.js b/test/integration/api-support/test/index.test.js index d4264ca765de9..fb27d6103f1b1 100644 --- a/test/integration/api-support/test/index.test.js +++ b/test/integration/api-support/test/index.test.js @@ -15,11 +15,8 @@ import { getPageFileFromBuildManifest, getPageFileFromPagesManifest, check, - isAppRunning, - waitFor, } from 'next-test-utils' import json from '../big.json' -import { LONG_RUNNING_MS } from '../pages/api/long-running' const appDir = join(__dirname, '../') let appPort @@ -539,45 +536,6 @@ function runTests(dev = false) { }, 'success') }) - it('should wait for requests to complete before exiting', async () => { - // let the dev server compile the route before running the test - if (dev) await fetchViaHTTP(appPort, '/api/long-running') - - let responseResolved = false - let killAppResolved = false - const resPromise = fetchViaHTTP(appPort, '/api/long-running') - .then((res) => { - responseResolved = true - return res - }) - .catch(() => {}) - const killAppPromise = killApp(app, LONG_RUNNING_MS * 2).then(() => { - killAppResolved = true - }) - expect(isAppRunning(app)).toBe(true) - - // Long running response should still be running after a bit - await waitFor(LONG_RUNNING_MS / 2) - expect(isAppRunning(app)).toBe(true) - expect(responseResolved).toBe(false) - expect(killAppResolved).toBe(false) - - // App responds as expected without being interrupted - 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(isAppRunning(app)).toBe(true) - expect(responseResolved).toBe(true) - expect(killAppResolved).toBe(false) - - // App finally shuts down - await killAppPromise - expect(isAppRunning(app)).toBe(false) - expect(killAppResolved).toBe(true) - }) - if (dev) { it('should compile only server code in development', async () => { await fetchViaHTTP(appPort, '/api/users') diff --git a/test/integration/api-support/pages/api/long-running.js b/test/integration/graceful-shutdown/pages/api/long-running.js similarity index 100% rename from test/integration/api-support/pages/api/long-running.js rename to test/integration/graceful-shutdown/pages/api/long-running.js diff --git a/test/integration/graceful-shutdown/test/index.test.js b/test/integration/graceful-shutdown/test/index.test.js new file mode 100644 index 0000000000000..6df6519d568fc --- /dev/null +++ b/test/integration/graceful-shutdown/test/index.test.js @@ -0,0 +1,144 @@ +/* eslint-env jest */ + +import { join } from 'path' +import { + killApp, + findPort, + launchApp, + fetchViaHTTP, + nextBuild, + nextStart, + isAppRunning, + waitFor, +} from 'next-test-utils' +import { LONG_RUNNING_MS } from '../pages/api/long-running' + +const appDir = join(__dirname, '../') +let appPort +let app + +function runTests(dev = false) { + if (dev) { + it('should shut down child immediately', async () => { + // let the dev server compile the route before running the test + await expect( + fetchViaHTTP(appPort, '/api/long-running') + ).resolves.toBeTruthy() + + let responseResolved = false + let killAppResolved = false + const resPromise = fetchViaHTTP(appPort, '/api/long-running').then( + (res) => { + responseResolved = true + return res + } + ) + const killAppPromise = killApp(app, LONG_RUNNING_MS * 2).then(() => { + killAppResolved = true + }) + expect(isAppRunning(app)).toBe(true) + + // `next dev` should kill the child immediately + let start = Date.now() + await expect(resPromise).rejects.toThrow(/socket hang up/) + let end = Date.now() + expect(end - start).toBeLessThan(LONG_RUNNING_MS) + expect(responseResolved).toBe(false) + + // `next dev` parent process is still running cleanup + expect(isAppRunning(app)).toBe(true) + expect(killAppResolved).toBe(false) + + // App finally shuts down + await killAppPromise + expect(isAppRunning(app)).toBe(false) + expect(killAppResolved).toBe(true) + }) + } else { + it('should wait for requests to complete before exiting', async () => { + let responseResolved = false + let killAppResolved = false + const resPromise = fetchViaHTTP(appPort, '/api/long-running') + .then((res) => { + responseResolved = true + return res + }) + .catch(() => {}) + const killAppPromise = killApp(app, LONG_RUNNING_MS * 2).then(() => { + killAppResolved = true + }) + expect(isAppRunning(app)).toBe(true) + + // Long running response should still be running after a bit + await waitFor(LONG_RUNNING_MS / 2) + expect(isAppRunning(app)).toBe(true) + expect(responseResolved).toBe(false) + expect(killAppResolved).toBe(false) + + // App responds as expected without being interrupted + 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(isAppRunning(app)).toBe(true) + expect(responseResolved).toBe(true) + expect(killAppResolved).toBe(false) + + // App finally shuts down + await killAppPromise + expect(isAppRunning(app)).toBe(false) + expect(killAppResolved).toBe(true) + }) + + it('should not accept new requests during shutdown cleanup', async () => { + const resPromise = fetchViaHTTP(appPort, '/api/long-running') + const killAppPromise = killApp(app, LONG_RUNNING_MS * 2) + expect(isAppRunning(app)).toBe(true) + + // Long running response should still be running after a bit + await waitFor(LONG_RUNNING_MS / 2) + expect(isAppRunning(app)).toBe(true) + + // 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.toBeTruthy() + 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(isAppRunning(app)).toBe(true) + + // App finally shuts down + await killAppPromise + expect(isAppRunning(app)).toBe(false) + }) + } +} + +describe('API routes', () => { + describe('dev support', () => { + beforeEach(async () => { + appPort = await findPort() + app = await launchApp(appDir, appPort) + }) + afterEach(() => killApp(app)) + + runTests(true) + }) + ;(process.env.TURBOPACK ? describe.skip : describe)('production mode', () => { + beforeAll(async () => { + await nextBuild(appDir) + }) + beforeEach(async () => { + appPort = await findPort() + app = await nextStart(appDir, appPort) + }) + afterEach(() => killApp(app)) + + runTests() + }) +}) diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 139ab79b12740..274448fdd50a0 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -551,12 +551,12 @@ async function waitForCondition( } // Kill a launched app -export async function killApp(instance: ChildProcess, maxWait: number = 100) { +export async function killApp(instance: ChildProcess, maxWait: number = 500) { if (instance && instance.pid) { // waits for the signal to be sent, but not for the process to exit await killProcess(instance.pid) // poll frequently to see if the process has exited - await waitForCondition(() => !isAppRunning(instance), maxWait, maxWait / 10) + await waitForCondition(() => !isAppRunning(instance), maxWait, maxWait / 20) } } From 7d94719260979f38a1b69394f359467684807bf5 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Fri, 29 Dec 2023 13:08:12 -0800 Subject: [PATCH 04/15] tests: add another test for graceful shutdown - shouldn't accept requests while server is shutting down and there's no activity - refactor some tests to use process.kill since it's faster and we aren't looking to kill the whole tree. makes killApp simpler too --- .../graceful-shutdown/test/index.test.js | 107 ++++++++++-------- test/lib/next-test-utils.ts | 9 +- 2 files changed, 65 insertions(+), 51 deletions(-) diff --git a/test/integration/graceful-shutdown/test/index.test.js b/test/integration/graceful-shutdown/test/index.test.js index 6df6519d568fc..b1c002a0a4922 100644 --- a/test/integration/graceful-shutdown/test/index.test.js +++ b/test/integration/graceful-shutdown/test/index.test.js @@ -2,14 +2,15 @@ import { join } from 'path' import { - killApp, + fetchViaHTTP, findPort, + isAppRunning, + killApp, launchApp, - fetchViaHTTP, nextBuild, nextStart, - isAppRunning, waitFor, + waitForAppShutdown, } from 'next-test-utils' import { LONG_RUNNING_MS } from '../pages/api/long-running' @@ -25,55 +26,44 @@ function runTests(dev = false) { fetchViaHTTP(appPort, '/api/long-running') ).resolves.toBeTruthy() - let responseResolved = false - let killAppResolved = false - const resPromise = fetchViaHTTP(appPort, '/api/long-running').then( - (res) => { - responseResolved = true - return res - } - ) - const killAppPromise = killApp(app, LONG_RUNNING_MS * 2).then(() => { - killAppResolved = true - }) + const resPromise = fetchViaHTTP(appPort, '/api/long-running') + + // yield event loop to kick off request before killing the app + await waitFor(0) + process.kill(app.pid, 'SIGTERM') expect(isAppRunning(app)).toBe(true) // `next dev` should kill the child immediately let start = Date.now() - await expect(resPromise).rejects.toThrow(/socket hang up/) - let end = Date.now() - expect(end - start).toBeLessThan(LONG_RUNNING_MS) - expect(responseResolved).toBe(false) + await expect(resPromise).rejects.toThrow() + expect(Date.now() - start).toBeLessThan(LONG_RUNNING_MS) // `next dev` parent process is still running cleanup expect(isAppRunning(app)).toBe(true) - expect(killAppResolved).toBe(false) // App finally shuts down - await killAppPromise + await waitForAppShutdown(app) expect(isAppRunning(app)).toBe(false) - expect(killAppResolved).toBe(true) }) } else { it('should wait for requests to complete before exiting', async () => { let responseResolved = false - let killAppResolved = false const resPromise = fetchViaHTTP(appPort, '/api/long-running') .then((res) => { responseResolved = true return res }) .catch(() => {}) - const killAppPromise = killApp(app, LONG_RUNNING_MS * 2).then(() => { - killAppResolved = true - }) + + // yield event loop to kick off request before killing the app + await waitFor(0) + process.kill(app.pid, 'SIGTERM') expect(isAppRunning(app)).toBe(true) // Long running response should still be running after a bit await waitFor(LONG_RUNNING_MS / 2) expect(isAppRunning(app)).toBe(true) expect(responseResolved).toBe(false) - expect(killAppResolved).toBe(false) // App responds as expected without being interrupted const res = await resPromise @@ -83,38 +73,59 @@ function runTests(dev = false) { // App is still running briefly after response returns expect(isAppRunning(app)).toBe(true) expect(responseResolved).toBe(true) - expect(killAppResolved).toBe(false) // App finally shuts down - await killAppPromise + await waitForAppShutdown(app) expect(isAppRunning(app)).toBe(false) - expect(killAppResolved).toBe(true) }) - it('should not accept new requests during shutdown cleanup', async () => { - const resPromise = fetchViaHTTP(appPort, '/api/long-running') - const killAppPromise = killApp(app, LONG_RUNNING_MS * 2) - expect(isAppRunning(app)).toBe(true) + describe('should not accept new requests during shutdown cleanup', () => { + it('when request is made before shutdown', async () => { + const resPromise = fetchViaHTTP(appPort, '/api/long-running') - // Long running response should still be running after a bit - await waitFor(LONG_RUNNING_MS / 2) - expect(isAppRunning(app)).toBe(true) + // yield event loop to kick off request before killing the app + await waitFor(0) + process.kill(app.pid, 'SIGTERM') + expect(isAppRunning(app)).toBe(true) - // Second request should be rejected - await expect(fetchViaHTTP(appPort, '/api/long-running')).rejects.toThrow() + // Long running response should still be running after a bit + await waitFor(LONG_RUNNING_MS / 2) + expect(isAppRunning(app)).toBe(true) - // Original request responds as expected without being interrupted - await expect(resPromise).resolves.toBeTruthy() - const res = await resPromise - expect(res?.status).toBe(200) - expect(await res?.json()).toStrictEqual({ hello: 'world' }) + // Second request should be rejected + await expect( + fetchViaHTTP(appPort, '/api/long-running') + ).rejects.toThrow() - // App is still running briefly after response returns - expect(isAppRunning(app)).toBe(true) + // Original request responds as expected without being interrupted + await expect(resPromise).resolves.toBeTruthy() + const res = await resPromise + expect(res?.status).toBe(200) + expect(await res?.json()).toStrictEqual({ hello: 'world' }) - // App finally shuts down - await killAppPromise - expect(isAppRunning(app)).toBe(false) + // App is still running briefly after response returns + expect(isAppRunning(app)).toBe(true) + + // App finally shuts down + await waitForAppShutdown(app) + expect(isAppRunning(app)).toBe(false) + }) + + it('when there is no activity', async () => { + process.kill(app.pid, 'SIGTERM') + expect(isAppRunning(app)).toBe(true) + + await expect( + fetchViaHTTP(appPort, '/api/long-running') + ).rejects.toThrow() + + // App is still running briefly while server is closing + expect(isAppRunning(app)).toBe(true) + + // App finally shuts down + await waitForAppShutdown(app) + expect(isAppRunning(app)).toBe(false) + }) }) } } diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 274448fdd50a0..06d8cf53715be 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -550,13 +550,16 @@ async function waitForCondition( throw new Error(`Timed out after ${maxWait}ms`) } +export async function waitForAppShutdown(instance: ChildProcess) { + await waitForCondition(() => !isAppRunning(instance), 300, 20) +} + // Kill a launched app -export async function killApp(instance: ChildProcess, maxWait: number = 500) { +export async function killApp(instance: ChildProcess) { if (instance && instance.pid) { // waits for the signal to be sent, but not for the process to exit await killProcess(instance.pid) - // poll frequently to see if the process has exited - await waitForCondition(() => !isAppRunning(instance), maxWait, maxWait / 20) + await waitForAppShutdown(instance) } } From 7c8ff12936a9f6a45578c14a492022082a1ad736 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Fri, 29 Dec 2023 16:08:37 -0800 Subject: [PATCH 05/15] refactor: use once(pid, "kill") instead of complex waitForCondition --- .../graceful-shutdown/test/index.test.js | 24 +++++++++++------ test/lib/next-test-utils.ts | 26 +++---------------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/test/integration/graceful-shutdown/test/index.test.js b/test/integration/graceful-shutdown/test/index.test.js index b1c002a0a4922..74c8ae6e9a0f6 100644 --- a/test/integration/graceful-shutdown/test/index.test.js +++ b/test/integration/graceful-shutdown/test/index.test.js @@ -10,9 +10,9 @@ import { nextBuild, nextStart, waitFor, - waitForAppShutdown, } from 'next-test-utils' import { LONG_RUNNING_MS } from '../pages/api/long-running' +import { once } from 'events' const appDir = join(__dirname, '../') let appPort @@ -21,6 +21,8 @@ let app 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') @@ -29,7 +31,7 @@ function runTests(dev = false) { const resPromise = fetchViaHTTP(appPort, '/api/long-running') // yield event loop to kick off request before killing the app - await waitFor(0) + await waitFor(20) process.kill(app.pid, 'SIGTERM') expect(isAppRunning(app)).toBe(true) @@ -42,11 +44,13 @@ function runTests(dev = false) { expect(isAppRunning(app)).toBe(true) // App finally shuts down - await waitForAppShutdown(app) + await appKilledPromise expect(isAppRunning(app)).toBe(false) }) } 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) => { @@ -56,7 +60,7 @@ function runTests(dev = false) { .catch(() => {}) // yield event loop to kick off request before killing the app - await waitFor(0) + await waitFor(20) process.kill(app.pid, 'SIGTERM') expect(isAppRunning(app)).toBe(true) @@ -75,16 +79,18 @@ function runTests(dev = false) { expect(responseResolved).toBe(true) // App finally shuts down - await waitForAppShutdown(app) + await appKilledPromise expect(isAppRunning(app)).toBe(false) }) 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(0) + await waitFor(20) process.kill(app.pid, 'SIGTERM') expect(isAppRunning(app)).toBe(true) @@ -107,11 +113,13 @@ function runTests(dev = false) { expect(isAppRunning(app)).toBe(true) // App finally shuts down - await waitForAppShutdown(app) + await appKilledPromise expect(isAppRunning(app)).toBe(false) }) it('when there is no activity', async () => { + const appKilledPromise = once(app, 'exit') + process.kill(app.pid, 'SIGTERM') expect(isAppRunning(app)).toBe(true) @@ -123,7 +131,7 @@ function runTests(dev = false) { expect(isAppRunning(app)).toBe(true) // App finally shuts down - await waitForAppShutdown(app) + await appKilledPromise expect(isAppRunning(app)).toBe(false) }) }) diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 06d8cf53715be..cdabd6ae7e5a4 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' @@ -535,31 +536,12 @@ export function isAppRunning(instance: ChildProcess) { } } -async function waitForCondition( - condition: () => boolean | Promise, - maxWait: number, - pollInterval: number -) { - const start = Date.now() - while (Date.now() - start < maxWait) { - if (await condition()) { - return - } - await waitFor(pollInterval) - } - throw new Error(`Timed out after ${maxWait}ms`) -} - -export async function waitForAppShutdown(instance: ChildProcess) { - await waitForCondition(() => !isAppRunning(instance), 300, 20) -} - // Kill a launched app export async function killApp(instance: ChildProcess) { - if (instance && instance.pid) { - // waits for the signal to be sent, but not for the process to exit + if (instance?.pid && isAppRunning(instance)) { + const exitPromise = once(instance, 'exit') await killProcess(instance.pid) - await waitForAppShutdown(instance) + await exitPromise } } From c739e112cb5c46b5f05cbfe96ba65fd770f0d0d4 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Fri, 29 Dec 2023 18:48:19 -0800 Subject: [PATCH 06/15] fix: standalone server graceful-shudown - moved some js files to ts and fixed types - fixed standalone server graceful-shutdown - added new tests for production standalone mode --- packages/next/src/build/utils.ts | 7 - packages/next/src/cli/next-dev.ts | 11 +- .../api/{long-running.js => long-running.ts} | 2 +- .../test/{index.test.js => index.test.ts} | 17 +- test/lib/next-modes/base.ts | 13 +- .../graceful-shutdown/index.test.ts | 159 ++++++++++++++++++ .../graceful-shutdown/next.config.js | 3 + .../pages/api/long-running.ts | 6 + 8 files changed, 191 insertions(+), 27 deletions(-) rename test/integration/graceful-shutdown/pages/api/{long-running.js => long-running.ts} (80%) rename test/integration/graceful-shutdown/test/{index.test.js => index.test.ts} (91%) create mode 100644 test/production/standalone-mode/graceful-shutdown/index.test.ts create mode 100644 test/production/standalone-mode/graceful-shutdown/next.config.js create mode 100644 test/production/standalone-mode/graceful-shutdown/pages/api/long-running.ts diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index 8a7d94b35cc63..c953b48669160 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -2049,13 +2049,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 2eb26aa174c6b..e1a99d27b1a2f 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -36,7 +36,7 @@ import os from 'os' import { once } from 'node:events' type Child = ReturnType -type ExitCode = Parameters[0] +type KillSignal = Parameters[0] let dir: string let child: undefined | Child @@ -46,14 +46,17 @@ let traceUploadUrl: string let sessionStopHandled = false let sessionStarted = Date.now() -const handleSessionStop = async (signal: ExitCode | null) => { +const handleSessionStop = async ( + signal: KillSignal | null, + childExited: boolean = false +) => { if (child) { child.kill(signal ?? 0) } if (sessionStopHandled) return sessionStopHandled = true - if (child) { + if (child && !childExited) { await once(child, 'exit').catch(() => {}) } @@ -298,7 +301,7 @@ const nextDev: CliCommand = async (args) => { } return startServer(options) } - await handleSessionStop(signal) + await handleSessionStop(signal, true) }) }) } diff --git a/test/integration/graceful-shutdown/pages/api/long-running.js b/test/integration/graceful-shutdown/pages/api/long-running.ts similarity index 80% rename from test/integration/graceful-shutdown/pages/api/long-running.js rename to test/integration/graceful-shutdown/pages/api/long-running.ts index 4187b207185c4..749713e990036 100644 --- a/test/integration/graceful-shutdown/pages/api/long-running.js +++ b/test/integration/graceful-shutdown/pages/api/long-running.ts @@ -1,4 +1,4 @@ -export const LONG_RUNNING_MS = 100 +export const LONG_RUNNING_MS = 400 export default async (req, res) => { await new Promise((resolve) => setTimeout(resolve, LONG_RUNNING_MS)) diff --git a/test/integration/graceful-shutdown/test/index.test.js b/test/integration/graceful-shutdown/test/index.test.ts similarity index 91% rename from test/integration/graceful-shutdown/test/index.test.js rename to test/integration/graceful-shutdown/test/index.test.ts index 74c8ae6e9a0f6..cdea34843359b 100644 --- a/test/integration/graceful-shutdown/test/index.test.js +++ b/test/integration/graceful-shutdown/test/index.test.ts @@ -18,6 +18,10 @@ const appDir = join(__dirname, '../') let appPort let app +function assertDefined(value: T | void): asserts value is T { + expect(value).toBeDefined() +} + function runTests(dev = false) { if (dev) { it('should shut down child immediately', async () => { @@ -26,7 +30,7 @@ function runTests(dev = false) { // let the dev server compile the route before running the test await expect( fetchViaHTTP(appPort, '/api/long-running') - ).resolves.toBeTruthy() + ).resolves.toBeDefined() const resPromise = fetchViaHTTP(appPort, '/api/long-running') @@ -71,8 +75,9 @@ function runTests(dev = false) { // App responds as expected without being interrupted const res = await resPromise - expect(res?.status).toBe(200) - expect(await res?.json()).toStrictEqual({ hello: 'world' }) + assertDefined(res) + expect(res.status).toBe(200) + expect(await res.json()).toStrictEqual({ hello: 'world' }) // App is still running briefly after response returns expect(isAppRunning(app)).toBe(true) @@ -104,10 +109,10 @@ function runTests(dev = false) { ).rejects.toThrow() // Original request responds as expected without being interrupted - await expect(resPromise).resolves.toBeTruthy() + await expect(resPromise).resolves.toBeDefined() const res = await resPromise - expect(res?.status).toBe(200) - expect(await res?.json()).toStrictEqual({ hello: 'world' }) + expect(res.status).toBe(200) + expect(await res.json()).toStrictEqual({ hello: 'world' }) // App is still running briefly after response returns expect(isAppRunning(app)).toBe(true) diff --git a/test/lib/next-modes/base.ts b/test/lib/next-modes/base.ts index 348624e818cf3..7370cacd35d99 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 } 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 = {} @@ -342,14 +343,8 @@ export class NextInstance { public async start(useDirArg: boolean = false): Promise {} public async stop(): Promise { this.isStopping = true - if (this.childProcess) { - let exitResolve - const exitPromise = new Promise((resolve) => { - exitResolve = resolve - }) - this.childProcess.addListener('exit', () => { - exitResolve() - }) + if (this.childProcess && !this.isStopping) { + const exitPromise = once(this.childProcess, 'exit') await new Promise((resolve) => { treeKill(this.childProcess.pid, 'SIGKILL', (err) => { if (err) { diff --git a/test/production/standalone-mode/graceful-shutdown/index.test.ts b/test/production/standalone-mode/graceful-shutdown/index.test.ts new file mode 100644 index 0000000000000..826faa109b913 --- /dev/null +++ b/test/production/standalone-mode/graceful-shutdown/index.test.ts @@ -0,0 +1,159 @@ +import { NextInstance, createNext } from 'e2e-utils' +import { + fetchViaHTTP, + findPort, + initNextServerScript, + isAppRunning, + killApp, + waitFor, +} from 'next-test-utils' +import glob from 'glob' +import { join } from 'path' +import fs from 'fs-extra' +import { once } from 'events' + +import { LONG_RUNNING_MS } from './pages/api/long-running' + +function assertDefined(value: T | void): asserts value is T { + expect(value).toBeDefined() +} + +describe('standalone mode - graceful shutdown', () => { + let next: NextInstance + let appPort + let serverFile + let app + + beforeAll(async () => { + next = await createNext({ + files: __dirname, + 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)) + console.log('removed', 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()) + + 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(isAppRunning(app)).toBe(true) + + // Long running response should still be running after a bit + await waitFor(LONG_RUNNING_MS / 2) + expect(isAppRunning(app)).toBe(true) + 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(isAppRunning(app)).toBe(true) + expect(responseResolved).toBe(true) + + // App finally shuts down + await appKilledPromise + expect(isAppRunning(app)).toBe(false) + }) + + 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(isAppRunning(app)).toBe(true) + + // Long running response should still be running after a bit + await waitFor(LONG_RUNNING_MS / 2) + expect(isAppRunning(app)).toBe(true) + + // 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(isAppRunning(app)).toBe(true) + + // App finally shuts down + await appKilledPromise + expect(isAppRunning(app)).toBe(false) + }) + + it('when there is no activity', async () => { + const appKilledPromise = once(app, 'exit') + + process.kill(app.pid, 'SIGTERM') + expect(isAppRunning(app)).toBe(true) + + // 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(isAppRunning(app)).toBe(false) + }) + }) +}) diff --git a/test/production/standalone-mode/graceful-shutdown/next.config.js b/test/production/standalone-mode/graceful-shutdown/next.config.js new file mode 100644 index 0000000000000..e97173b4b3799 --- /dev/null +++ b/test/production/standalone-mode/graceful-shutdown/next.config.js @@ -0,0 +1,3 @@ +module.exports = { + output: 'standalone', +} diff --git a/test/production/standalone-mode/graceful-shutdown/pages/api/long-running.ts b/test/production/standalone-mode/graceful-shutdown/pages/api/long-running.ts new file mode 100644 index 0000000000000..749713e990036 --- /dev/null +++ b/test/production/standalone-mode/graceful-shutdown/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' }) +} From 4f74aee9987c5a82d498dfebed9be2365a1e7a6b Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Sat, 30 Dec 2023 13:27:03 -0800 Subject: [PATCH 07/15] refactor: combine graceful-shutdown tests rather than having an integration test with dev and production modes, and then a separate test for standalone-mode that copies all the same tests, we can just follow the same pattern we had in integration but include the standalone-mode tests in the same file. --- .../graceful-shutdown}/index.test.ts | 130 ++++++++++---- .../pages/api/long-running.ts | 0 .../graceful-shutdown/tsconfig.json | 18 ++ .../graceful-shutdown/index.test.ts | 159 ------------------ .../graceful-shutdown/next.config.js | 3 - .../pages/api/long-running.ts | 6 - 6 files changed, 117 insertions(+), 199 deletions(-) rename test/{integration/graceful-shutdown/test => production/graceful-shutdown}/index.test.ts (63%) rename test/{integration => production}/graceful-shutdown/pages/api/long-running.ts (100%) create mode 100644 test/production/graceful-shutdown/tsconfig.json delete mode 100644 test/production/standalone-mode/graceful-shutdown/index.test.ts delete mode 100644 test/production/standalone-mode/graceful-shutdown/next.config.js delete mode 100644 test/production/standalone-mode/graceful-shutdown/pages/api/long-running.ts diff --git a/test/integration/graceful-shutdown/test/index.test.ts b/test/production/graceful-shutdown/index.test.ts similarity index 63% rename from test/integration/graceful-shutdown/test/index.test.ts rename to test/production/graceful-shutdown/index.test.ts index cdea34843359b..7e4aa03e58bdf 100644 --- a/test/integration/graceful-shutdown/test/index.test.ts +++ b/test/production/graceful-shutdown/index.test.ts @@ -1,9 +1,9 @@ -/* eslint-env jest */ - import { join } from 'path' +import { NextInstance, createNext, FileRef } from 'e2e-utils' import { fetchViaHTTP, findPort, + initNextServerScript, isAppRunning, killApp, launchApp, @@ -11,10 +11,11 @@ import { nextStart, waitFor, } from 'next-test-utils' -import { LONG_RUNNING_MS } from '../pages/api/long-running' +import fs from 'fs-extra' +import glob from 'glob' +import { LONG_RUNNING_MS } from './pages/api/long-running' import { once } from 'events' -const appDir = join(__dirname, '../') let appPort let app @@ -22,6 +23,98 @@ 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(__dirname, appPort) + }) + afterEach(() => killApp(app)) + + runTests(true) + }) + ;(process.env.TURBOPACK ? describe.skip : describe)( + 'production (next start)', + () => { + beforeAll(async () => { + await nextBuild(__dirname) + }) + beforeEach(async () => { + appPort = await findPort() + app = await nextStart(__dirname, 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: __dirname, dot: false })) { + projectFiles[file] = new FileRef(join(__dirname, 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 () => { @@ -128,13 +221,12 @@ function runTests(dev = false) { process.kill(app.pid, 'SIGTERM') expect(isAppRunning(app)).toBe(true) + // yield event loop to allow server to start the shutdown process + await waitFor(20) await expect( fetchViaHTTP(appPort, '/api/long-running') ).rejects.toThrow() - // App is still running briefly while server is closing - expect(isAppRunning(app)).toBe(true) - // App finally shuts down await appKilledPromise expect(isAppRunning(app)).toBe(false) @@ -142,27 +234,3 @@ function runTests(dev = false) { }) } } - -describe('API routes', () => { - describe('dev support', () => { - beforeEach(async () => { - appPort = await findPort() - app = await launchApp(appDir, appPort) - }) - afterEach(() => killApp(app)) - - runTests(true) - }) - ;(process.env.TURBOPACK ? describe.skip : describe)('production mode', () => { - beforeAll(async () => { - await nextBuild(appDir) - }) - beforeEach(async () => { - appPort = await findPort() - app = await nextStart(appDir, appPort) - }) - afterEach(() => killApp(app)) - - runTests() - }) -}) diff --git a/test/integration/graceful-shutdown/pages/api/long-running.ts b/test/production/graceful-shutdown/pages/api/long-running.ts similarity index 100% rename from test/integration/graceful-shutdown/pages/api/long-running.ts rename to test/production/graceful-shutdown/pages/api/long-running.ts diff --git a/test/production/graceful-shutdown/tsconfig.json b/test/production/graceful-shutdown/tsconfig.json new file mode 100644 index 0000000000000..1c4f693a991e7 --- /dev/null +++ b/test/production/graceful-shutdown/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"] +} diff --git a/test/production/standalone-mode/graceful-shutdown/index.test.ts b/test/production/standalone-mode/graceful-shutdown/index.test.ts deleted file mode 100644 index 826faa109b913..0000000000000 --- a/test/production/standalone-mode/graceful-shutdown/index.test.ts +++ /dev/null @@ -1,159 +0,0 @@ -import { NextInstance, createNext } from 'e2e-utils' -import { - fetchViaHTTP, - findPort, - initNextServerScript, - isAppRunning, - killApp, - waitFor, -} from 'next-test-utils' -import glob from 'glob' -import { join } from 'path' -import fs from 'fs-extra' -import { once } from 'events' - -import { LONG_RUNNING_MS } from './pages/api/long-running' - -function assertDefined(value: T | void): asserts value is T { - expect(value).toBeDefined() -} - -describe('standalone mode - graceful shutdown', () => { - let next: NextInstance - let appPort - let serverFile - let app - - beforeAll(async () => { - next = await createNext({ - files: __dirname, - 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)) - console.log('removed', 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()) - - 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(isAppRunning(app)).toBe(true) - - // Long running response should still be running after a bit - await waitFor(LONG_RUNNING_MS / 2) - expect(isAppRunning(app)).toBe(true) - 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(isAppRunning(app)).toBe(true) - expect(responseResolved).toBe(true) - - // App finally shuts down - await appKilledPromise - expect(isAppRunning(app)).toBe(false) - }) - - 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(isAppRunning(app)).toBe(true) - - // Long running response should still be running after a bit - await waitFor(LONG_RUNNING_MS / 2) - expect(isAppRunning(app)).toBe(true) - - // 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(isAppRunning(app)).toBe(true) - - // App finally shuts down - await appKilledPromise - expect(isAppRunning(app)).toBe(false) - }) - - it('when there is no activity', async () => { - const appKilledPromise = once(app, 'exit') - - process.kill(app.pid, 'SIGTERM') - expect(isAppRunning(app)).toBe(true) - - // 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(isAppRunning(app)).toBe(false) - }) - }) -}) diff --git a/test/production/standalone-mode/graceful-shutdown/next.config.js b/test/production/standalone-mode/graceful-shutdown/next.config.js deleted file mode 100644 index e97173b4b3799..0000000000000 --- a/test/production/standalone-mode/graceful-shutdown/next.config.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - output: 'standalone', -} diff --git a/test/production/standalone-mode/graceful-shutdown/pages/api/long-running.ts b/test/production/standalone-mode/graceful-shutdown/pages/api/long-running.ts deleted file mode 100644 index 749713e990036..0000000000000 --- a/test/production/standalone-mode/graceful-shutdown/pages/api/long-running.ts +++ /dev/null @@ -1,6 +0,0 @@ -export const LONG_RUNNING_MS = 400 - -export default async (req, res) => { - await new Promise((resolve) => setTimeout(resolve, LONG_RUNNING_MS)) - res.json({ hello: 'world' }) -} From 90af3bb96439e556e554d4003b4c74a842ced5ae Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Sat, 30 Dec 2023 13:46:03 -0800 Subject: [PATCH 08/15] refactor: move graceful-shutdown test-app to src folder the tsconfig.json file was causing typescript issues when looking for e2e-utils and next-test-utils --- test/production/graceful-shutdown/index.test.ts | 13 +++++++------ .../{ => src}/pages/api/long-running.ts | 0 .../graceful-shutdown/{ => src}/tsconfig.json | 0 3 files changed, 7 insertions(+), 6 deletions(-) rename test/production/graceful-shutdown/{ => src}/pages/api/long-running.ts (100%) rename test/production/graceful-shutdown/{ => src}/tsconfig.json (100%) diff --git a/test/production/graceful-shutdown/index.test.ts b/test/production/graceful-shutdown/index.test.ts index 7e4aa03e58bdf..e0200e3f996a1 100644 --- a/test/production/graceful-shutdown/index.test.ts +++ b/test/production/graceful-shutdown/index.test.ts @@ -13,9 +13,10 @@ import { } from 'next-test-utils' import fs from 'fs-extra' import glob from 'glob' -import { LONG_RUNNING_MS } from './pages/api/long-running' +import { LONG_RUNNING_MS } from './src/pages/api/long-running' import { once } from 'events' +const appDir = join(__dirname, './src') let appPort let app @@ -27,7 +28,7 @@ describe('Graceful Shutdown', () => { describe('development (next dev)', () => { beforeEach(async () => { appPort = await findPort() - app = await launchApp(__dirname, appPort) + app = await launchApp(appDir, appPort) }) afterEach(() => killApp(app)) @@ -37,11 +38,11 @@ describe('Graceful Shutdown', () => { 'production (next start)', () => { beforeAll(async () => { - await nextBuild(__dirname) + await nextBuild(appDir) }) beforeEach(async () => { appPort = await findPort() - app = await nextStart(__dirname, appPort) + app = await nextStart(appDir, appPort) }) afterEach(() => killApp(app)) @@ -58,8 +59,8 @@ describe('Graceful Shutdown', () => { 'next.config.mjs': `export default { output: 'standalone' }`, } - for (const file of glob.sync('*', { cwd: __dirname, dot: false })) { - projectFiles[file] = new FileRef(join(__dirname, file)) + for (const file of glob.sync('*', { cwd: appDir, dot: false })) { + projectFiles[file] = new FileRef(join(appDir, file)) } beforeAll(async () => { diff --git a/test/production/graceful-shutdown/pages/api/long-running.ts b/test/production/graceful-shutdown/src/pages/api/long-running.ts similarity index 100% rename from test/production/graceful-shutdown/pages/api/long-running.ts rename to test/production/graceful-shutdown/src/pages/api/long-running.ts diff --git a/test/production/graceful-shutdown/tsconfig.json b/test/production/graceful-shutdown/src/tsconfig.json similarity index 100% rename from test/production/graceful-shutdown/tsconfig.json rename to test/production/graceful-shutdown/src/tsconfig.json From b1734961823729a2cff6356b73f7a46018191fc2 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Thu, 4 Jan 2024 11:28:06 -0800 Subject: [PATCH 09/15] fix: remove check for !this.isStopping keeping code refactor to use "once", but adding that extra check broke a lot of tests --- test/lib/next-modes/base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/next-modes/base.ts b/test/lib/next-modes/base.ts index ec7d32490a73c..4ba274724f2df 100644 --- a/test/lib/next-modes/base.ts +++ b/test/lib/next-modes/base.ts @@ -345,7 +345,7 @@ export class NextInstance { public async start(useDirArg: boolean = false): Promise {} public async stop(): Promise { this.isStopping = true - if (this.childProcess && !this.isStopping) { + if (this.childProcess) { const exitPromise = once(this.childProcess, 'exit') await new Promise((resolve) => { treeKill(this.childProcess.pid, 'SIGKILL', (err) => { From df3a4e5a2263d4edea62a7658b627b1d18b77697 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Thu, 4 Jan 2024 13:03:19 -0800 Subject: [PATCH 10/15] fix: allow SIGINT or SIGTERM to kill build --- packages/next/src/bin/next.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/next/src/bin/next.ts b/packages/next/src/bin/next.ts index de7064e7c060d..b01debaebb4bc 100755 --- a/packages/next/src/bin/next.ts +++ b/packages/next/src/bin/next.ts @@ -108,6 +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' +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) From c40330aa1033a66a1687cc0942016c8137139024 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Thu, 4 Jan 2024 13:32:00 -0800 Subject: [PATCH 11/15] fix: css-test with proxy not properly exiting --- test/integration/css-client-nav/test/index.test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/css-client-nav/test/index.test.js b/test/integration/css-client-nav/test/index.test.js index b2f61541f1b29..2213a645ae377 100644 --- a/test/integration/css-client-nav/test/index.test.js +++ b/test/integration/css-client-nav/test/index.test.js @@ -7,6 +7,7 @@ import { remove } from 'fs-extra' import { findPort, killApp, + killProcess, launchApp, nextBuild, nextStart, @@ -180,6 +181,8 @@ describe('CSS Module client-side navigation', () => { }) afterAll(async () => { proxyServer.close() + // something is hanging onto the process, so we need to SIGKILL + await killProcess(app.pid, 'SIGKILL') await killApp(app) }) From 8e273b8142c6329553dcb0f2e291729404788ded Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Sat, 13 Jan 2024 09:20:19 -0800 Subject: [PATCH 12/15] fix: use sigkill in tests by default, simplify types and isAppRunning --- packages/next/src/cli/next-dev.ts | 19 +++++-------- .../css-client-nav/test/index.test.js | 2 -- .../telemetry/test/page-features.test.js | 4 +-- test/lib/next-test-utils.ts | 23 +++++----------- .../graceful-shutdown/index.test.ts | 27 +++++++++---------- 5 files changed, 28 insertions(+), 47 deletions(-) diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts index c6936de6356b6..d9781a7ce9a9b 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -28,6 +28,7 @@ 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, @@ -35,28 +36,20 @@ import { import os from 'os' import { once } from 'node:events' -type Child = ReturnType -type KillSignal = Parameters[0] - let dir: string -let child: undefined | Child +let child: undefined | ChildProcess let config: NextConfigComplete let isTurboSession = false let traceUploadUrl: string let sessionStopHandled = false let sessionStarted = Date.now() -const handleSessionStop = async ( - signal: KillSignal | null, - childExited: boolean = false -) => { - if (child) { - child.kill(signal ?? 0) - } +const handleSessionStop = async (signal: NodeJS.Signals | number | null) => { + if (child?.pid) child.kill(signal ?? 0) if (sessionStopHandled) return sessionStopHandled = true - if (child && !childExited) { + if (child?.pid && child.exitCode === null) { await once(child, 'exit').catch(() => {}) } @@ -303,7 +296,7 @@ const nextDev: CliCommand = async (args) => { } return startServer(options) } - await handleSessionStop(signal, true) + await handleSessionStop(signal) }) }) } diff --git a/test/integration/css-client-nav/test/index.test.js b/test/integration/css-client-nav/test/index.test.js index 2213a645ae377..500d77d4aeec0 100644 --- a/test/integration/css-client-nav/test/index.test.js +++ b/test/integration/css-client-nav/test/index.test.js @@ -181,8 +181,6 @@ describe('CSS Module client-side navigation', () => { }) afterAll(async () => { proxyServer.close() - // something is hanging onto the process, so we need to SIGKILL - await killProcess(app.pid, 'SIGKILL') await killApp(app) }) 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-test-utils.ts b/test/lib/next-test-utils.ts index cdabd6ae7e5a4..7136a4147ac50 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -498,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,23 +524,14 @@ export async function killProcess( }) } -export function isAppRunning(instance: ChildProcess) { - if (!instance?.pid) return false - - try { - // 0 is a special signal that tests the existence of a process - process.kill(instance.pid, 0) - return true - } catch { - return false - } -} - // Kill a launched app -export async function killApp(instance: ChildProcess) { - if (instance?.pid && isAppRunning(instance)) { +export async function killApp( + instance: ChildProcess, + signal: NodeJS.Signals | number = 'SIGKILL' +) { + if (instance?.pid && instance.exitCode === null) { const exitPromise = once(instance, 'exit') - await killProcess(instance.pid) + 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 index e0200e3f996a1..1af4758170b78 100644 --- a/test/production/graceful-shutdown/index.test.ts +++ b/test/production/graceful-shutdown/index.test.ts @@ -4,7 +4,6 @@ import { fetchViaHTTP, findPort, initNextServerScript, - isAppRunning, killApp, launchApp, nextBuild, @@ -131,7 +130,7 @@ function runTests(dev = false) { // yield event loop to kick off request before killing the app await waitFor(20) process.kill(app.pid, 'SIGTERM') - expect(isAppRunning(app)).toBe(true) + expect(app.exitCode).toBe(null) // `next dev` should kill the child immediately let start = Date.now() @@ -139,11 +138,11 @@ function runTests(dev = false) { expect(Date.now() - start).toBeLessThan(LONG_RUNNING_MS) // `next dev` parent process is still running cleanup - expect(isAppRunning(app)).toBe(true) + expect(app.exitCode).toBe(null) // App finally shuts down await appKilledPromise - expect(isAppRunning(app)).toBe(false) + expect(app.exitCode).toBe(0) }) } else { it('should wait for requests to complete before exiting', async () => { @@ -160,11 +159,11 @@ function runTests(dev = false) { // yield event loop to kick off request before killing the app await waitFor(20) process.kill(app.pid, 'SIGTERM') - expect(isAppRunning(app)).toBe(true) + expect(app.exitCode).toBe(null) // Long running response should still be running after a bit await waitFor(LONG_RUNNING_MS / 2) - expect(isAppRunning(app)).toBe(true) + expect(app.exitCode).toBe(null) expect(responseResolved).toBe(false) // App responds as expected without being interrupted @@ -174,12 +173,12 @@ function runTests(dev = false) { expect(await res.json()).toStrictEqual({ hello: 'world' }) // App is still running briefly after response returns - expect(isAppRunning(app)).toBe(true) + expect(app.exitCode).toBe(null) expect(responseResolved).toBe(true) // App finally shuts down await appKilledPromise - expect(isAppRunning(app)).toBe(false) + expect(app.exitCode).toBe(0) }) describe('should not accept new requests during shutdown cleanup', () => { @@ -191,11 +190,11 @@ function runTests(dev = false) { // yield event loop to kick off request before killing the app await waitFor(20) process.kill(app.pid, 'SIGTERM') - expect(isAppRunning(app)).toBe(true) + expect(app.exitCode).toBe(null) // Long running response should still be running after a bit await waitFor(LONG_RUNNING_MS / 2) - expect(isAppRunning(app)).toBe(true) + expect(app.exitCode).toBe(null) // Second request should be rejected await expect( @@ -209,18 +208,18 @@ function runTests(dev = false) { expect(await res.json()).toStrictEqual({ hello: 'world' }) // App is still running briefly after response returns - expect(isAppRunning(app)).toBe(true) + expect(app.exitCode).toBe(null) // App finally shuts down await appKilledPromise - expect(isAppRunning(app)).toBe(false) + expect(app.exitCode).toBe(0) }) it('when there is no activity', async () => { const appKilledPromise = once(app, 'exit') process.kill(app.pid, 'SIGTERM') - expect(isAppRunning(app)).toBe(true) + expect(app.exitCode).toBe(null) // yield event loop to allow server to start the shutdown process await waitFor(20) @@ -230,7 +229,7 @@ function runTests(dev = false) { // App finally shuts down await appKilledPromise - expect(isAppRunning(app)).toBe(false) + expect(app.exitCode).toBe(0) }) }) } From e340c417e2f99c1027d7a1ba4d11ee28c895bafd Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Sat, 13 Jan 2024 09:24:23 -0800 Subject: [PATCH 13/15] fix: linting --- test/integration/css-client-nav/test/index.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/css-client-nav/test/index.test.js b/test/integration/css-client-nav/test/index.test.js index 500d77d4aeec0..b2f61541f1b29 100644 --- a/test/integration/css-client-nav/test/index.test.js +++ b/test/integration/css-client-nav/test/index.test.js @@ -7,7 +7,6 @@ import { remove } from 'fs-extra' import { findPort, killApp, - killProcess, launchApp, nextBuild, nextStart, From 3f65ded1cca0b9d92e701869656a85101441c949 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Sat, 13 Jan 2024 10:29:58 -0800 Subject: [PATCH 14/15] fix: also check for signalCode --- packages/next/src/cli/next-dev.ts | 2 +- test/lib/next-test-utils.ts | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts index d9781a7ce9a9b..aba4e22d49c55 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -49,7 +49,7 @@ const handleSessionStop = async (signal: NodeJS.Signals | number | null) => { if (sessionStopHandled) return sessionStopHandled = true - if (child?.pid && child.exitCode === null) { + if (child?.pid && child.exitCode === null && child.signalCode === null) { await once(child, 'exit').catch(() => {}) } diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 7136a4147ac50..6dc5d5631bfab 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -529,7 +529,11 @@ export async function killApp( instance: ChildProcess, signal: NodeJS.Signals | number = 'SIGKILL' ) { - if (instance?.pid && instance.exitCode === null) { + if ( + instance.pid && + instance.exitCode === null && + instance.signalCode === null + ) { const exitPromise = once(instance, 'exit') await killProcess(instance.pid, signal) await exitPromise From e49fb6d0b7c2a810e4989f251e0ce47a2e1b2a29 Mon Sep 17 00:00:00 2001 From: "Braden M. Kelley" Date: Sat, 13 Jan 2024 10:44:04 -0800 Subject: [PATCH 15/15] fix: killApp instance is actually optional --- test/lib/next-test-utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 6dc5d5631bfab..6d64ad2e56601 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -526,11 +526,11 @@ export async function killProcess( // Kill a launched app export async function killApp( - instance: ChildProcess, + instance?: ChildProcess, signal: NodeJS.Signals | number = 'SIGKILL' ) { if ( - instance.pid && + instance?.pid && instance.exitCode === null && instance.signalCode === null ) {