Skip to content

onImmediate is not a function #6298

Description

@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);
    }
  }
}

Activity

  1. added
    timersIssues and PRs related to timers, setImmediate(), setInterval(), and setTimeout().
    on Apr 20, 2016
  2. addaleax commented on Apr 20, 2016

    @addaleax
    Member

    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?

  3. r0bot commented on Apr 20, 2016

    @r0bot
    Author

    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.

  4. addaleax commented on Apr 20, 2016

    @addaleax
    Member

    @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.

  5. mscdex commented on Apr 20, 2016

    @mscdex
    Contributor

    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?

  6. Fishrock123 commented on Apr 20, 2016

    @Fishrock123
    Contributor

    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().

  7. r0bot commented on Apr 20, 2016

    @r0bot
    Author

    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.

  8. addaleax commented on Apr 20, 2016

    @addaleax
    Member

    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?

  9. r0bot commented on Apr 20, 2016

    @r0bot
    Author

    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.

  10. r0bot commented on Apr 20, 2016

    @r0bot
    Author

    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.

  11. addaleax commented on Apr 20, 2016

    @addaleax
    Member

    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.

  12. MylesBorins commented on Apr 20, 2016

    @MylesBorins
    Contributor

    @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

  13. addaleax commented on Apr 20, 2016

    @addaleax
    Member

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

  14. MylesBorins commented on Apr 20, 2016

    @MylesBorins
    Contributor

    @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

  15. jasnell commented on Apr 20, 2016

    @jasnell
    Member

    That could work.

  16. addaleax commented on Apr 20, 2016

    @addaleax
    Member

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

  17. MylesBorins commented on Apr 20, 2016

    @MylesBorins
    Contributor

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

    edit: it should be against v4.x-staging

  18. addaleax commented on Apr 20, 2016

    @addaleax
    Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionIssues asking questions about Node.js.timersIssues and PRs related to timers, setImmediate(), setInterval(), and setTimeout().

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions