Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): allow configuration of http-server's timeout configuration #35827

Merged
merged 16 commits into from
Jun 26, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/api-reference/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ PORT=4000 npx next start

> Note: `PORT` can not be set in `.env` as booting up the HTTP server happens before any other code is initialized.

### Working with Load-balancer Timeouts
balazsorban44 marked this conversation as resolved.
Show resolved Hide resolved

When deploying Next.js behind a downstream proxy (e.g. a load-balancer like AWS ELB/ALB) it's important to configure Next's underlying http-server with [keep-alive timeouts](https://nodejs.org/api/http.html#http_server_keepalivetimeout) that are _larger_ than the downstream proxies' timeouts. Otherwise, once a keep-alive timeout is reached for a given TCP connection, Node.js will immediately terminate that connection without notifying the downstream proxies. This results in the downstream proxies throwing intermittent 5xx errors whenever they attempt to reuse a connection that Node.js has already terminated.
balazsorban44 marked this conversation as resolved.
Show resolved Hide resolved

To configure the timeout values for the production Next.js server, pass `--keepAliveTimeout` (in milliseconds) to `next start`, like so:

```bash
npx next start --keepAliveTimeout 70000
```

## Lint

`next lint` runs ESLint for all files in the `pages`, `components`, and `lib` directories. It also
Expand Down
20 changes: 20 additions & 0 deletions packages/next/cli/next-start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const nextStart: cliCommand = (argv) => {
'--help': Boolean,
'--port': Number,
'--hostname': String,
'--keepAliveTimeout': Number,

// Aliases
'-h': '--help',
Expand Down Expand Up @@ -44,6 +45,7 @@ const nextStart: cliCommand = (argv) => {
Options
--port, -p A port number on which to start the application
--hostname, -H Hostname on which to start the application (default: 0.0.0.0)
--keepAliveTimeout Max milliseconds to wait before closing inactive connections
--help, -h Displays this message
`)
process.exit(0)
Expand All @@ -58,10 +60,28 @@ const nextStart: cliCommand = (argv) => {
port = 0
}

const keepAliveTimeoutArg: number | undefined = args['--keepAliveTimeout']
if (
typeof keepAliveTimeoutArg !== 'undefined' &&
(Number.isNaN(keepAliveTimeoutArg) ||
!Number.isFinite(keepAliveTimeoutArg) ||
keepAliveTimeoutArg < 0)
) {
printAndExit(
`Invalid --keepAliveTimeout, expected a non negative number but received "${keepAliveTimeoutArg}"`,
1
)
}

const keepAliveTimeout = keepAliveTimeoutArg
? Math.ceil(keepAliveTimeoutArg)
: undefined

startServer({
dir,
hostname: host,
port,
keepAliveTimeout,
})
.then(async (app) => {
const appUrl = `http://${app.hostname}:${app.port}`
Expand Down
5 changes: 5 additions & 0 deletions packages/next/server/lib/start-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import next from '../next'

interface StartServerOptions extends NextServerOptions {
allowRetry?: boolean
keepAliveTimeout?: number
}

export function startServer(opts: StartServerOptions) {
Expand All @@ -14,6 +15,10 @@ export function startServer(opts: StartServerOptions) {
return requestHandler(req, res)
})

if (opts.keepAliveTimeout) {
server.keepAliveTimeout = opts.keepAliveTimeout
}

return new Promise<NextServer>((resolve, reject) => {
let port = opts.port
let retryCount = 0
Expand Down
48 changes: 48 additions & 0 deletions test/integration/cli/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,54 @@ describe('CLI Usage', () => {
'Invalid project directory provided, no such directory'
)
})

test('--keepAliveTimeout string arg', async () => {
const { stderr } = await runNextCommand(
['start', '--keepAliveTimeout', 'string'],
{
stderr: true,
}
)
expect(stderr).toContain(
'Invalid --keepAliveTimeout, expected a non negative number but received "NaN"'
)
})

test('--keepAliveTimeout negative number', async () => {
const { stderr } = await runNextCommand(
['start', '--keepAliveTimeout=-100'],
{
stderr: true,
}
)
expect(stderr).toContain(
'Invalid --keepAliveTimeout, expected a non negative number but received "-100"'
)
})

test('--keepAliveTimeout Infinity', async () => {
const { stderr } = await runNextCommand(
['start', '--keepAliveTimeout', 'Infinity'],
{
stderr: true,
}
)
expect(stderr).toContain(
'Invalid --keepAliveTimeout, expected a non negative number but received "Infinity"'
)
})

test('--keepAliveTimeout happy path', async () => {
const { stderr } = await runNextCommand(
['start', '--keepAliveTimeout', '100'],
{
stderr: true,
}
)
expect(stderr).not.toContain(
'Invalid keep alive timeout provided, expected a non negative number'
)
})
})

describe('export', () => {
Expand Down
34 changes: 34 additions & 0 deletions test/unit/cli.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { nextStart } from 'next/dist/cli/next-start'
import httpMock, { Server } from 'http'

// Prevents bin from running
jest.mock('next/dist/bin/next', () => ({}))
jest.mock('next/dist/lib/get-project-dir', () => ({ getProjectDir: () => '' }))

jest.mock('http')

describe('start', () => {
test('--keepAliveTimeout changes server.keepAliveTimeout when passed', () => {
const server = {
on: () => {},
listen: () => {},
} as any as Server
;(httpMock as any).createServer.mockReturnValue(server)

expect(server.keepAliveTimeout).toBe(undefined)
nextStart(['--keepAliveTimeout', '1234'])
expect(server.keepAliveTimeout).toBe(1234)
})

test("--keepAliveTimeout doesn't change server.keepAliveTimeout when not passed", () => {
const server = {
on: () => {},
listen: () => {},
} as any as Server
;(httpMock as any).createServer.mockReturnValue(server)

expect(server.keepAliveTimeout).toBe(undefined)
nextStart([])
expect(server.keepAliveTimeout).toBe(undefined)
})
})