Protect setTimeout from being overridden by sinon.useFakeTimers() and the like

[limulus]

Testing libraries that help developers mock timers will globally replace setTimeout and setImmediate. We only use these for getting an new call stack, so per taylorhakes/promise-polyfill#15, it is justified to keep these from getting replaced.

The change should be a simple addition of var setTimeout = setTimeout, plus a comment to explain why we are doing it. Our usage of setImmediate happens to already be protected, but a comment that covers that should be used as well.

[polomsky]

In my opinion correct solution is to remove next method. Promise/A+ says that everything in then is called in the next tick so current code behaves like next tick, next tick callback.

[limulus]

Hi again, @polomsky! Thanks for taking a look at this!

Yeah, I would actually very much like to remove the next() method, because the extra tick it adds on top of the tick from Promise/A+ is undesirable. But I’ve done a bad job explaining why I think it is necessary, so let’s see if I can explain it…

The next() calls aren’t for the extra tick, but in order to escape from the try block the then() handlers are called from. For example, suppose someone is using a call-me-maybe powered method, doSomethingAsync(), like this:

doSomethingAsync(function (err) {
  if (err) throw err
})

What the user is expecting to happen in the above is that err is thrown and either uncaught (current call stack is terminated and the error is reported in the console). If call-me-maybe did not wrap the callback with next(), then the error that gets thrown here would wind up being caught by the promise, and the promise would be rejected, which would wind up being an unhandled promise rejection.

If there is a better way of avoiding this unexpected behavior that does not incur an extra tick, that would be very useful!

[polomsky]

Hi.
I have only one idea, but it didn't solve the actual problem with fake timer

module.exports = function maybe (cb, promise) {
  if (cb) {
    promise
      .then(function (result) {
        cb(null, result)
      }, function (err) {
        cb(err)
      }).then(null, function(err) {
        next(function () { throw err })
      })
    return undefined
  }
  else {
    return promise
  }
}