Skip to content

Commit

Permalink
Ensure next commands respect termination signals (vercel#19433)
Browse files Browse the repository at this point in the history
Fixes vercel#16173

## What

Restores handling of termination signals, `SIGTERM` and `SIGINT`, to allow graceful termination of next commands. Seems to have been removed during a child process refactor vercel#6450, was this intentional?

## Why 

Currently the command processes have to be forcefully killed. This would help those using Next.js with custom servers and tools like Docker and Kubernetes that rely on termination signals to shutdown instances.

---

Where would be a good location to add some tests? [test/integration/cli/test/index.test.js](https://github.com/vercel/next.js/blob/fc98c13a2e6bc545073de0eedbe045352b387f6d/test/integration/cli/test/index.test.js)?
  • Loading branch information
jamsinclair authored Nov 25, 2020
1 parent f4bf8a4 commit 397a375
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 1 deletion.
4 changes: 4 additions & 0 deletions packages/next/bin/next.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ if (typeof React.Suspense === 'undefined') {
)
}

// Make sure commands gracefully respect termination signals (e.g. from Docker)
process.on('SIGTERM', () => process.exit(0))
process.on('SIGINT', () => process.exit(0))

commands[command]().then((exec) => exec(forwardedArgs))

if (command === 'dev') {
Expand Down
100 changes: 100 additions & 0 deletions test/integration/cli/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
findPort,
killApp,
launchApp,
nextBuild,
runNextCommand,
runNextCommandDev,
} from 'next-test-utils'
Expand Down Expand Up @@ -85,6 +86,35 @@ describe('CLI Usage', () => {
})
expect(stderr).not.toContain('UnhandledPromiseRejectionWarning')
})

test('should exit when SIGINT is signalled', async () => {
const killSigint = (instance) =>
setTimeout(() => instance.kill('SIGINT'), 500)
const { code, signal } = await runNextCommand(['build', dir], {
ignoreFail: true,
instance: killSigint,
})
// Node can only partially emulate signals on Windows. Our signal handlers won't affect the exit code.
// See: https://nodejs.org/api/process.html#process_signal_events
const expectedExitCode = process.platform === `win32` ? null : 0
const expectedExitSignal = process.platform === `win32` ? 'SIGINT' : null
expect(code).toBe(expectedExitCode)
expect(signal).toBe(expectedExitSignal)
})
test('should exit when SIGTERM is signalled', async () => {
const killSigterm = (instance) =>
setTimeout(() => instance.kill('SIGTERM'), 500)
const { code, signal } = await runNextCommand(['build', dir], {
ignoreFail: true,
instance: killSigterm,
})
// Node can only partially emulate signals on Windows. Our signal handlers won't affect the exit code.
// See: https://nodejs.org/api/process.html#process_signal_events
const expectedExitCode = process.platform === `win32` ? null : 0
const expectedExitSignal = process.platform === `win32` ? 'SIGTERM' : null
expect(code).toBe(expectedExitCode)
expect(signal).toBe(expectedExitSignal)
})
})

describe('dev', () => {
Expand Down Expand Up @@ -193,6 +223,37 @@ describe('CLI Usage', () => {
})
expect(stderr).not.toContain('UnhandledPromiseRejectionWarning')
})

test('should exit when SIGINT is signalled', async () => {
const killSigint = (instance) =>
setTimeout(() => instance.kill('SIGINT'), 500)
const port = await findPort()
const { code, signal } = await runNextCommand(['dev', dir, '-p', port], {
ignoreFail: true,
instance: killSigint,
})
// Node can only partially emulate signals on Windows. Our signal handlers won't affect the exit code.
// See: https://nodejs.org/api/process.html#process_signal_events
const expectedExitCode = process.platform === `win32` ? null : 0
const expectedExitSignal = process.platform === `win32` ? 'SIGINT' : null
expect(code).toBe(expectedExitCode)
expect(signal).toBe(expectedExitSignal)
})
test('should exit when SIGTERM is signalled', async () => {
const killSigterm = (instance) =>
setTimeout(() => instance.kill('SIGTERM'), 500)
const port = await findPort()
const { code, signal } = await runNextCommand(['dev', dir, '-p', port], {
ignoreFail: true,
instance: killSigterm,
})
// Node can only partially emulate signals on Windows. Our signal handlers won't affect the exit code.
// See: https://nodejs.org/api/process.html#process_signal_events
const expectedExitCode = process.platform === `win32` ? null : 0
const expectedExitSignal = process.platform === `win32` ? 'SIGTERM' : null
expect(code).toBe(expectedExitCode)
expect(signal).toBe(expectedExitSignal)
})
})

describe('start', () => {
Expand Down Expand Up @@ -312,6 +373,45 @@ describe('CLI Usage', () => {

await killApp(instance)
})

test('should exit when SIGINT is signalled', async () => {
const killSigint = (instance) =>
setTimeout(() => instance.kill('SIGINT'), 500)
await nextBuild(dir)
const port = await findPort()
const { code, signal } = await runNextCommand(
['start', dir, '-p', port],
{
ignoreFail: true,
instance: killSigint,
}
)
// Node can only partially emulate signals on Windows. Our signal handlers won't affect the exit code.
// See: https://nodejs.org/api/process.html#process_signal_events
const expectedExitCode = process.platform === `win32` ? null : 0
const expectedExitSignal = process.platform === `win32` ? 'SIGINT' : null
expect(code).toBe(expectedExitCode)
expect(signal).toBe(expectedExitSignal)
})
test('should exit when SIGTERM is signalled', async () => {
const killSigterm = (instance) =>
setTimeout(() => instance.kill('SIGTERM'), 500)
await nextBuild(dir)
const port = await findPort()
const { code, signal } = await runNextCommand(
['start', dir, '-p', port],
{
ignoreFail: true,
instance: killSigterm,
}
)
// Node can only partially emulate signals on Windows. Our signal handlers won't affect the exit code.
// See: https://nodejs.org/api/process.html#process_signal_events
const expectedExitCode = process.platform === `win32` ? null : 0
const expectedExitSignal = process.platform === `win32` ? 'SIGTERM' : null
expect(code).toBe(expectedExitCode)
expect(signal).toBe(expectedExitSignal)
})
})

describe('export', () => {
Expand Down
3 changes: 2 additions & 1 deletion test/lib/next-test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export function runNextCommand(argv, options = {}) {
})
}

instance.on('close', (code) => {
instance.on('close', (code, signal) => {
if (
!options.stderr &&
!options.stdout &&
Expand All @@ -142,6 +142,7 @@ export function runNextCommand(argv, options = {}) {

resolve({
code,
signal,
stdout: stdoutOutput,
stderr: stderrOutput,
})
Expand Down

0 comments on commit 397a375

Please sign in to comment.