Permalink
Browse files

src,http: fix uncaughtException miss in http

In AsyncWrap::MakeCallback always return empty handle if there is an
error. In the future this should change to return a v8::MaybeLocal, but
that major change will have to wait for v6.x, and these changes are
meant to be backported to v4.x.

The HTTParser call to AsyncWrap::MakeCallback failed because it expected
a thrown call to return an empty handle.

In node::MakeCallback return an empty handle if the call is
in_makecallback(), otherwise return v8::Undefined() as usual to preserve
backwards compatibility.

Fixes: #5555
PR-URL: #5591
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
  • Loading branch information...
trevnorris authored and Fishrock123 committed Mar 4, 2016
1 parent 8501345 commit 5e6d7067583632ac5885b8f8669ca25d2e61f627
Showing with 31 additions and 4 deletions.
  1. +3 −3 src/async-wrap.cc
  2. +5 −1 src/node.cc
  3. +23 −0 test/parallel/test-http-catch-uncaughtexception.js
@@ -193,7 +193,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
if (has_domain) {
domain = domain_v.As<Object>();
if (domain->Get(env()->disposed_string())->IsTrue())
return Undefined(env()->isolate());
return Local<Value>();
}
}
@@ -220,7 +220,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
}
if (ret.IsEmpty()) {
return Undefined(env()->isolate());
return ret;
}
if (has_domain) {
@@ -249,7 +249,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
}
if (env()->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) {
return Undefined(env()->isolate());
return Local<Value>();
}
return ret;
@@ -1185,7 +1185,11 @@ Local<Value> MakeCallback(Environment* env,
}
if (ret.IsEmpty()) {
return Undefined(env->isolate());
if (callback_scope.in_makecallback())
return ret;
// NOTE: Undefined() is returned here for backwards compatibility.
else
return Undefined(env->isolate());
}
if (has_domain) {
@@ -0,0 +1,23 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');
const uncaughtCallback = common.mustCall(function(er) {
assert.equal(er.message, 'get did fail');
});
process.on('uncaughtException', uncaughtCallback);
const server = http.createServer(function(req, res) {
res.writeHead(200, { 'Content-Type': 'text/plain' });
res.end('bye');
}).listen(common.PORT, function() {
http.get({ port: common.PORT }, function(res) {
res.resume();
throw new Error('get did fail');
}).on('close', function() {
server.close();
});
});

0 comments on commit 5e6d706

Please sign in to comment.