Skip to content

Commit

Permalink
New process.exceptionCatcher is inherited by callbacks.
Browse files Browse the repository at this point in the history
This allows a web framework to catch at the top level exceptions
thrown by page dispatch code, and to return a 500 Internal Server Error
for that request without disturbing other requests. This works
even if the page dispatch code is async.

FatalException tries process.exceptionCatcher first before
falling back to emitting an "uncaughtException" event.

Whenever a callback is registered, EventEmitter.addListener()
sets callback._exceptionCatcher to the current value of
process.exceptionCatcher. When the callback is dispatched,
EventEmitter.emit() sets process.exceptionCatcher to the
callback's ._exceptionCatcher.

In this way callbacks registered by callbacks also inherit the
exceptionHandler. It is also possible to chain exceptionHandlers.
  • Loading branch information
benw committed Jan 22, 2010
1 parent fe48b5f commit 78b69e7
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 0 deletions.
32 changes: 32 additions & 0 deletions src/node.cc
Expand Up @@ -57,6 +57,7 @@ static Persistent<String> heap_used_symbol;
static Persistent<String> listeners_symbol;
static Persistent<String> uncaught_exception_symbol;
static Persistent<String> emit_symbol;
static Persistent<String> exception_catcher_symbol;

static int dash_dash_index = 0;
static bool use_debug_agent = false;
Expand Down Expand Up @@ -818,11 +819,40 @@ static void OnFatalError(const char* location, const char* message) {
exit(1);
}

Local<Value> GetProcessExceptionCatcher() {
return process->Get(exception_catcher_symbol);
}

bool SetProcessExceptionCatcher(Handle<Value> value) {
return process->Set(exception_catcher_symbol, value);
}

static int exception_catcher_counter = 0;
static int uncaught_exception_counter = 0;

void FatalException(TryCatch &try_catch) {
HandleScope scope;

// First try process.exceptionCatcher --
// if it's a function, and we're not already in it.
Local<Value> catcher_v = GetProcessExceptionCatcher();
if (0 == exception_catcher_counter && catcher_v->IsFunction()) {
exception_catcher_counter++;
// Call process.exceptionCatcher(exception). If it throws, FatalException
// is reentered with nonzero exception_catcher_counter, and we drop
// through to the "uncaughtException" event below, or barf.
TryCatch inner_try_catch;

Local<Function> catcher = Local<Function>::Cast(catcher_v);
Local<Value> argv[1] = { try_catch.Exception() };
Local<Value> ret = catcher->Call(process, 1, argv);
if (inner_try_catch.HasCaught()) {
FatalException(inner_try_catch);
}
exception_catcher_counter--;
return;
}

// Check if uncaught_exception_counter indicates a recursion
if (uncaught_exception_counter > 0) {
ReportException(&try_catch);
Expand Down Expand Up @@ -977,6 +1007,8 @@ static Local<Object> Load(int argc, char *argv[]) {
process->Set(String::NewSymbol("EventEmitter"),
EventEmitter::constructor_template->GetFunction());

exception_catcher_symbol = NODE_PSYMBOL("exceptionCatcher");

// Initialize the stats object
Local<FunctionTemplate> stat_templ = FunctionTemplate::New();
stats_constructor_template = Persistent<FunctionTemplate>::New(stat_templ);
Expand Down
4 changes: 4 additions & 0 deletions src/node.h
Expand Up @@ -42,6 +42,10 @@ enum encoding ParseEncoding(v8::Handle<v8::Value> encoding_v,
enum encoding _default = BINARY);
void FatalException(v8::TryCatch &try_catch);

// Get and set process.exceptionCatcher
v8::Local<v8::Value> GetProcessExceptionCatcher();
bool SetProcessExceptionCatcher(v8::Handle<v8::Value> value);

v8::Local<v8::Value> Encode(const void *buf, size_t len,
enum encoding encoding = BINARY);

Expand Down
13 changes: 13 additions & 0 deletions src/node.js
Expand Up @@ -192,6 +192,18 @@ process.mixin = function() {
return target;
};


// This stub allows chaining to the previous process.exceptionCatcher
// without checking that it is a valid function.
// e.g.:
// var oldCatcher = process.exceptionCatcher;
// process.exceptionCatcher = function (e) {
// if (e instanceof MyError) { ... }
// else oldCatcher(e);
// }
process.exceptionCatcher = function (e) { throw e; }


// Event

var eventsModule = createInternalModule('events', function (exports) {
Expand All @@ -207,6 +219,7 @@ var eventsModule = createInternalModule('events', function (exports) {
// adding it to the listeners, first emit "newListeners".
this.emit("newListener", type, listener);
this._events[type].push(listener);
listener._exceptionCatcher = process.exceptionCatcher;
}
return this;
};
Expand Down
3 changes: 3 additions & 0 deletions src/node_events.cc
Expand Up @@ -23,6 +23,7 @@ using namespace v8;
Persistent<FunctionTemplate> EventEmitter::constructor_template;

static Persistent<String> events_symbol;
static Persistent<String> exception_catcher_symbol;

void EventEmitter::Initialize(Local<FunctionTemplate> ctemplate) {
HandleScope scope;
Expand All @@ -35,6 +36,7 @@ void EventEmitter::Initialize(Local<FunctionTemplate> ctemplate) {
constructor_template->SetClassName(String::NewSymbol("EventEmitter"));

events_symbol = NODE_PSYMBOL("_events");
exception_catcher_symbol = NODE_PSYMBOL("_exceptionCatcher");

// All other prototype methods are defined in events.js
}
Expand All @@ -59,6 +61,7 @@ static bool ReallyEmit(Handle<Object> self,
Local<Value> listener_v = listeners->Get(Integer::New(i));
if (!listener_v->IsFunction()) continue;
Local<Function> listener = Local<Function>::Cast(listener_v);
SetProcessExceptionCatcher(listener->Get(exception_catcher_symbol));

TryCatch try_catch;

Expand Down

1 comment on commit 78b69e7

@isaacs
Copy link

@isaacs isaacs commented on 78b69e7 Jan 22, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving the exception catcher on the callback is problematic and leads trivially to confusing cases. Observe: http://gist.github.com/283562

The way to avoid this situation might be to trap the current exceptionCatcher in a closure around the callback. Ie, something like:

callback = (function (ec, fn) { return function () { try { fn() } catch (ex) { ec(ex) } } })(process.exceptionCatcher, callback);

However, I'd be wary of that adding the extra scope closure around every callback.

I can't really comment on your C++, I'm sure it's lovely. That's how I'd avoid the problem in JavaScript, though.

I dig the rethrow default handler to support catch-and-release chaining.

Please sign in to comment.