diff --git a/lib/server.js b/lib/server.js index e64bdd19e..f47deb446 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1077,7 +1077,7 @@ Server.prototype._onHandlerError = function _onHandlerError( var self = this; // Call route by name - if (_.isString(err)) { + if (!isUncaught && _.isString(err)) { var routeName = err; var routeHandler = self.router.lookupByName(routeName, req, res); @@ -1322,22 +1322,26 @@ Server.prototype._emitErrorEvents = function _emitErrorEvents( cb ) { var self = this; - var errName = errEvtNameFromError(err); + // Error can be of any type: undefined, Error, Number, etc. so we need + // to protect ourselves from trying to resolve names from non Error objects + var errName = err && err.name; + var normalizedErrName = errName && errEvtNameFromError(err); req.log.trace( { err: err, - errName: errName + errName: normalizedErrName }, 'entering emitErrorEvents', - err.name + errName ); var errEvtNames = []; // if we have listeners for the specific error, fire those first. - if (self.listeners(errName).length > 0) { - errEvtNames.push(errName); + // if there's no error name, we should not emit an event + if (normalizedErrName && self.listeners(normalizedErrName).length > 0) { + errEvtNames.push(normalizedErrName); } // or if we have a generic error listener. always fire generic error event @@ -1482,9 +1486,11 @@ function errEvtNameFromError(err) { } else if (err.name === 'InvalidVersionError') { // remap the name for router errors return 'VersionNotAllowed'; - } else { + } else if (err.name) { return err.name.replace(/Error$/, ''); } + // If the err is not an Error, then just return an empty string + return ''; } /** diff --git a/test/server.test.js b/test/server.test.js index f9b20d2ad..ce896fa36 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -2477,6 +2477,96 @@ test('should emit error with multiple next calls with strictNext', function(t) { }); }); +test('uncaughtException should not trigger named routeHandler', function(t) { + SERVER.get( + { + name: 'foo', + path: '/foo' + }, + function(req, res, next) { + throw 'bar'; //eslint-disable-line no-throw-literal + } + ); + + SERVER.get( + { + name: 'bar', + path: '/bar' + }, + function(req, res, next) { + // This code should not run, but we can test against the status code + res.send(200); + next(); + } + ); + + CLIENT.get('/foo', function(err, _, res) { + t.ok(err); + t.equal(res.statusCode, 500); + t.end(); + }); +}); + +test('uncaughtException should handle thrown undefined literal', function(t) { + SERVER.get( + { + name: 'foo', + path: '/foo' + }, + function(req, res, next) { + throw undefined; //eslint-disable-line no-throw-literal + } + ); + + SERVER.get( + { + name: 'bar', + path: '/bar' + }, + function(req, res, next) { + // This code should not run, but we can test against the status code + res.send(200); + next(); + } + ); + + CLIENT.get('/foo', function(err, _, res) { + t.ok(err); + t.equal(res.statusCode, 500); + t.end(); + }); +}); + +test('uncaughtException should handle thrown Number', function(t) { + SERVER.get( + { + name: 'foo', + path: '/foo' + }, + function(req, res, next) { + throw 1; //eslint-disable-line no-throw-literal + } + ); + + SERVER.get( + { + name: 'bar', + path: '/bar' + }, + function(req, res, next) { + // This code should not run, but we can test against the status code + res.send(200); + next(); + } + ); + + CLIENT.get('/foo', function(err, _, res) { + t.ok(err); + t.equal(res.statusCode, 500); + t.end(); + }); +}); + test('should have proxy event handlers as instance', function(t) { var server = restify.createServer({ handleUpgrades: false