From 3a3f6bada4bc85baa87d8d671919ca583c626a76 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Sun, 18 Sep 2022 19:27:52 +0900 Subject: [PATCH] process: runtime deprecate coercion to integer in `process.exit()` This is a follow up of doc-only deprecation https://github.com/nodejs/node/pull/43738. Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- doc/api/deprecations.md | 5 +- lib/internal/bootstrap/node.js | 25 +++++ lib/internal/process/per_thread.js | 35 ++++++ lib/internal/process/warning.js | 4 +- lib/internal/util.js | 7 +- .../test-process-exit-code-deprecation.js | 100 ++++++++++++++++++ 6 files changed, 171 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-process-exit-code-deprecation.js diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 2a218661d0ea00..2a26f80c2ee9ce 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3175,6 +3175,9 @@ thing instead. -Type: Documentation-only +Type: Runtime 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/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index f267d28d3821cd..18d23253b1c94e 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -102,6 +102,31 @@ process.domain = null; } process._exiting = false; +{ + const warnIntegerCoercionDeprecation = deprecate( + () => {}, + 'Implicit coercion to integer for exit code is deprecated.', + 'DEP0164' + ); + + let exitCode; + + ObjectDefineProperty(process, 'exitCode', { + __proto__: null, + get() { + return exitCode; + }, + set(code) { + if (perThreadSetup.isDeprecatedExitCode(code)) { + warnIntegerCoercionDeprecation(); + } + exitCode = code; + }, + enumerable: true, + configurable: false, + }); +} + // process.config is serialized config.gypi const nativeModule = internalBinding('builtins'); diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index c2be274659e6c7..40821b5ecd990c 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -13,7 +13,10 @@ const { ArrayPrototypeSplice, BigUint64Array, Float64Array, + Number, + NumberIsInteger, NumberMAX_SAFE_INTEGER, + NumberMIN_SAFE_INTEGER, ObjectFreeze, ObjectDefineProperty, ReflectApply, @@ -179,9 +182,23 @@ 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) process.exitCode = code; @@ -406,6 +423,23 @@ 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, @@ -414,4 +448,5 @@ module.exports = { hrtime, hrtimeBigInt, refreshHrtimeBuffer, + isDeprecatedExitCode, }; diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js index 9c2a98af058957..3ce00004dab476 100644 --- a/lib/internal/process/warning.js +++ b/lib/internal/process/warning.js @@ -166,8 +166,8 @@ function emitWarning(warning, type, code, ctor) { process.nextTick(doEmitWarning, warning); } -function emitWarningSync(warning) { - process.emit('warning', createWarningObject(warning)); +function emitWarningSync(warning, type, code, ctor) { + process.emit('warning', createWarningObject(warning, type, code, ctor)); } function createWarningObject(warning, type, code, ctor, detail) { diff --git a/lib/internal/util.js b/lib/internal/util.js index e250a798af7153..76d3fd9122383e 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -96,7 +96,7 @@ let validateString; // Mark that a method should not be used. // Returns a modified function which warns once by default. // If --no-deprecation is set, then it is a no-op. -function deprecate(fn, msg, code) { +function deprecate(fn, msg, code, useEmitSync) { if (process.noDeprecation === true) { return fn; } @@ -114,7 +114,10 @@ function deprecate(fn, msg, code) { warned = true; if (code !== undefined) { if (!codesWarned.has(code)) { - process.emitWarning(msg, 'DeprecationWarning', code, deprecated); + const emitWarning = useEmitSync ? + require('internal/process/warning').emitWarningSync : + process.emitWarning; + emitWarning(msg, 'DeprecationWarning', code, deprecated); codesWarned.add(code); } } else { diff --git a/test/parallel/test-process-exit-code-deprecation.js b/test/parallel/test-process-exit-code-deprecation.js new file mode 100644 index 00000000000000..17da793dfc2276 --- /dev/null +++ b/test/parallel/test-process-exit-code-deprecation.js @@ -0,0 +1,100 @@ +'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); + } +}