timers: warn on overflowed timeout duration

Cherry-pick from ayo Ayo commit log: > Previously there wasn't any clear indicator when you hit the overflow > other than possibly unexpected behavior, and I think emitting a warning > may be appropriate. > PR-URL: ayojs/ayo#71 > Reviewed-By: Scott Trinh <[email protected]> > Reviewed-By: Alexey Orlenko <[email protected]> > Reviewed-By: Stephen Belanger <[email protected]> > Reviewed-By: Anna Henningsen <[email protected]> > Reviewed-By: Benjamin Gruenbaum <[email protected]> PR-URL: #15627 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
nodejs · Sep 29, 2017 · 6af6964 · 6af6964
1 parent 347b6fc
commit 6af6964
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 2 deletions.
diff --git a/lib/timers.js b/lib/timers.js
@@ -405,6 +405,9 @@ exports.enroll = function(item, 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;
   }
 
@@ -449,8 +452,15 @@ exports.setTimeout = setTimeout;
 
 function createSingleTimeout(callback, after, args) {
   after *= 1; // coalesce to number or NaN
-  if (!(after >= 1 && after <= TIMEOUT_MAX))
+  if (!(after >= 1 && after <= TIMEOUT_MAX)) {
+    if (after > TIMEOUT_MAX) {
+      process.emitWarning(`${after} does not fit into` +
+                          ' a 32-bit signed integer.' +
+                          '\nTimeout duration was set to 1.',
+                          'TimeoutOverflowWarning');
+    }
     after = 1; // schedule on next tick, follows browser behavior
+  }
 
   var timer = new Timeout(after, callback, args);
   if (process.domain)
@@ -538,8 +548,15 @@ exports.setInterval = function(callback, repeat, arg1, arg2, arg3) {
 
 function createRepeatTimeout(callback, repeat, args) {
   repeat *= 1; // coalesce to number or NaN
-  if (!(repeat >= 1 && repeat <= TIMEOUT_MAX))
+  if (!(repeat >= 1 && repeat <= TIMEOUT_MAX)) {
+    if (repeat > TIMEOUT_MAX) {
+      process.emitWarning(`${repeat} does not fit into` +
+                          ' a 32-bit signed integer.' +
+                          '\nInterval duration was set to 1.',
+                          'TimeoutOverflowWarning');
+    }
     repeat = 1; // schedule on next tick, follows browser behavior
+  }
 
   var timer = new Timeout(repeat, callback, args);
   timer._repeat = repeat;

diff --git a/test/parallel/test-timers-max-duration-warning.js b/test/parallel/test-timers-max-duration-warning.js
@@ -0,0 +1,40 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const timers = require('timers');
+
+const OVERFLOW = Math.pow(2, 31); // TIMEOUT_MAX is 2^31-1
+
+function timerNotCanceled() {
+  common.fail('Timer should be canceled');
+}
+
+process.on('warning', common.mustCall((warning) => {
+  const lines = warning.message.split('\n');
+
+  assert.strictEqual(warning.name, 'TimeoutOverflowWarning');
+  assert.strictEqual(lines[0], `${OVERFLOW} does not fit into a 32-bit signed` +
+                               ' integer.');
+  assert.strictEqual(lines.length, 2);
+}, 3));
+
+
+{
+  const timeout = setTimeout(timerNotCanceled, OVERFLOW);
+  clearTimeout(timeout);
+}
+
+{
+  const interval = setInterval(timerNotCanceled, OVERFLOW);
+  clearInterval(interval);
+}
+
+{
+  const timer = {
+    _onTimeout: timerNotCanceled
+  };
+  timers.enroll(timer, OVERFLOW);
+  timers.active(timer);
+  timers.unenroll(timer);
+}