Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Q uses process.nextTick instead of setImmediate in Node 0.10.x #541

Open
joe-spanning opened this issue Jun 18, 2014 · 6 comments
Open

Q uses process.nextTick instead of setImmediate in Node 0.10.x #541

joe-spanning opened this issue Jun 18, 2014 · 6 comments

Comments

@joe-spanning
Copy link

I am using Q 1.0.1 and Node.js 0.10.24

I have a application that uses Q to recursively process large amounts of data using the following pattern:

doPromiseyThing()
  .then(function(result) {
    if (moreToDo) {
      return doPromiseyThing();
    }

    return result;
  });

If there is a large amount of data to process in this way, I eventually get the error:

Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.

node.js:375
        throw new Error(msg);
              ^
Error: (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
    at maxTickWarn (node.js:375:15)
    at process.nextTick (node.js:480:9)
    at onwrite (_stream_writable.js:260:15)
    at WritableState.onwrite (_stream_writable.js:97:5)
    at Socket._write (net.js:651:5)
    at doWrite (_stream_writable.js:221:10)
    at writeOrBuffer (_stream_writable.js:211:5)
    at Socket.Writable.write (_stream_writable.js:180:11)
    at Socket.write (net.js:613:40)
    at Console.warn (console.js:61:16)

This error is not caught by my fail handler function, so I cannot recover from it. It comes across as an uncaught exception in Node. However, this is not the point of me creating the ticket.

The error states that setImmediate should be used instead of process.nextTick. Looking at the source of q.js lines 158 - 177:

if (typeof process !== "undefined" && process.nextTick) {
        // Node.js before 0.9. Note that some fake-Node environments, like the
        // Mocha test runner, introduce a `process` global without a `nextTick`.
        isNodeJS = true;

        requestTick = function () {
            process.nextTick(flush);
        };

    } else if (typeof setImmediate === "function") {
        // In IE10, Node.js 0.9+, or https://github.com/NobleJS/setImmediate
        if (typeof window !== "undefined") {
            requestTick = setImmediate.bind(window, flush);
        } else {
            requestTick = function () {
                setImmediate(flush);
            };
        }

    }

It prefers process.nextTick to setImmediate.

If would be nice if this code was changed to use setImmediate, or if I could configure Q to prefer setImmediate over process.nextTick via an environment variable.

@dsimmons
Copy link

+1

I too just started having the same exact problem. I'm also processing large amounts of data, using recursive promises, to elegantly handle both rate limiting and calls for subsequent pages where I don't know where the end is.

It hums along great for several hours (was watching htop the entire time to monitor for potential memory leaks), but eventually terminates with the above error.

@dantman
Copy link

dantman commented Jul 22, 2014

to elegantly handle both rate limiting and calls for subsequent pages where I don't know where the end is.

On this note I should note a separate relevant, IE has flawed handling of its setImmediate function. In some cases setTimeout loops and DOM modifications can result in setImmediate callbacks never being called.
http://codeforhire.com/2013/09/21/setimmediate-and-messagechannel-broken-on-internet-explorer-10/

@dsimmons
Copy link

@dantman: that's a valid concern that should be taken into consideration, but just in the interest of debugging this: at least for my case, it's entirely in a server-side environment with no front-end interaction.

@jimlloyd
Copy link

@joe-spanning: Thanks for logging this issue, you saved me some debugging time. I am using the exact same pattern. My code looks like:

function onScriptInput() {
  self.promptForInput()
    .then(lineReader.readLine)
    .then(self.onEchoInput)
    .then(self.compileAndExecuteScript)
    .then(self.printResult)
    .catch(function(err) { console.error(chalk.bold.red(err)); })
    .done(onScriptInput);
}

It seems I can work around the problem by inserting a delay of 1ms:

function onScriptInput() {
  self.promptForInput()
    .then(lineReader.readLine)
    .then(self.onEchoInput)
    .then(self.compileAndExecuteScript)
    .then(self.printResult)
    .catch(function(err) { console.error(chalk.bold.red(err)); })
    .delay(1)
    .done(onScriptInput);
}

@sideroad
Copy link

sideroad commented Oct 6, 2014

+1

@riegelTech
Copy link

+1, this is a bad design that produces heavy effects when application load increases, late in development flow :-(

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

No branches or pull requests

6 participants