Optionally use setImmediate instead of nextTick in middleware

[bhuga]

When upgrading from node 0.8 to 0.10, we were failing with errors like this:

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

node.js:0
// Copyright Joyent, Inc. and other Node contributors.
^
RangeError: Maximum call stack size exceeded

This uses setImmediate instead of nextTick if available, allowing our hubot to boot.

cc @technicalpickles

[technicalpickles]

Maybe this would be better on Robot than middleware? There's stuff in listener using it too.

[bhuga]

Maybe this would be better on Robot than middleware? There's stuff in listener using it too.

nextTick is not deprecated entirely, only when used recursively. That's how we use it in middleware. I got the use case in listener that's recursive and left the other one alone.

[technicalpickles]

Sounds good then 👍

[michaelansel]

Hmm.. the docs recommend using process.nextTick if you are async-ifying a function: https://nodejs.org/api/process.html#process_process_nexttick_callback_arg

The reason the warning is there is because the behavior changed from "after I/O" to "before I/O" in 0.10. I think we are okay with blocking I/O here, so that warning is noise. I wonder if it got removed in later versions...

[bhuga]

The reason the warning is there is because the behavior changed from "after I/O" to "before I/O" in 0.10.

Not just that. nextTick was officially deprecated in recursive contexts, which our middleware turns out to be. We still have a few other nextTicks that are fine; only the ones done in the middleware's inner loop are bad. Ctrl-f for 'recursive' in https://github.com/nodejs/node/wiki/API-changes-between-v0.8-and-v0.10.

I'm going to release 2.17 without this. We haven't been running it and I want to do our upgrade internally before the rest of the world tests it extensively.

[bhuga]

We've been using this for about a week and no catastrophes have popped up. Any objections to merging it?

[technicalpickles]

We've been using this for awhile without problem, so it's good to merge & release.