diff --git a/CHANGES.md b/CHANGES.md index b45e74460..a0347e9d8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,8 @@ ## not yet released + - #1024 Add server option to disable the uncaughtException handler. + ## 4.2.0 - #925 Support passing (most) [qs](https://github.com/ljharb/qs#readme) options diff --git a/docs/index.restdown b/docs/index.restdown index 04e620e9b..e206ea816 100644 --- a/docs/index.restdown +++ b/docs/index.restdown @@ -142,6 +142,7 @@ below (and `listen()` takes the same arguments as node's ||certificate||String||If you want to create an HTTPS server, pass in the PEM-encoded certificate and key|| ||key||String||If you want to create an HTTPS server, pass in the PEM-encoded certificate and key|| ||formatters||Object||Custom response formatters for `res.send()`|| +||handleUncaughtExceptions||Boolean||When true (the default) restify will use a domain to catch and respond to any uncaught exceptions that occur in it's handler stack|| ||log||Object||You can optionally pass in a [bunyan](https://github.com/trentm/node-bunyan) instance; not required|| ||name||String||By default, this will be set in the `Server` response header, default is `restify` || ||spdy||Object||Any options accepted by [node-spdy](https://github.com/indutny/node-spdy)|| @@ -774,10 +775,11 @@ them. `function (request, response, route, error) {}` -Emitted when some handler throws an uncaughtException somewhere in the chain. -The default behavior is to just call `res.send(error)`, and let the built-ins -in restify handle transforming, but you can override to whatever you want -here. +Emitted when some handler throws an uncaughtException somewhere in the chain and +only when 'handleUncaughtExceptions' is set to true on the restify server. The +restify server has a default handler for this event - which is to just call +`res.send(error)`, and lets the built-ins in restify handle transforming, but +you can override the default handler to do whatever you want. ### Properties diff --git a/lib/index.js b/lib/index.js index 652873201..3ff3a7644 100644 --- a/lib/index.js +++ b/lib/index.js @@ -141,15 +141,18 @@ function createServer(options) { opts.router = opts.router || new Router(opts); server = new Server(opts); - server.on('uncaughtException', function (req, res, route, e) { - if (this.listeners('uncaughtException').length > 1 || - res.headersSent) { - return (false); - } - - res.send(new InternalError(e, e.message || 'unexpected error')); - return (true); - }); + + if (server.handleUncaughtExceptions) { + server.on('uncaughtException', function (req, res, route, e) { + if (this.listeners('uncaughtException').length > 1 || + res.headersSent) { + return (false); + } + + res.send(new InternalError(e, e.message || 'unexpected error')); + return (true); + }); + } return (server); } diff --git a/lib/server.js b/lib/server.js index de0b5fe12..396199dc9 100644 --- a/lib/server.js +++ b/lib/server.js @@ -257,6 +257,12 @@ function Server(options) { this.acceptable = fmt.acceptable; this.formatters = fmt.formatters; + if (options.hasOwnProperty('handleUncaughtExceptions')) { + this.handleUncaughtExceptions = options.handleUncaughtExceptions; + } else { + this.handleUncaughtExceptions = true; + } + if (options.spdy) { this.spdy = true; this.server = spdy.createServer(options.spdy); @@ -945,6 +951,12 @@ Server.prototype._run = function _run(req, res, route, chain, cb) { ]); }); + if (!self.handleUncaughtExceptions) { + n1(); + return; + } + + // Add the uncaughtException error handler. d = domain.create(); d.add(req); d.add(res); diff --git a/test/lib/server-withDisableUncaughtException.js b/test/lib/server-withDisableUncaughtException.js new file mode 100644 index 000000000..a58fb4ffa --- /dev/null +++ b/test/lib/server-withDisableUncaughtException.js @@ -0,0 +1,31 @@ +// A simple node process that will start a restify server with the +// uncaughtException handler disabled. Responds to a 'serverPortRequest' message +// and sends back the server's bound port number. + +'use strict'; + +var restify = require('../../lib'); + +function main() { + var port = process.env.UNIT_TEST_PORT || 0; + var server = restify.createServer({ handleUncaughtExceptions: false }); + server.get('/', function (req, res, next) { + throw new Error('Catch me!'); + }); + server.listen(0, function () { + port = server.address().port; + console.log('port: ', port); + + process.on('message', function (msg) { + if (msg.task !== 'serverPortRequest') { + process.send({error: 'Unexpected message: ' + msg}); + return; + } + process.send({task: 'serverPortResponse', port: port}); + }); + }); +} + +if (require.main === module) { + main(); +} diff --git a/test/server.test.js b/test/server.test.js index 8689fc7b4..146a044d9 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -3,6 +3,7 @@ 'use strict'; var assert = require('assert-plus'); +var childprocess = require('child_process'); var http = require('http'); var filed = require('filed'); @@ -1957,3 +1958,50 @@ test('GH-877 content-type should be case insensitive', function (t) { }); client.end(); }); + + +test('GH-1024 disable uncaughtException handler', function (t) { + // With uncaughtException handling disabled, the node process will abort, + // so testing of this feature must occur in a separate node process. + + var allStderr = ''; + var serverPath = __dirname + '/lib/server-withDisableUncaughtException.js'; + var serverProc = childprocess.fork(serverPath, {silent: true}); + + // Record stderr, to check for the correct exception stack. + serverProc.stderr.on('data', function (data) { + allStderr += String(data); + }); + + // Handle serverPortResponse and then make the client request - the request + // should receive a connection closed error (because the server aborts). + serverProc.on('message', function (msg) { + if (msg.task !== 'serverPortResponse') { + serverProc.kill(); + t.end(); + return; + } + + var port = msg.port; + var client = restify.createJsonClient({ + url: 'http://127.0.0.1:' + port, + dtrace: helper.dtrace, + retry: false + }); + + client.get('/', function (err, _, res) { + // Should get a connection closed error, but no response object. + t.ok(err); + t.equal(err.code, 'ECONNRESET'); + t.equal(res, undefined); + + serverProc.kill(); // Ensure it's dead. + + t.ok(allStderr.indexOf('Error: Catch me!') > 0); + + t.end(); + }); + }); + + serverProc.send({task: 'serverPortRequest'}); +});