Skip to content

Commit

Permalink
process: runtime deprecate coercion to integer in process.exit()
Browse files Browse the repository at this point in the history
This is a follow up of doc-only deprecation
#43738.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
PR-URL: #44711
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
daeyeon committed Oct 7, 2022
1 parent 24d255f commit ada2d05
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 5 deletions.
5 changes: 4 additions & 1 deletion doc/api/deprecations.md
Expand Up @@ -3175,6 +3175,9 @@ thing instead.

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/44711
description: Runtime deprecation.
- version: v18.10.0
pr-url: https://github.com/nodejs/node/pull/44714
description: Documentation-only deprecation of `process.exitCode` integer
Expand All @@ -3187,7 +3190,7 @@ changes:
coercion.
-->

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
Expand Down
25 changes: 25 additions & 0 deletions lib/internal/bootstrap/node.js
Expand Up @@ -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');

Expand Down
35 changes: 35 additions & 0 deletions lib/internal/process/per_thread.js
Expand Up @@ -13,7 +13,10 @@ const {
ArrayPrototypeSplice,
BigUint64Array,
Float64Array,
Number,
NumberIsInteger,
NumberMAX_SAFE_INTEGER,
NumberMIN_SAFE_INTEGER,
ObjectFreeze,
ObjectDefineProperty,
ReflectApply,
Expand Down Expand Up @@ -180,9 +183,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;

Expand Down Expand Up @@ -407,6 +424,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,
Expand All @@ -415,4 +449,5 @@ module.exports = {
hrtime,
hrtimeBigInt,
refreshHrtimeBuffer,
isDeprecatedExitCode,
};
4 changes: 2 additions & 2 deletions lib/internal/process/warning.js
Expand Up @@ -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) {
Expand Down
7 changes: 5 additions & 2 deletions lib/internal/util.js
Expand Up @@ -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;
}
Expand All @@ -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 {
Expand Down
100 changes: 100 additions & 0 deletions 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);
}
}

0 comments on commit ada2d05

Please sign in to comment.