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

Ensure next commands respect termination signals #19433

Merged
merged 8 commits into from Nov 25, 2020

Conversation

jamsinclair
Copy link
Contributor

@jamsinclair jamsinclair commented Nov 23, 2020

Fixes #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 #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?

@vercel vercel bot temporarily deployed to Preview November 23, 2020 08:42 Inactive
@ijjk
Copy link
Member

ijjk commented Nov 23, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 9.2s 9.2s ⚠️ +35ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Page Load Tests Overall increase ✓
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
/ failed reqs 0 0
/ total time (seconds) 2.066 2.044 -0.02
/ avg req/sec 1210.18 1223.02 +12.84
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.244 1.215 -0.03
/error-in-render avg req/sec 2009.1 2057.95 +48.85
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-9cc22a8..b04f.js gzip 6.53 kB 6.53 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Rendered Page Sizes
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
index.html gzip 615 B 615 B
link.html gzip 622 B 622 B
withRouter.html gzip 609 B 609 B
Overall change 1.85 kB 1.85 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 11s 10.6s -372ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-9cc22a8..b04f.js gzip 6.53 kB 6.53 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Serverless bundles
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_error.js 915 kB 915 kB
404.html 2.67 kB 2.67 kB
hooks.html 1.92 kB 1.92 kB
index.js 915 kB 915 kB
link.js 973 kB 973 kB
routerDirect.js 966 kB 966 kB
withRouter.js 966 kB 966 kB
Overall change 4.74 MB 4.74 MB
Commit: 1028b6f

@timneutkens
Copy link
Member

test/integration/cli would be the right place to add tests indeed!

@vercel vercel bot temporarily deployed to Preview November 23, 2020 23:30 Inactive
@ijjk
Copy link
Member

ijjk commented Nov 23, 2020

Failing test suites

Commit: 15761a5

test/integration/cli/test/index.test.js

  • CLI Usage > dev > should exit with code 0 when SIGINT is signalled
  • CLI Usage > dev > should exit with code 0 when SIGTERM is signalled
  • CLI Usage > start > should exit with code 0 when SIGINT is signalled
  • CLI Usage > start > should exit with code 0 when SIGTERM is signalled
Expand output

● CLI Usage › dev › should exit with code 0 when SIGINT is signalled

command failed with code null

  138 |         code !== 0
  139 |       ) {
> 140 |         return reject(new Error(`command failed with code ${code}`))
      |                       ^
  141 |       }
  142 | 
  143 |       resolve({

  at ChildProcess.<anonymous> (lib/next-test-utils.js:140:23)

● CLI Usage › dev › should exit with code 0 when SIGTERM is signalled

command failed with code null

  138 |         code !== 0
  139 |       ) {
> 140 |         return reject(new Error(`command failed with code ${code}`))
      |                       ^
  141 |       }
  142 | 
  143 |       resolve({

  at ChildProcess.<anonymous> (lib/next-test-utils.js:140:23)

● CLI Usage › start › should exit with code 0 when SIGINT is signalled

command failed with code null

  138 |         code !== 0
  139 |       ) {
> 140 |         return reject(new Error(`command failed with code ${code}`))
      |                       ^
  141 |       }
  142 | 
  143 |       resolve({

  at ChildProcess.<anonymous> (lib/next-test-utils.js:140:23)

● CLI Usage › start › should exit with code 0 when SIGTERM is signalled

command failed with code null

  138 |         code !== 0
  139 |       ) {
> 140 |         return reject(new Error(`command failed with code ${code}`))
      |                       ^
  141 |       }
  142 | 
  143 |       resolve({

  at ChildProcess.<anonymous> (lib/next-test-utils.js:140:23)

@ijjk
Copy link
Member

ijjk commented Nov 23, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 9.9s 10.2s ⚠️ +247ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
/ failed reqs 0 0
/ total time (seconds) 2.2 2.308 ⚠️ +0.11
/ avg req/sec 1136.56 1083.29 ⚠️ -53.27
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.272 1.256 -0.02
/error-in-render avg req/sec 1966.07 1989.66 +23.59
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-0103832..fd40.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Rendered Page Sizes
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
index.html gzip 615 B 615 B
link.html gzip 622 B 622 B
withRouter.html gzip 609 B 609 B
Overall change 1.85 kB 1.85 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 11.6s 11.7s ⚠️ +119ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-0103832..fd40.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Serverless bundles
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_error.js 915 kB 915 kB
404.html 2.67 kB 2.67 kB
hooks.html 1.92 kB 1.92 kB
index.js 915 kB 915 kB
link.js 973 kB 973 kB
routerDirect.js 966 kB 966 kB
withRouter.js 966 kB 966 kB
Overall change 4.74 MB 4.74 MB
Commit: 15761a5

@vercel vercel bot temporarily deployed to Preview November 24, 2020 00:12 Inactive
@ijjk
Copy link
Member

ijjk commented Nov 24, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 10.1s 9.2s -850ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Page Load Tests Overall increase ✓
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
/ failed reqs 0 0
/ total time (seconds) 2.238 2.072 -0.17
/ avg req/sec 1116.94 1206.84 +89.9
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.232 1.169 -0.06
/error-in-render avg req/sec 2028.55 2139.14 +110.59
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-0103832..fd40.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Rendered Page Sizes
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
index.html gzip 615 B 615 B
link.html gzip 622 B 622 B
withRouter.html gzip 609 B 609 B
Overall change 1.85 kB 1.85 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 11.6s 12s ⚠️ +460ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-0103832..fd40.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Serverless bundles
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_error.js 915 kB 915 kB
404.html 2.67 kB 2.67 kB
hooks.html 1.92 kB 1.92 kB
index.js 915 kB 915 kB
link.js 973 kB 973 kB
routerDirect.js 966 kB 966 kB
withRouter.js 966 kB 966 kB
Overall change 4.74 MB 4.74 MB
Commit: e5febc2

@jamsinclair
Copy link
Contributor Author

jamsinclair commented Nov 24, 2020

Looks like the Azure tests are failing. Not sure if a timing issue or because the Azure environment doesn't support Unix signals. I'll look into later. (edit: looks to be timing related, it's a shame these tests are inherently flaky.)

@vercel vercel bot temporarily deployed to Preview November 24, 2020 02:29 Inactive
@vercel vercel bot temporarily deployed to Preview November 24, 2020 02:35 Inactive
@ijjk
Copy link
Member

ijjk commented Nov 24, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 8.4s 8.5s ⚠️ +121ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Page Load Tests Overall increase ✓
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
/ failed reqs 0 0
/ total time (seconds) 1.918 1.913 0
/ avg req/sec 1303.58 1306.59 +3.01
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.122 1.098 -0.02
/error-in-render avg req/sec 2227.32 2276.92 +49.6
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-0103832..fd40.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Rendered Page Sizes
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
index.html gzip 615 B 615 B
link.html gzip 622 B 622 B
withRouter.html gzip 609 B 609 B
Overall change 1.85 kB 1.85 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 9.9s 9.8s -83ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-0103832..fd40.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Serverless bundles
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_error.js 915 kB 915 kB
404.html 2.67 kB 2.67 kB
hooks.html 1.92 kB 1.92 kB
index.js 915 kB 915 kB
link.js 973 kB 973 kB
routerDirect.js 966 kB 966 kB
withRouter.js 966 kB 966 kB
Overall change 4.74 MB 4.74 MB
Commit: b5615b5

@ijjk

This comment has been minimized.

@ijjk
Copy link
Member

ijjk commented Nov 24, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 8.9s 9.1s ⚠️ +207ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
/ failed reqs 0 0
/ total time (seconds) 2.066 2.097 ⚠️ +0.03
/ avg req/sec 1210.31 1191.9 ⚠️ -18.41
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.365 1.409 ⚠️ +0.04
/error-in-render avg req/sec 1831.25 1774.82 ⚠️ -56.43
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-0103832..fd40.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Rendered Page Sizes
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
index.html gzip 615 B 615 B
link.html gzip 622 B 622 B
withRouter.html gzip 609 B 609 B
Overall change 1.85 kB 1.85 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 10.3s 10.2s -84ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-0103832..fd40.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Serverless bundles
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_error.js 915 kB 915 kB
404.html 2.67 kB 2.67 kB
hooks.html 1.92 kB 1.92 kB
index.js 915 kB 915 kB
link.js 973 kB 973 kB
routerDirect.js 966 kB 966 kB
withRouter.js 966 kB 966 kB
Overall change 4.74 MB 4.74 MB
Commit: 8a8d912

@timneutkens
Copy link
Member

timneutkens commented Nov 24, 2020

Hmm yeah we've had some issues with Azure Pipelines (Windows) tests before. @Timer @ijjk might have thoughts on it 👍 Sounds kind of expected that it fails on Windows though given it's UNIX termination signals like you said.

Edit: did some research and found this post: https://stackoverflow.com/questions/10021373/what-is-the-windows-equivalent-of-process-onsigint-in-node-js which suggests Node.js on Windows interops the signals

@vercel vercel bot temporarily deployed to Preview November 24, 2020 15:53 Inactive
@jamsinclair
Copy link
Contributor Author

@timneutkens Thanks for taking a look 😄, your link lead me down a rabbit hole 🐰. This has been a fun one.

According to nodejs/node-v0.x-archive#8713 (comment) and the current node documentation for signals.

[Windows] Sending SIGINT, SIGTERM, and SIGKILL will cause the unconditional termination of the target process, and afterwards, subprocess will report that the process was terminated by signal.

So the signals do cause termination but our process cannot handle the signals internally. With that in mind, would we be okay with conditional test logic for platforms? I've updated the branch to give you an idea what that could look like.

@ijjk
Copy link
Member

ijjk commented Nov 24, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 10.2s 10.4s ⚠️ +235ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Page Load Tests Overall increase ✓
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
/ failed reqs 0 0
/ total time (seconds) 2.251 2.237 -0.01
/ avg req/sec 1110.48 1117.79 +7.31
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.269 1.255 -0.01
/error-in-render avg req/sec 1970.29 1992.11 +21.82
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-fbd7082..8b7a.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Rendered Page Sizes
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
index.html gzip 614 B 614 B
link.html gzip 621 B 621 B
withRouter.html gzip 608 B 608 B
Overall change 1.84 kB 1.84 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 12s 11.8s -208ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-fbd7082..8b7a.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Serverless bundles
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_error.js 915 kB 915 kB
404.html 2.67 kB 2.67 kB
hooks.html 1.92 kB 1.92 kB
index.js 915 kB 915 kB
link.js 973 kB 973 kB
routerDirect.js 966 kB 966 kB
withRouter.js 966 kB 966 kB
Overall change 4.74 MB 4.74 MB
Commit: 05b6189

@timneutkens
Copy link
Member

So the signals do cause termination but our process cannot handle the signals internally. With that in mind, would we be okay with conditional test logic for platforms? I've updated the branch to give you an idea what that could look like.

Yeah definitely fine to conditionally run it 👍

@vercel vercel bot temporarily deployed to Preview November 25, 2020 10:44 Inactive
@ijjk
Copy link
Member

ijjk commented Nov 25, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 10.6s 10.3s -264ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
/ failed reqs 0 0
/ total time (seconds) 2.244 2.279 ⚠️ +0.03
/ avg req/sec 1113.95 1096.74 ⚠️ -17.21
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.297 1.317 ⚠️ +0.02
/error-in-render avg req/sec 1927.17 1898.12 ⚠️ -29.05
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-fbd7082..8b7a.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Rendered Page Sizes
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
index.html gzip 614 B 614 B
link.html gzip 621 B 621 B
withRouter.html gzip 608 B 608 B
Overall change 1.84 kB 1.84 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 11.9s 11.9s ⚠️ +60ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-fbd7082..8b7a.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Serverless bundles
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_error.js 915 kB 915 kB
404.html 2.67 kB 2.67 kB
hooks.html 1.92 kB 1.92 kB
index.js 915 kB 915 kB
link.js 973 kB 973 kB
routerDirect.js 966 kB 966 kB
withRouter.js 966 kB 966 kB
Overall change 4.74 MB 4.74 MB
Commit: 927a134

Copy link
Member

@timneutkens timneutkens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR @jamsinclair 💯

@vercel vercel bot temporarily deployed to Preview November 25, 2020 13:09 Inactive
@ijjk
Copy link
Member

ijjk commented Nov 25, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 8.8s 9.1s ⚠️ +304ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
/ failed reqs 0 0
/ total time (seconds) 1.979 2.017 ⚠️ +0.04
/ avg req/sec 1263.21 1239.51 ⚠️ -23.7
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.159 1.207 ⚠️ +0.05
/error-in-render avg req/sec 2156.33 2071.58 ⚠️ -84.75
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-fbd7082..8b7a.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Rendered Page Sizes
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
index.html gzip 614 B 614 B
link.html gzip 621 B 621 B
withRouter.html gzip 608 B 608 B
Overall change 1.84 kB 1.84 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
buildDuration 10.4s 10.4s -17ms
nodeModulesSize 84.9 MB 84.9 MB ⚠️ +666 B
Client Bundles (main, webpack, commons)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
677f882d2ed8..8b81.js gzip 12.8 kB 12.8 kB
framework.HASH.js gzip 39 kB 39 kB
main-fbd7082..8b7a.js gzip 6.54 kB 6.54 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 59 kB 59 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB
_error-6f635..c393.js gzip 3.44 kB 3.44 kB
hooks-d4ffc3..9e0f.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Serverless bundles
vercel/next.js canary jamsinclair/next.js fix/safe-shutdown-2 Change
_error.js 915 kB 915 kB
404.html 2.67 kB 2.67 kB
hooks.html 1.92 kB 1.92 kB
index.js 915 kB 915 kB
link.js 973 kB 973 kB
routerDirect.js 966 kB 966 kB
withRouter.js 966 kB 966 kB
Overall change 4.74 MB 4.74 MB
Commit: 1454bf0

@kodiakhq kodiakhq bot merged commit 397a375 into vercel:canary Nov 25, 2020
1 check passed
@NMinhNguyen
Copy link
Contributor

Sorry to be commenting on a merged PR, but if I'm not mistaken, this would shut down the process straight away and thus not allow in-flight requests to complete (see #90)? Cloud providers usually give you a few seconds to respond to a SIGTERM and finish ongoing requests:

@jamsinclair
Copy link
Contributor Author

jamsinclair commented Dec 2, 2020

@NMinhNguyen Great point. If I'm not mistaken, this PR doesn't change the current behaviour. As previously the processes would be shutdown non-gracefully after a timeout by cloud providers as the SIGTERM would be ignored by Next.

I'll let the maintainers discuss 😅 , but I think new work would need to be done to expose the server instance in the cli so we can gracefully close it?

In the interim, you can still implement graceful shutdown manually via a custom server, https://nextjs.org/docs/advanced-features/custom-server.

@NMinhNguyen
Copy link
Contributor

NMinhNguyen commented Dec 2, 2020

@NMinhNguyen Great point. If I'm not mistaken, this PR doesn't change the current behaviour.

Ah yes, you're right :) I was just looking for occurrences of SIGTERM in the codebase and stumbled upon your PR. From what I understand, the process would've exited straight away as well, except with the wrong status code 1 which this PR fixed. And thanks for the tip regarding a custom server, I was just trying to avoid it as much as possible 😅

kamermans pushed a commit to kamermans/next.js that referenced this pull request Dec 14, 2020
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)?
@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle SIGTERM to allow graceful process exit
4 participants