timers: extract enroll() validation into a fn

This should help keep everything consistent.

PR-URL: https://github.com/nodejs/node/pull/17704
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

lib/internal/timers.js

@@ -26,6 +26,7 @@ module.exports = {
   trigger_async_id_symbol,
   Timeout,
   setUnrefTimeout,
+  validateTimerDuration
 };
 
 // Timer constructor function.
@@ -105,3 +106,26 @@ function setUnrefTimeout(callback, after, arg1, arg2, arg3) {
 
   return timer;
 }
+
+// Type checking used by timers.enroll() and Socket#setTimeout()
+function validateTimerDuration(msecs) {
+  if (typeof msecs !== 'number') {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'msecs',
+                               'number', msecs);
+  }
+
+  if (msecs < 0 || !isFinite(msecs)) {
+    throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE', 'msecs',
+                                'a non-negative finite number', msecs);
+  }
+
+  // Ensure that msecs fits into signed int32
+  if (msecs > TIMEOUT_MAX) {
+    process.emitWarning(`${msecs} does not fit into a 32-bit signed integer.` +
+                        `\nTimer duration was truncated to ${TIMEOUT_MAX}.`,
+                        'TimeoutOverflowWarning');
+    return TIMEOUT_MAX;
+  }
+
+  return msecs;
+}

lib/net.js

@@ -57,7 +57,11 @@ var cluster = null;
 const errnoException = util._errnoException;
 const exceptionWithHostPort = util._exceptionWithHostPort;
 
-const { kTimeout, TIMEOUT_MAX, setUnrefTimeout } = require('internal/timers');
+const {
+  kTimeout,
+  setUnrefTimeout,
+  validateTimerDuration
+} = require('internal/timers');
 
 function noop() {}
 
@@ -394,23 +398,7 @@ Socket.prototype.read = function(n) {
 
 Socket.prototype.setTimeout = function(msecs, callback) {
   // Type checking identical to timers.enroll()
-  if (typeof msecs !== 'number') {
+  msecs = validateTimerDuration(msecs);
-    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'msecs',
-                               'number', msecs);
-  }
-
-  if (msecs < 0 || !isFinite(msecs)) {
-    throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE', 'msecs',
-                                'a non-negative finite number', msecs);
-  }
-
-  // Ensure that msecs fits into signed int32
-  if (msecs > TIMEOUT_MAX) {
-    process.emitWarning(`${msecs} does not fit into a 32-bit signed integer.` +
-                        `\nTimer duration was truncated to ${TIMEOUT_MAX}.`,
-                        'TimeoutOverflowWarning');
-    msecs = TIMEOUT_MAX;
-  }
 
   // Attempt to clear an existing timer lear in both cases -
   //  even if it will be rescheduled we don't want to leak an existing timer.

lib/timers.js

@@ -55,9 +55,6 @@ delete process._scheduledImmediateCount;
 const activateImmediateCheck = process._activateImmediateCheck;
 delete process._activateImmediateCheck;
 
-// Timeout values > TIMEOUT_MAX are set to 1.
-const TIMEOUT_MAX = timerInternals.TIMEOUT_MAX;
-
 // The Timeout class
 const Timeout = timerInternals.Timeout;
 
@@ -392,29 +389,12 @@ const unenroll = exports.unenroll = function(item) {
 // This function does not start the timer, see `active()`.
 // Using existing objects as timers slightly reduces object overhead.
 exports.enroll = function(item, msecs) {
-  if (typeof msecs !== 'number') {
+  item._idleTimeout = timerInternals.validateTimerDuration(msecs);
-    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'msecs',
-                               'number', msecs);
-  }
-
-  if (msecs < 0 || !isFinite(msecs)) {
-    throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE', 'msecs',
-                                'a non-negative finite number', msecs);
-  }
 
   // if this item was already in a list somewhere
   // then we should unenroll it from that
   if (item._idleNext) unenroll(item);
 
-  // Ensure that msecs fits into signed int32
-  if (msecs > TIMEOUT_MAX) {
-    process.emitWarning(`${msecs} does not fit into a 32-bit signed integer.` +
-                        `\nTimer duration was truncated to ${TIMEOUT_MAX}.`,
-                        'TimeoutOverflowWarning');
-    msecs = TIMEOUT_MAX;
-  }
-
-  item._idleTimeout = msecs;
   L.init(item);
 };