diff --git a/lib/runnable.js b/lib/runnable.js index 73da817793..2e62a6bc8b 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -50,23 +50,43 @@ function Runnable(title, fn) { utils.inherits(Runnable, EventEmitter); /** - * Set & get timeout `ms`. + * Get current timeout value in msecs. * - * @api private - * @param {number|string} ms - * @return {Runnable|number} ms or Runnable instance. + * @private + * @returns {number} current timeout threshold value + */ +/** + * @summary + * Set timeout threshold value (msecs). + * + * @description + * A string argument can use shorthand (e.g., "2s") and will be converted. + * The value will be clamped to range [0, 2^31-1]. + * If clamped value matches either range endpoint, timeouts will be disabled. + * + * @private + * @see {@link https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout#Maximum_delay_value} + * @param {number|string} ms - Timeout threshold value. + * @returns {Runnable} this + * @chainable */ Runnable.prototype.timeout = function(ms) { if (!arguments.length) { return this._timeout; } - // see #1652 for reasoning - if (ms === 0 || ms > Math.pow(2, 31)) { - this._enableTimeouts = false; - } if (typeof ms === 'string') { ms = milliseconds(ms); } + + // Clamp to range + var INT_MAX = Math.pow(2, 31) - 1; + var range = [0, INT_MAX]; + ms = utils.clamp(ms, range); + + // see #1652 for reasoning + if (ms === range[0] || ms === range[1]) { + this._enableTimeouts = false; + } debug('timeout %d', ms); this._timeout = ms; if (this.timer) { diff --git a/lib/utils.js b/lib/utils.js index e67bf74414..573607d2a8 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -663,6 +663,17 @@ exports.isPromise = function isPromise(value) { return typeof value === 'object' && typeof value.then === 'function'; }; +/** + * Clamps a numeric value to an inclusive range. + * + * @param {number} value - Value to be clamped. + * @param {numer[]} range - Two element array specifying [min, max] range. + * @returns {number} clamped value + */ +exports.clamp = function clamp(value, range) { + return Math.min(Math.max(value, range[0]), range[1]); +}; + /** * It's a noop. * @api diff --git a/test/unit/runnable.spec.js b/test/unit/runnable.spec.js index 9ea00b6d07..0fb29db6e5 100644 --- a/test/unit/runnable.spec.js +++ b/test/unit/runnable.spec.js @@ -46,18 +46,84 @@ describe('Runnable(title, fn)', function() { }); describe('#timeout(ms)', function() { - it('should set the timeout', function() { - var run = new Runnable(); - run.timeout(1000); - assert(run.timeout() === 1000); + var MIN_TIMEOUT = 0; + var MAX_TIMEOUT = 2147483647; // INT_MAX (32-bit signed integer) + + describe('when value is less than lower bound', function() { + it('should clamp to lower bound given numeric', function() { + var run = new Runnable(); + run.timeout(-1); + assert(run.timeout() === MIN_TIMEOUT); + }); + // :TODO: Our internal version of `ms` can't handle negative time, + // but package version can. Skip this check until that change is merged. + it.skip('should clamp to lower bound given timestamp', function() { + var run = new Runnable(); + run.timeout('-1 ms'); + assert(run.timeout() === MIN_TIMEOUT); + }); }); - }); - describe('#timeout(ms) when ms>2^31', function() { - it('should set disabled', function() { - var run = new Runnable(); - run.timeout(1e10); - assert(run.enableTimeouts() === false); + describe('when value is equal to lower bound', function() { + it('should set the value and disable timeouts given numeric', function() { + var run = new Runnable(); + run.timeout(MIN_TIMEOUT); + assert(run.timeout() === MIN_TIMEOUT); + assert(run.enableTimeouts() === false); + }); + it('should set the value and disable timeouts given timestamp', function() { + var run = new Runnable(); + run.timeout(MIN_TIMEOUT + 'ms'); + assert(run.timeout() === MIN_TIMEOUT); + assert(run.enableTimeouts() === false); + }); + }); + + describe('when value is within `setTimeout` bounds', function() { + var oneSecond = 1000; + + it('should set the timeout given numeric', function() { + var run = new Runnable(); + run.timeout(oneSecond); + assert(run.timeout() === oneSecond); + assert(run.enableTimeouts() === true); + }); + it('should set the timeout given timestamp', function() { + var run = new Runnable(); + run.timeout('1s'); + assert(run.timeout() === oneSecond); + assert(run.enableTimeouts() === true); + }); + }); + + describe('when value is equal to upper bound', function() { + it('should set the value and disable timeout given numeric', function() { + var run = new Runnable(); + run.timeout(MAX_TIMEOUT); + assert(run.timeout() === MAX_TIMEOUT); + assert(run.enableTimeouts() === false); + }); + it('should set the value and disable timeout given timestamp', function() { + var run = new Runnable(); + run.timeout(MAX_TIMEOUT + 'ms'); + assert(run.timeout() === MAX_TIMEOUT); + assert(run.enableTimeouts() === false); + }); + }); + + describe('when value is greater than `setTimeout` limit', function() { + it('should clamp to upper bound given numeric', function() { + var run = new Runnable(); + run.timeout(MAX_TIMEOUT + 1); + assert(run.timeout() === MAX_TIMEOUT); + assert(run.enableTimeouts() === false); + }); + it('should clamp to upper bound given timestamp', function() { + var run = new Runnable(); + run.timeout('24.9d'); // 2151360000ms + assert(run.timeout() === MAX_TIMEOUT); + assert(run.enableTimeouts() === false); + }); }); });