diff --git a/doc/api/process.md b/doc/api/process.md index b17739eeb5b..0dcfed70f94 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -676,6 +676,10 @@ A few of the warning types that are most common include: * `'TimeoutOverflowWarning'` - Indicates that a numeric value that cannot fit within a 32-bit signed integer has been provided to either the `setTimeout()` or `setInterval()` functions. +* `'TimeoutNegativeWarning'` - Indicates that a negative number has provided to + either the `setTimeout()` or `setInterval()` functions. +* `'TimeoutNaNWarning'` - Indicates that a value which is not a number has + provided to either the `setTimeout()` or `setInterval()` functions. * `'UnsupportedWarning'` - Indicates use of an unsupported option or feature that will be ignored rather than treated as an error. One example is use of the HTTP response status message when using the HTTP/2 compatibility API. diff --git a/doc/api/timers.md b/doc/api/timers.md index 3789e9efd00..fd459e2a007 100644 --- a/doc/api/timers.md +++ b/doc/api/timers.md @@ -239,8 +239,8 @@ changes: Schedules repeated execution of `callback` every `delay` milliseconds. -When `delay` is larger than `2147483647` or less than `1`, the `delay` will be -set to `1`. Non-integer delays are truncated to an integer. +When `delay` is larger than `2147483647` or less than `1` or `NaN`, the `delay` +will be set to `1`. Non-integer delays are truncated to an integer. If `callback` is not a function, a [`TypeError`][] will be thrown. @@ -272,7 +272,7 @@ Node.js makes no guarantees about the exact timing of when callbacks will fire, nor of their ordering. The callback will be called as close as possible to the time specified. -When `delay` is larger than `2147483647` or less than `1`, the `delay` +When `delay` is larger than `2147483647` or less than `1` or `NaN`, the `delay` will be set to `1`. Non-integer delays are truncated to an integer. If `callback` is not a function, a [`TypeError`][] will be thrown. diff --git a/lib/internal/timers.js b/lib/internal/timers.js index 5c56cf10635..01800556611 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -76,6 +76,7 @@ const { MathMax, MathTrunc, NumberIsFinite, + NumberIsNaN, NumberMIN_SAFE_INTEGER, ReflectApply, Symbol, @@ -164,17 +165,36 @@ function initAsyncResource(resource, type) { if (initHooksExist()) emitInit(asyncId, type, triggerAsyncId, resource); } + +let warnedNegativeNumber = false; +let warnedNotNumber = false; + class Timeout { // Timer constructor function. // The entire prototype is defined in lib/timers.js constructor(callback, after, args, isRepeat, isRefed) { - after *= 1; // Coalesce to number or NaN + if (after === undefined) { + after = 1; + } else { + after *= 1; // Coalesce to number or NaN + } + 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'); + } else if (after < 0 && !warnedNegativeNumber) { + warnedNegativeNumber = true; + process.emitWarning(`${after} is a negative number.` + + '\nTimeout duration was set to 1.', + 'TimeoutNegativeWarning'); + } else if (NumberIsNaN(after) && !warnedNotNumber) { + warnedNotNumber = true; + process.emitWarning(`${after} is not a number.` + + '\nTimeout duration was set to 1.', + 'TimeoutNaNWarning'); } after = 1; // Schedule on next tick, follows browser behavior } diff --git a/test/fixtures/source-map/output/source_map_throw_set_immediate.snapshot b/test/fixtures/source-map/output/source_map_throw_set_immediate.snapshot index d2d838ca35a..ee3294a2871 100644 --- a/test/fixtures/source-map/output/source_map_throw_set_immediate.snapshot +++ b/test/fixtures/source-map/output/source_map_throw_set_immediate.snapshot @@ -6,6 +6,6 @@ Error: goodbye at Hello (*uglify-throw-original.js:5:9) at Immediate. (*uglify-throw-original.js:9:3) - at process.processImmediate (node:internal*timers:483:21) + at process.processImmediate (node:internal*timers:503:21) Node.js * diff --git a/test/parallel/test-timers-nan-duration-emit-once-per-process.js b/test/parallel/test-timers-nan-duration-emit-once-per-process.js new file mode 100644 index 00000000000..4dd2eed5ace --- /dev/null +++ b/test/parallel/test-timers-nan-duration-emit-once-per-process.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); + +const NOT_A_NUMBER = NaN; + +function timerNotCanceled() { + assert.fail('Timer should be canceled'); +} + +process.on( + 'warning', + common.mustCall((warning) => { + if (warning.name === 'DeprecationWarning') return; + + const lines = warning.message.split('\n'); + + assert.strictEqual(warning.name, 'TimeoutNaNWarning'); + assert.strictEqual(lines[0], `${NOT_A_NUMBER} is not a number.`); + assert.strictEqual(lines.length, 2); + }, 1) +); + +{ + const timeout = setTimeout(timerNotCanceled, NOT_A_NUMBER); + clearTimeout(timeout); +} + +{ + const interval = setInterval(timerNotCanceled, NOT_A_NUMBER); + clearInterval(interval); +} + +{ + const timeout = setTimeout(timerNotCanceled, NOT_A_NUMBER); + timeout.refresh(); + clearTimeout(timeout); +} diff --git a/test/parallel/test-timers-nan-duration-warning.js b/test/parallel/test-timers-nan-duration-warning.js new file mode 100644 index 00000000000..f8bbf65a086 --- /dev/null +++ b/test/parallel/test-timers-nan-duration-warning.js @@ -0,0 +1,67 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const child_process = require('child_process'); +const path = require('path'); + +const NOT_A_NUMBER = NaN; + +function timerNotCanceled() { + assert.fail('Timer should be canceled'); +} + +const testCases = ['timeout', 'interval', 'refresh']; + +function runTests() { + const args = process.argv.slice(2); + + const testChoice = args[0]; + + if (!testChoice) { + const filePath = path.join(__filename); + + testCases.forEach((testCase) => { + const { stdout } = child_process.spawnSync( + process.execPath, + [filePath, testCase], + { encoding: 'utf8' } + ); + + const lines = stdout.split('\n'); + + if (lines[0] === 'DeprecationWarning') return; + + assert.strictEqual(lines[0], 'TimeoutNaNWarning'); + assert.strictEqual(lines[1], `${NOT_A_NUMBER} is not a number.`); + assert.strictEqual(lines[2], 'Timeout duration was set to 1.'); + }); + } + + if (args[0] === testCases[0]) { + const timeout = setTimeout(timerNotCanceled, NOT_A_NUMBER); + clearTimeout(timeout); + } + + if (args[0] === testCases[1]) { + const interval = setInterval(timerNotCanceled, NOT_A_NUMBER); + clearInterval(interval); + } + + if (args[0] === testCases[2]) { + const timeout = setTimeout(timerNotCanceled, NOT_A_NUMBER); + timeout.refresh(); + clearTimeout(timeout); + } + + process.on( + 'warning', + + (warning) => { + console.log(warning.name); + console.log(warning.message); + } + ); +} + +runTests(); diff --git a/test/parallel/test-timers-negative-duration-warning-emit-once-per-process.js b/test/parallel/test-timers-negative-duration-warning-emit-once-per-process.js new file mode 100644 index 00000000000..2cf3197789f --- /dev/null +++ b/test/parallel/test-timers-negative-duration-warning-emit-once-per-process.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); + +const NEGATIVE_NUMBER = -1; + +function timerNotCanceled() { + assert.fail('Timer should be canceled'); +} + +process.on( + 'warning', + common.mustCall((warning) => { + if (warning.name === 'DeprecationWarning') return; + + const lines = warning.message.split('\n'); + + assert.strictEqual(warning.name, 'TimeoutNegativeWarning'); + assert.strictEqual(lines[0], `${NEGATIVE_NUMBER} is a negative number.`); + assert.strictEqual(lines.length, 2); + }, 1) +); + +{ + const timeout = setTimeout(timerNotCanceled, NEGATIVE_NUMBER); + clearTimeout(timeout); +} + +{ + const interval = setInterval(timerNotCanceled, NEGATIVE_NUMBER); + clearInterval(interval); +} + +{ + const timeout = setTimeout(timerNotCanceled, NEGATIVE_NUMBER); + timeout.refresh(); + clearTimeout(timeout); +} diff --git a/test/parallel/test-timers-negative-duration-warning.js b/test/parallel/test-timers-negative-duration-warning.js new file mode 100644 index 00000000000..4e236e1af9c --- /dev/null +++ b/test/parallel/test-timers-negative-duration-warning.js @@ -0,0 +1,67 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const child_process = require('child_process'); +const path = require('path'); + +const NEGATIVE_NUMBER = -1; + +function timerNotCanceled() { + assert.fail('Timer should be canceled'); +} + +const testCases = ['timeout', 'interval', 'refresh']; + +function runTests() { + const args = process.argv.slice(2); + + const testChoice = args[0]; + + if (!testChoice) { + const filePath = path.join(__filename); + + testCases.forEach((testCase) => { + const { stdout } = child_process.spawnSync( + process.execPath, + [filePath, testCase], + { encoding: 'utf8' } + ); + + const lines = stdout.split('\n'); + + if (lines[0] === 'DeprecationWarning') return; + + assert.strictEqual(lines[0], 'TimeoutNegativeWarning'); + assert.strictEqual(lines[1], `${NEGATIVE_NUMBER} is a negative number.`); + assert.strictEqual(lines[2], 'Timeout duration was set to 1.'); + }); + } + + if (args[0] === testCases[0]) { + const timeout = setTimeout(timerNotCanceled, NEGATIVE_NUMBER); + clearTimeout(timeout); + } + + if (args[0] === testCases[1]) { + const interval = setInterval(timerNotCanceled, NEGATIVE_NUMBER); + clearInterval(interval); + } + + if (args[0] === testCases[2]) { + const timeout = setTimeout(timerNotCanceled, NEGATIVE_NUMBER); + timeout.refresh(); + clearTimeout(timeout); + } + + process.on( + 'warning', + + (warning) => { + console.log(warning.name); + console.log(warning.message); + } + ); +} + +runTests(); diff --git a/test/parallel/test-timers-not-emit-duration-zero.js b/test/parallel/test-timers-not-emit-duration-zero.js new file mode 100644 index 00000000000..c6a51c25b30 --- /dev/null +++ b/test/parallel/test-timers-not-emit-duration-zero.js @@ -0,0 +1,31 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); + +function timerNotCanceled() { + assert.fail('Timer should be canceled'); +} + +process.on( + 'warning', + common.mustNotCall(() => { + assert.fail('Timer should be canceled'); + }) +); + +{ + const timeout = setTimeout(timerNotCanceled, 0); + clearTimeout(timeout); +} + +{ + const interval = setInterval(timerNotCanceled, 0); + clearInterval(interval); +} + +{ + const timeout = setTimeout(timerNotCanceled, 0); + timeout.refresh(); + clearTimeout(timeout); +}