onImmediate is not a function

[r0bot]

• 4.4.3:
• Ubuntu 14.04:

Hi,
We tried to update from 0.10.33 to 4.4.3, but the code below causes some issues. As you can see in the code below in 0.10.33 there was a check to see if _onImmediate was defined and in 4.4.3 there is not which causes the following error.

immediate._onImmediate is not a function

Is this a know issue and is there a know fix, or we have to go and find and fix the places where _undefined_ is passed as onImmediate function?

Node 0.10.33

if (immediate._onImmediate) {
    if (immediate.domain) immediate.domain.enter();

    immediate._onImmediate();

    if (immediate.domain) immediate.domain.exit();
  }

Node 4.4.3

try {
  immediate._onImmediate();
  threw = false;
} finally {
  if (threw) {
    if (!L.isEmpty(queue)) {
      // Handle any remaining on next tick, assuming we're still
      // alive to do so.
      while (!L.isEmpty(immediateQueue)) {
        L.append(queue, L.shift(immediateQueue));
      }
      immediateQueue = queue;
      process.nextTick(processImmediate);
    }
  }
}

[addaleax]

The code has changed, yes. This should not cause any problems as long as you are passing a function as the first argument to setImmediate, though. Can you post a piece of code to reproduce the problem you are experiencing?

[r0bot]

Here is the Error.stack which unfortunately is not very complete, but this is what I get when the exception fires.

immediate._onImmediate is not a function TypeError: immediate._onImmediate is not a function
at processImmediate (timers.js:383:17)
at nextTickCallbackWith0Args (node.js:420:9)
at process._tickDomainCallback (node.js:390:13)

This is also probably in some dependency as in our code there is no _setImmediate_ used. If you can give an idea on how to obtain more of the stack i would appreciate that.

[addaleax]

@r0bot There’s https://www.npmjs.com/package/longjohn that can help you get longer stack trace information.
You can also try out one of the current Node.js v6 release candidates from here; One of the things that will change in v6 is that calls to setImmediate, setTimeout, etc. will fail immediately when the callback argument is not a function, so you can get a proper stack trace.

[mscdex]

I know this was before the timer rework by @Fishrock123, but maybe he can shed some light on this after having spent time with that module?

[Fishrock123]

This means something is manually setting immediate._onImmediate = undefined.

If you use clearImmediate(), it removes it from the linkedlist also: https://github.com/nodejs/node/blob/master/lib/timers.js#L624-L634

That being said, we do a check like this for timers, so we could also for immediates: https://github.com/nodejs/node/blob/master/lib/timers.js#L182

I suggest grepping for _onImmediate = in your dependencies, in either case you'll be safer to use clearImmediate().

[r0bot]

We are currently trying to track the issue with longjohn. Thing is that now that longjohn is active the issue stopped. Looking at the source of longjohn I see things like

 if (global.setImmediate != null) {
    _setImmediate = global.setImmediate;
    global.setImmediate = function(callback) {
      var args;
      args = Array.prototype.slice.call(arguments);
      args[0] = wrap_callback(callback, 'global.setImmediate');
      return _setImmediate.apply(this, args);
    };
}

which might be fixing the issue.
As for the @Fishrock123 suggestion to look for _onImmediate = in the dependencies, I cannot find an occurrence of it.

[addaleax]

I think that’s a good sign, because it means that the problem is actually very likely to be that some call to setImmediate passes a non-function, rather than something messing with the timer internals. Have you had a chance to try this with the v6 release candidate?

[r0bot]

Unfortunately not as the issue is happening on a remote server and updating the node at this point is not possible. I can see that in the master branch setImmediate function has a check like this:

if (typeof callback !== 'function') {
    throw new TypeError('"callback" argument must be a function');
}

We will try to implement this check and see if it helps solves the problem.

[r0bot]

Adding this piece of code:

if (global.setImmediate != null) {
    var _setImmediate = global.setImmediate;
    global.setImmediate = function(callback) {
         if (typeof callback !== 'function') {
            throw new TypeError('"callback" argument must be a function');
         }

        var args = Array.prototype.slice.call(arguments);
        return _setImmediate.apply(this, args);
    };
}

Added the check that was missing and threw the error in the right moment, so the stack was full and we were able to track the issue. One of our third party modules was calling setImmediate with undefined as a callback in some edge cases.
Thank you all for the quick responses, the issue can be closed. Also it might be a good idea to implement this in Node v4 in some of the next releases.

[addaleax]

Good to hear that this worked!

Also it might be a good idea to implement this in Node v4 in some of the next releases.

Unfortunately, I don’t think that’s possible… In #4362 people seemed to agree that this was a semver-major change because it changes the way in which errors are thrown, i.e. it won’t be able to go into v4 or v5.

[MylesBorins]

@r0bot it looks like the code that you highlighted is at least 2 years old so I don't think that we will be updating it in v4.x

If you can come up with a way to introduce a guard that is not at all breaking we might be able to consider backporting it

[addaleax]

@thealphanerd One could do something like #4362 but with warnings instead of throwing an error… would that be worthwhile?

[MylesBorins]

@addaleax could we not add a guard right before calling it?

At the top of setTimeout we could simply add

if (typeof callback !== 'function') {
  callback = function () {}
}

and warn. That way at the very least it is a noop

/cc @jasnell

[jasnell]

That could work.

[addaleax]

@thealphanerd Sorry if I expressed myself a bit incomprehensibly… that is precisely what I had in mind. 👍

[MylesBorins]

@addaleax awesome! Would you want to do that PR or should I?

edit: it should be against v4.x-staging

[addaleax]

@thealphanerd If you have the time, feel free to to that :)