Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

The timers' ms arg should be an integer #897

Closed
wants to merge 1 commit into from

4 participants

@xk
xk commented

setTimeout() / setInterval() do strange things when it isn't.

@xk xk the timer's ms arg ought to be an integer
Timers do strange things when it isn't.
5bfe1ad
@felixge
Owner

Patch LGTM.

What strange things did you observe?

Anyway, I just tested and setTimeout seems to accept timeouts as strings in the browser, so doing it the same in node is probably a good idea.

@xk
xk commented

The weird thing that happens when passing a float is that it takes longer to fire than expected. This demonstrates the problem https://gist.github.com/932844

You can pass strings:
[Math.floor('3.14'), typeof Math.floor('3.14')] -> [ 3, 'number' ]

@isaacs
Owner

Seems like the Bad Thing happens when passing in a number between 0 and 1. JS doens't have sub-millisecond precision anyhow, so flooring is the right thing to do.

@ry
ry commented

i don't think there is any problem. we already take the IntegerValue in the binding.

@isaacs
Owner

@ry: Somehow it ends up doing a setTimeout for 1000 when provided a number between 0 and 1. Try it:

var t = Date.now()
setTimeout(function () {
  console.error(Date.now() - t)
}, 0.1)

When provided with a float greater than 1, it works as expected.

@ry
ry commented

ah okay. then it is a bug in the binding and should be fixed there. test should be added too.

@ry ry closed this pull request from a commit
@ry ry Fix timeouts with floating point numbers bug
fixes #897.
c8e447e
@ry ry closed this in c8e447e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 11, 2011
  1. @xk

    the timer's ms arg ought to be an integer

    xk authored
    Timers do strange things when it isn't.
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 1 deletion.
  1. +2 −1  lib/timers.js
View
3  lib/timers.js
@@ -148,6 +148,7 @@ exports.active = function(item) {
exports.setTimeout = function(callback, after) {
var timer;
+ after = Math.floor(after);
if (after <= 0) {
// Use the slow case for after == 0
timer = new Timer();
@@ -201,7 +202,7 @@ exports.clearTimeout = function(timer) {
exports.setInterval = function(callback, repeat) {
var timer = new Timer();
-
+ repeat = Math.floor(repeat);
if (arguments.length > 2) {
var args = Array.prototype.slice.call(arguments, 2);
timer.callback = function() {
Something went wrong with that request. Please try again.