process: runtime deprecate coercion to integer in process.exit()

This is a follow up of doc-only deprecation nodejs#43738. Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
daeyeon · Sep 30, 2022 · 3a3f6ba · 3a3f6ba
1 parent 2a4452a
commit 3a3f6ba
Show file tree

Hide file tree

Showing 6 changed files with 171 additions and 5 deletions.
diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
@@ -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
@@ -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

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);
+  }
+}