Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(server): error in pre handler triggers after event (#1500)
  • Loading branch information
William Blankenship committed Sep 19, 2017
1 parent 78b0900 commit c2e6dea
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 9 deletions.
13 changes: 5 additions & 8 deletions lib/server.js
Expand Up @@ -753,17 +753,14 @@ Server.prototype._handle = function _handle(req, res) {
// run pre() handlers first before routing and running
if (self.before.length > 0) {
self._run(req, res, null, self.before, function (err) {
// check for return false here - like with the regular handlers,
// if false is returned we already sent a response and should stop
// processing.
if (err === false) {
self._finishReqResCycle(req, res);
// Like with regular handlers, if we are provided an error, we
// should abort the middleware chain and fire after events.
if (err === false || err instanceof Error) {
self._finishReqResCycle(req, res, null, err);
return;
}

if (!err) {
routeAndRun();
}
routeAndRun();
});
} else {
routeAndRun();
Expand Down
19 changes: 18 additions & 1 deletion test/server.test.js
Expand Up @@ -630,7 +630,6 @@ test('GH-64 prerouting chain with error', function (t) {
});
});


test('GH-67 extend access-control headers', function (t) {
SERVER.get('/hello/:name', function tester(req, res, next) {
res.header('Access-Control-Allow-Headers',
Expand Down Expand Up @@ -2337,6 +2336,24 @@ test('calling next(false) should early exit from pre handlers', function (t) {

});

test('calling next(err) from pre should still emit after event', function (t) {
setTimeout(function () {
t.fail('Timed out');
t.end();
}, 2000);
var error = new Error();
SERVER.pre(function (req, res, next) {
next(error);
});
SERVER.get('/', function (req, res, next) {
t.fail('should have aborted stack before routing');
});
SERVER.on('after', function (req, res, route, err) {
t.equal(err, error);
t.end();
});
CLIENT.get('/', function () {});
});

test('GH-1078: server name should default to restify', function (t) {

Expand Down

0 comments on commit c2e6dea

Please sign in to comment.