Skip to content
This repository has been archived by the owner. It is now read-only.
Permalink
Browse files

Fix process.nextTick throw call sites

This patch now reports the proper throw call site for exceptions
triggered within process.nextTick. So instead of this:

node.js:201
        throw e; // process.nextTick error, or 'error' event on first tick
              ^

You will now see:

mydir/myscript.js:15
  throw new Error('My Error');
          ^

From my testing this patch causes no performance regressions, but does
greatly simplify processing the nextTickQueue.
  • Loading branch information...
felixge authored and isaacs committed May 8, 2012
1 parent ee437c0 commit 814033365b146afb095fad6c2c05d0da0237615f
@@ -255,9 +255,7 @@ static void Spin(uv_idle_t* handle, int status) {
Tick();
}


static Handle<Value> NeedTickCallback(const Arguments& args) {
HandleScope scope;
static void StartTickSpinner() {
need_tick_cb = true;
// TODO: this tick_spinner shouldn't be necessary. An ev_prepare should be
// sufficent, the problem is only in the case of the very last "tick" -
@@ -268,9 +266,12 @@ static Handle<Value> NeedTickCallback(const Arguments& args) {
uv_idle_start(&tick_spinner, Spin);
uv_ref(uv_default_loop());
}
return Undefined();
}

static Handle<Value> NeedTickCallback(const Arguments& args) {
StartTickSpinner();
return Undefined();
}

static void PrepareTick(uv_prepare_t* handle, int status) {
assert(handle == &prepare_tick_watcher);
@@ -1694,6 +1695,9 @@ void FatalException(TryCatch &try_catch) {
emit->Call(process, 2, event_argv);
// Decrement so we know if the next exception is a recursion or not
uncaught_exception_counter--;

// This makes sure uncaught exceptions don't interfere with process.nextTick
StartTickSpinner();
}


@@ -180,26 +180,18 @@

startup.processNextTick = function() {
var nextTickQueue = [];
var nextTickIndex = 0;

process._tickCallback = function() {
var l = nextTickQueue.length;
if (l === 0) return;
var nextTickLength = nextTickQueue.length;
if (nextTickLength === 0) return;

var q = nextTickQueue;
nextTickQueue = [];

try {
for (var i = 0; i < l; i++) q[i]();
}
catch (e) {
if (i + 1 < l) {
nextTickQueue = q.slice(i + 1).concat(nextTickQueue);
}
if (nextTickQueue.length) {
process._needTickCallback();
}
throw e; // process.nextTick error, or 'error' event on first tick
while (nextTickIndex < nextTickLength) {
nextTickQueue[nextTickIndex++]();
}

nextTickQueue.splice(0, nextTickIndex);
nextTickIndex = 0;
};

process.nextTick = function(callback) {
@@ -1,6 +1,6 @@
before

node.js:*
throw e; // process.nextTick error, or 'error' event on first tick
^
module.js:311
throw err;
^
RangeError: Maximum call stack size exceeded
@@ -1,6 +1,6 @@
before

node.js:*
throw e; // process.nextTick error, or 'error' event on first tick
^
module.js:311
throw err;
^
MyCustomError: This is a custom message
@@ -1,6 +1,6 @@
before

node.js:*
throw e; // process.nextTick error, or 'error' event on first tick
^
module.js:311
throw err;
^
[object Object]
@@ -1,8 +1,8 @@
before

node.js:*
throw e; // process.nextTick error, or 'error' event on first tick
^
module.js:311
throw err;
^
ReferenceError: foo is not defined
at evalmachine.<anonymous>:*
at Object.<anonymous> (*test*message*undefined_reference_in_new_context.js:*)

6 comments on commit 8140333

@mikeal

This comment has been minimized.

Copy link

replied May 9, 2012

w00t!

@chilts

This comment has been minimized.

Copy link

replied May 9, 2012

I have no idea about the patch, but what it does is fantastic! :) Good on yer!

@nisc

This comment has been minimized.

Copy link

replied May 9, 2012

Felix has just fixed Node.js, I hope

@fivetanley

This comment has been minimized.

Copy link

replied May 12, 2012

THANK YOU! This is fantastic!

@seriousManual

This comment has been minimized.

Copy link

replied May 15, 2012

thumbsup

@xer0x

This comment has been minimized.

Copy link

replied May 17, 2012

Great idea

Please sign in to comment.
You can’t perform that action at this time.