Using Nan::MakeCallback and handling exceptions thrown from JS

[dpolivy]

I've been tracking down the source of random 'hangs' when using the edge module on node >v0.10 (see tjanczuk/edge#325 for one example). The module was calling v8::Function::Call instead of Nan::MakeCallback to call JS async callbacks; per #284 (comment), I realize this is bad, and can result in the next tick never happening (which is exactly the scenario we're hitting).

Updating the module to use Nan::MakeCallback and Nan::Callback fixes the problem, but creates another one: exceptions thrown in the JS callback are just swallowed and not passed back to C++ as they are with v8::Function::Call. This behavior does seem to be changed in Node v6+, and exceptions are properly propagated back to C++.

For earlier versions (>0.10 and <6), is there any way to achieve consistent behavior and have JS exceptions propagate back to C++? Alternately, is there another approach to kicking off the next tick, if we continue to call v8::Function::Call (or Nan::Call) directly?

[kkoopa]

You could call an empty js function using Nan::MakeCallback at the end, to kick of the next tick. Since the function is empty, it cannot produce exceptions, so it does not matter that exceptions get swallowed in the tick kicker callback, since it does not produce any exceptions.

[dpolivy]

Interesting; so basically switch [back] to using Nan::Call, and then at the end of the processing call Nan::MakeCallback with a no-op function to kick the event loop. I'm curious about the performance overhead of that approach, as well as what else we may be losing out on by not calling MakeCallback (in other threads, it was implied there's a lot more processing in there that needs to happen for things to work correctly).

If the behavior change in v6+ where the exception is marshalled back to C++ is expected, I could try to target the fix to just the affected versions. Any thought of patching this in Nan itself?

[kkoopa]

Cannot do anything. All Nan::MakeCallback does is call node::MakeCallback. A better option would be to handle exceptions in javascript. If you need to signal failure to the calling C++ code, return some value indicating failure, then handle it or throw a js exception from C++.

[dpolivy]

For now, I went the route of just kicking the next tick with an empty function. For reference, here's the implementation of that:

https://github.com/tjanczuk/edge/blob/83d950743998c6afcf1d7866cde2ff930cdcbc93/src/common/callbackhelper.cpp

Thanks for the pointer; this isn't ideal, but it seems to get the job done.