From 9135a6ad918e09d32d2c0d4a19d62427ea2a9c9a Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Thu, 7 Jul 2022 20:59:10 +0900 Subject: [PATCH 1/3] process: remove `process.exit()`, `process.exitCode` coercion to integer This removes the deprecation, `DEP0164` and validates the exit code for both `process.exit([code])` and `process.exitCode`. Signed-off-by: Daeyeon Jeong --- doc/api/process.md | 16 ++- lib/internal/bootstrap/node.js | 17 +-- lib/internal/process/per_thread.js | 38 +---- .../test-process-exit-code-validation.js | 132 ++++++++++++++++++ ...orker-terminate-http2-respond-with-file.js | 2 +- 5 files changed, 158 insertions(+), 47 deletions(-) create mode 100644 test/parallel/test-process-exit-code-validation.js diff --git a/doc/api/process.md b/doc/api/process.md index c89cc9b5f99c4b..015bdfd88adad1 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -1708,9 +1708,15 @@ that started the Node.js process. Symbolic links, if any, are resolved. -* `code` {integer} The exit code. **Default:** `0`. +* `code` {integer|string|null|undefined} The exit code. For string type, only + integer strings (e.g.,'1') are allowed. **Default:** `0`. The `process.exit()` method instructs Node.js to terminate the process synchronously with an exit status of `code`. If `code` is omitted, exit uses @@ -1810,9 +1816,15 @@ than the current process. -* {integer} +* {integer|string|null|undefined} The exit code. For string type, only + integer strings (e.g.,'1') are allowed. **Default:** `undefined`. A number which will be the process exit code, when the process either exits gracefully, or is exited via [`process.exit()`][] without specifying diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 18d23253b1c94e..2499f455c772bd 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -60,6 +60,7 @@ const { ArrayPrototypeFill, FunctionPrototypeCall, JSONParse, + Number, ObjectDefineProperty, ObjectGetPrototypeOf, ObjectSetPrototypeOf, @@ -74,6 +75,9 @@ const { deprecate, exposeInterface, } = require('internal/util'); +const { + validateInteger, +} = require('internal/validators'); const { exiting_aliased_Uint32Array, getHiddenValue, @@ -103,12 +107,6 @@ process.domain = null; process._exiting = false; { - const warnIntegerCoercionDeprecation = deprecate( - () => {}, - 'Implicit coercion to integer for exit code is deprecated.', - 'DEP0164' - ); - let exitCode; ObjectDefineProperty(process, 'exitCode', { @@ -117,8 +115,11 @@ process._exiting = false; return exitCode; }, set(code) { - if (perThreadSetup.isDeprecatedExitCode(code)) { - warnIntegerCoercionDeprecation(); + if (code !== null && code !== undefined) { + validateInteger( + typeof code === 'string' && code !== '' ? Number(code) : code, + 'code' + ); } exitCode = code; }, diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index 65136e701fd20d..b99a836e53ad87 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -13,10 +13,7 @@ const { ArrayPrototypeSplice, BigUint64Array, Float64Array, - Number, - NumberIsInteger, NumberMAX_SAFE_INTEGER, - NumberMIN_SAFE_INTEGER, ObjectFreeze, ObjectDefineProperty, ReflectApply, @@ -183,25 +180,12 @@ function wrapProcessMethods(binding) { memoryUsage.rss = rss; - const { deprecate } = require('internal/util'); - const warnIntegerCoercionDeprecationSync = deprecate( - () => {}, - 'Implicit coercion to integer for exit code is deprecated.', - 'DEP0164', - true - ); - function exit(code) { process.off('exit', handleProcessExit); - if (isDeprecatedExitCode(code)) { - // Emit the deprecation warning synchronously since deprecation warning is - // generally emitted in a next tick but we have no next tick timing here. - warnIntegerCoercionDeprecationSync(); - } - - if (code || code === 0) + if (arguments.length !== 0) { process.exitCode = code; + } if (!process._exiting) { process._exiting = true; @@ -424,23 +408,6 @@ function toggleTraceCategoryState(asyncHooksEnabled) { } } -function isDeprecatedExitCode(code) { - if (code !== null && code !== undefined) { - const value = - typeof code === 'string' && code !== '' ? Number(code) : code; - // Check if the value is an integer. - if ( - typeof value !== 'number' || - !NumberIsInteger(value) || - value < NumberMIN_SAFE_INTEGER || - value > NumberMAX_SAFE_INTEGER - ) { - return true; - } - } - return false; -} - module.exports = { toggleTraceCategoryState, assert, @@ -449,5 +416,4 @@ module.exports = { hrtime, hrtimeBigInt, refreshHrtimeBuffer, - isDeprecatedExitCode, }; diff --git a/test/parallel/test-process-exit-code-validation.js b/test/parallel/test-process-exit-code-validation.js new file mode 100644 index 00000000000000..490becd20bc1b2 --- /dev/null +++ b/test/parallel/test-process-exit-code-validation.js @@ -0,0 +1,132 @@ +'use strict'; + +require('../common'); + +const invalids = [ + { + code: '', + expected: 1, + }, + { + code: '1 one', + expected: 1, + }, + { + code: 'two', + expected: 1, + }, + { + code: {}, + expected: 1, + }, + { + code: [], + expected: 1, + }, + { + code: true, + expected: 1, + }, + { + code: false, + expected: 1, + }, + { + code: 2n, + expected: 1, + }, + { + code: 2.1, + expected: 1, + }, + { + code: Infinity, + expected: 1, + }, + { + code: NaN, + expected: 1, + }, +]; +const valids = [ + { + code: 1, + expected: 1, + }, + { + code: '2', + expected: 2, + }, + { + code: undefined, + expected: 0, + }, + { + code: null, + expected: 0, + }, + { + code: 0, + expected: 0, + }, + { + code: '0', + expected: 0, + }, +]; +const args = [...invalids, ...valids]; + +if (process.argv[2] === undefined) { + const { spawnSync } = require('node:child_process'); + const { inspect, debuglog } = require('node:util'); + const { throws, strictEqual } = require('node:assert'); + + const debug = debuglog('test'); + const node = process.execPath; + const test = (index, useProcessExitCode) => { + const { status: code } = spawnSync(node, [ + __filename, + index, + useProcessExitCode, + ]); + debug(`actual: ${code}, ${inspect(args[index])} ${!!useProcessExitCode}`); + strictEqual( + code, + args[index].expected, + `actual: ${code}, ${inspect(args[index])}` + ); + }; + + // Check process.exitCode + for (const arg of invalids) { + debug(`invaild code: ${inspect(arg.code)}`); + throws(() => (process.exitCode = arg.code), Error); + } + for (const arg of valids) { + debug(`vaild code: ${inspect(arg.code)}`); + process.exitCode = arg.code; + } + + throws(() => { + delete process.exitCode; + }, /Cannot delete property 'exitCode' of #/); + process.exitCode = 0; + + // Check process.exit([code]) + for (const index of args.keys()) { + test(index); + test(index, true); + } +} else { + const index = parseInt(process.argv[2]); + const useProcessExitCode = process.argv[3] !== 'undefined'; + if (Number.isNaN(index)) { + return process.exit(100); + } + + if (useProcessExitCode) { + process.exitCode = args[index].code; + } else { + process.exit(args[index].code); + } +} diff --git a/test/parallel/test-worker-terminate-http2-respond-with-file.js b/test/parallel/test-worker-terminate-http2-respond-with-file.js index 80b58ddfc5bac0..8fa8602330fcfe 100644 --- a/test/parallel/test-worker-terminate-http2-respond-with-file.js +++ b/test/parallel/test-worker-terminate-http2-respond-with-file.js @@ -33,7 +33,7 @@ if (isMainThread) { assert.strictEqual(headers[':status'], 200); })); - req.on('data', common.mustCall(process.exit)); + req.on('data', common.mustCall(() => process.exit())); req.on('end', common.mustNotCall()); req.end(); } From 48a992fd7a081e209ede546f993a5dd9c5a78298 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Tue, 11 Oct 2022 00:40:29 +0900 Subject: [PATCH 2/3] fixup: fix error message Signed-off-by: Daeyeon Jeong --- lib/internal/bootstrap/node.js | 11 +++++++---- test/parallel/test-process-exit-code-validation.js | 13 ++++++++++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 2499f455c772bd..571888ebc8d6cb 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -61,6 +61,7 @@ const { FunctionPrototypeCall, JSONParse, Number, + NumberIsNaN, ObjectDefineProperty, ObjectGetPrototypeOf, ObjectSetPrototypeOf, @@ -116,10 +117,12 @@ process._exiting = false; }, set(code) { if (code !== null && code !== undefined) { - validateInteger( - typeof code === 'string' && code !== '' ? Number(code) : code, - 'code' - ); + let value = code; + if (typeof code === 'string' && code !== '' && + NumberIsNaN((value = Number(code)))) { + value = code; + } + validateInteger(value, 'code'); } exitCode = code; }, diff --git a/test/parallel/test-process-exit-code-validation.js b/test/parallel/test-process-exit-code-validation.js index 490becd20bc1b2..9987b58c867c99 100644 --- a/test/parallel/test-process-exit-code-validation.js +++ b/test/parallel/test-process-exit-code-validation.js @@ -6,46 +6,57 @@ const invalids = [ { code: '', expected: 1, + pattern: "Received type string \\(''\\)$", }, { code: '1 one', expected: 1, + pattern: "Received type string \\('1 one'\\)$", }, { code: 'two', expected: 1, + pattern: "Received type string \\('two'\\)$", }, { code: {}, expected: 1, + pattern: 'Received an instance of Object$', }, { code: [], expected: 1, + pattern: 'Received an instance of Array$', }, { code: true, expected: 1, + pattern: 'Received type boolean \\(true\\)$', }, { code: false, expected: 1, + pattern: 'Received type boolean \\(false\\)$', }, { code: 2n, expected: 1, + pattern: 'Received type bigint \\(2n\\)$', }, { code: 2.1, expected: 1, + pattern: 'Received 2.1$', }, { code: Infinity, expected: 1, + pattern: 'Received Infinity$', }, { code: NaN, expected: 1, + pattern: 'Received NaN$', }, ]; const valids = [ @@ -100,7 +111,7 @@ if (process.argv[2] === undefined) { // Check process.exitCode for (const arg of invalids) { debug(`invaild code: ${inspect(arg.code)}`); - throws(() => (process.exitCode = arg.code), Error); + throws(() => (process.exitCode = arg.code), new RegExp(arg.pattern)); } for (const arg of valids) { debug(`vaild code: ${inspect(arg.code)}`); From 7b226e0fec3a334c9a3f46caec2007d804d8c5ce Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Tue, 11 Oct 2022 00:53:21 +0900 Subject: [PATCH 3/3] fixup: update the doc and remove the test Signed-off-by: Daeyeon Jeong --- doc/api/deprecations.md | 5 +- .../test-process-exit-code-deprecation.js | 100 ------------------ 2 files changed, 4 insertions(+), 101 deletions(-) delete mode 100644 test/parallel/test-process-exit-code-deprecation.js diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 123cceadd300f0..8f130ca7a9fb92 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3178,6 +3178,9 @@ thing instead. -Type: Runtime +Type: End-of-Life Values other than `undefined`, `null`, integer numbers, and integer strings (e.g., `'1'`) are deprecated as value for the `code` parameter in diff --git a/test/parallel/test-process-exit-code-deprecation.js b/test/parallel/test-process-exit-code-deprecation.js deleted file mode 100644 index 17da793dfc2276..00000000000000 --- a/test/parallel/test-process-exit-code-deprecation.js +++ /dev/null @@ -1,100 +0,0 @@ -'use strict'; - -require('../common'); - -const deprecated = [ - { - code: '', - expected: 0, - }, - { - code: '1 one', - expected: 0, - }, - { - code: 'two', - expected: 0, - }, - { - code: {}, - expected: 0, - }, - { - code: [], - expected: 0, - }, - { - code: true, - expected: 1, - }, - { - code: false, - expected: 0, - }, - { - code: 2n, - expected: 0, - expected_useProcessExitCode: 1, - }, - { - code: 2.1, - expected: 2, - }, - { - code: Infinity, - expected: 0, - }, - { - code: NaN, - expected: 0, - }, -]; -const args = deprecated; - -if (process.argv[2] === undefined) { - const { spawnSync } = require('node:child_process'); - const { inspect, debuglog } = require('node:util'); - const { strictEqual } = require('node:assert'); - - const debug = debuglog('test'); - const node = process.execPath; - const test = (index, useProcessExitCode) => { - const { status: code, stderr } = spawnSync(node, [ - __filename, - index, - useProcessExitCode, - ]); - debug(`actual: ${code}, ${inspect(args[index])} ${!!useProcessExitCode}`); - debug(`${stderr}`); - - const expected = - useProcessExitCode && args[index].expected_useProcessExitCode ? - args[index].expected_useProcessExitCode : - args[index].expected; - - strictEqual(code, expected, `actual: ${code}, ${inspect(args[index])}`); - strictEqual( - ['[DEP0164]'].some((pattern) => stderr.includes(pattern)), - true - ); - }; - - for (const index of args.keys()) { - // Check `process.exit([code])` - test(index); - // Check exit with `process.exitCode` - test(index, true); - } -} else { - const index = parseInt(process.argv[2]); - const useProcessExitCode = process.argv[3] !== 'undefined'; - if (Number.isNaN(index)) { - return process.exit(100); - } - - if (useProcessExitCode) { - process.exitCode = args[index].code; - } else { - process.exit(args[index].code); - } -}