backport #1024: add server option to disable the uncaughtException ha…

…ndler

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

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

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);
 }

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);

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();
+}

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'});
+});