Skip to content

Commit

Permalink
feat: send 500s for unhandled requests (#1777)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonutEspresso committed Jul 2, 2020
1 parent 35871c9 commit 885cecd
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 1 deletion.
21 changes: 20 additions & 1 deletion lib/server.js
Expand Up @@ -1360,7 +1360,10 @@ Server.prototype._finishReqResCycle = function _finishReqResCycle(
var self = this;
var route = req.route; // can be undefined when 404 or error

if (res._finished) {
// if the returned err value was a string, then we're handling the
// next('foo') case where we redirect to another middleware stack. don't
// do anything here because we're not done yet.
if (res._finished || _.isString(err)) {
return;
}

Expand All @@ -1374,6 +1377,22 @@ Server.prototype._finishReqResCycle = function _finishReqResCycle(
var finalErr = err || res.err;
req.emit('restifyDone', route, finalErr);
self.emit('after', req, res, route, finalErr);
} else if (
res._handlersFinished === true &&
res.headersSent === false &&
!res.err
) {
// if we reached the end of the handler chain and headers haven't been
// sent AND there isn't an existing res.err (e.g., req abort/close),
// it's possible it's a user error and a response was never written.
// send a 500.
res.send(
new errors.InternalServerError(
'reached the end of the handler chain without ' +
'writing a response!'
)
);
return;
} else {
// Store error for when the response is flushed and we actually emit the
// 'after' event. The "err" object passed to this method takes
Expand Down
42 changes: 42 additions & 0 deletions test/server.test.js
Expand Up @@ -2497,6 +2497,48 @@ test('should emit error with multiple next calls with strictNext', function(t) {
});
});

test(
'should send 500 if we reached the end of handler chain w/o sending ' +
'headers',
function(t) {
var server = restify.createServer({
dtrace: helper.dtrace,
strictNext: true,
log: helper.getLog('server')
});
var client;
var port;

server.listen(PORT + 1, '127.0.0.1', function() {
port = server.address().port;
client = restifyClients.createJsonClient({
url: 'http://127.0.0.1:' + port,
dtrace: helper.dtrace,
retry: false
});

server.get('/noResponse', function(req, res, next) {
next();
});

client.get('/noResponse', function(err, _, res) {
t.ok(err);
t.equal(res.statusCode, 500);
t.equal(err.name, 'InternalServerError');
t.equal(
err.message,
'reached the end of the handler chain without ' +
'writing a response!'
);
client.close();
server.close(function() {
t.end();
});
});
});
}
);

test('uncaughtException should not trigger named routeHandler', function(t) {
SERVER.get(
{
Expand Down

0 comments on commit 885cecd

Please sign in to comment.