Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

setTimeout fired immediately if delay is far future #8656

Closed
danieljuhl opened this issue Nov 1, 2014 · 8 comments
Closed

setTimeout fired immediately if delay is far future #8656

danieljuhl opened this issue Nov 1, 2014 · 8 comments

Comments

@danieljuhl
Copy link

When the following code is executed, some of the timers are fired immediately if their delay is far future (some quick testing says approx. 25 days).

function createTimer (wait) {
  function show () {
    console.log(wait/1000/60/60/24);
  }

  setTimeout(show, wait);
}

createTimer(1000*60*60*24*365);
createTimer(1000*60*60*24*60);
createTimer(1000*60*60*24*30);
createTimer(1000*60*60*24*25);
createTimer(1000*60*60*24*24);
createTimer(1000*60*60*24*21);
createTimer(1000*60*60*24*14);
createTimer(1000*60*60*24*7);

Prints the following to the screen

365
60
30
25
@danieljuhl
Copy link
Author

Reproduced using Node.js v0.10.32; on Ubuntu 14.04 and Node.js v0.10.30; on Mac OS X 10.9.3

@danieljuhl
Copy link
Author

It is also reproducible using setInterval instead of setTimeout

@bjouhier
Copy link

bjouhier commented Nov 1, 2014

Explanation from https://developer.mozilla.org/en-US/docs/Web/API/WindowTimers.setTimeout:

Browsers including Internet Explorer, Chrome, Safari, and Firefox store the delay as a 32-bit signed Integer internally. This causes an Integer overflow when using delays larger than 2147483647, resulting in the timeout being executed immediately.

This is not so much a problem in the browser but it can be on server side (we ran into it too). Would be nice to have a helper function in node to set long timeouts.

@hustxiaoc
Copy link

Actually timeout has it's max value on node.js, that is 2147483647 (2^31-1)

if (!(after >= 1 && after <= TIMEOUT_MAX)) {
    after = 1; 
  }

and 1000_60_60_24_25 > 2147483647
https://github.com/joyent/node/blob/master/lib/timers.js#L29

@jasnell
Copy link
Member

jasnell commented Dec 17, 2014

Just to point out... the maximum timeout is clearly pointed out in the docs (http://nodejs.org/api/all.html#all_settimeout_cb_ms) ...

Setting timeouts so far in the future seems questionable, but if you really need something like that, you can work around the max timeout limit using something like the following:

function setLongTimeout(callback, ms) {
  if (typeof callback !== 'function')
    throw new Error('Callback must be a function');
   ms = parseInt(ms);
   if (Number.isNaN(ms))
    throw new Error('Delay must be an integer');

  var args = Array.prototype.slice.call(arguments,2);
  var cb = callback.bind.apply(callback, [this].concat(args));

  var longTimeout = {
    timer: null,
    clear: function() {
      if (this.timer)
        clearTimeout(this.timer);
    }
  };

  var max = 2147483647;
  if (ms <= max) 
    longTimeout.timer = setTimeout(cb, ms);
  else {
    var count = Math.floor(ms / max); // the number of times we need to delay by max
    var rem = ms % max; // the length of the final delay
    (function delay() {
      if (count > 0) {
        count--;
        longTimeout.timer = setTimeout(delay, max);
      } else {
        longTimeout.timer = setTimeout(cb, rem);
      }
    })();
  }
  return longTimeout;
}

function clearLongTimeout(longTimeoutObject) {
  if (longTimeoutObject && 
      typeof longTimeoutObject.clear === 'function')
    longTimeoutObject.clear()
}

@danieljuhl
Copy link
Author

@jasnell After the above comments, I do know that it is working as designed :)

You are right, that it is in rare cases that you need such long timeout, and you're suggestion is probably the workaround. As you can see in the related issue from node-bunyan, we're made a workaround much like this. Bunyan's log rotation is such an edge case, where, due to the design of the module, a long timeout is useful.

I'm closing the issue

@360disrupt
Copy link

360disrupt commented Oct 12, 2017

@danieljuhl I would recommend you using https://github.com/kelektiv/node-cron#another-example-with-date combined with https://github.com/moment/moment you can specify a cronjob at a certain date:

var CronJob = require('cron').CronJob;
var moment = require('moment')
var job = new CronJob(moment().add(25, 'days'), function() {
  /* runs once at the specified date. */
  }, function () {
    /* This function is executed when the job stops */
  },
  true, /* Start the job right now */
  timeZone /* Time zone of this job. */
);

Wouldn't it be a better solution to throw an error instead of setting the timeout to 1.

@cjihrig
Copy link

cjihrig commented Oct 15, 2017

In the future, this will emit a warning such as:

(node:9077) TimeoutOverflowWarning: 1000000000000 does not fit into a 32-bit signed integer.

You can also use a module like big-time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants