fix: properly handle non-errors thrown in domains (#1757)

restify · Mar 16, 2019 · cb2e717 · cb2e717
1 parent 51f9ae5
commit cb2e717
Show file tree

Hide file tree

Showing 2 changed files with 141 additions and 11 deletions.
diff --git a/lib/server.js b/lib/server.js
@@ -1303,19 +1303,24 @@ Server.prototype._routeErrorResponse = function _routeErrorResponse(
         return;
     }
 
-    self._emitErrorEvents(req, res, null, err, function emitError(emitErr) {
+    self._emitErrorEvents(req, res, null, err, function emitError() {
         // Prevent double handling
         if (res._sent) {
             return;
         }
 
-        // Expose only knonw errors
-        if (_.isNumber(err.statusCode)) {
+        // only automatically send errors that are known (e.g., restify-errors)
+        if (err instanceof Error && _.isNumber(err.statusCode)) {
             res.send(err);
             return;
         }
 
-        res.send(new errors.InternalError(emitErr || err));
+        // if the thrown exception is not really an Error object, e.g.,
+        //    "throw 'foo';"
+        // try to do best effort here to pass on that value by casting it to a
+        // string. This should work even for falsy values like 0, false, null,
+        // or undefined.
+        res.send(new errors.InternalError(String(err)));
     });
 };
 
@@ -1385,10 +1390,9 @@ Server.prototype._emitErrorEvents = function _emitErrorEvents(
             }
         },
         // eslint-disable-next-line handle-callback-err
-        function onResult(nullErr, results) {
-            // vasync will never return error here. callback with the original
-            // error to pass it on.
-            return cb(err);
+        function onResult(__, results) {
+            // vasync will never return error here since we throw them away.
+            return cb();
         }
     );
 };

diff --git a/test/server.test.js b/test/server.test.js
@@ -2482,6 +2482,37 @@ test('uncaughtException should not trigger named routeHandler', function(t) {
     });
 });
 
+test('uncaughtException should handle thrown null', function(t) {
+    SERVER.get(
+        {
+            name: 'foo',
+            path: '/foo'
+        },
+        function(req, res, next) {
+            throw null; //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, data) {
+        t.ok(err);
+        t.equal(res.statusCode, 500);
+        t.equal(data.message, 'null');
+        t.end();
+    });
+});
+
 test('uncaughtException should handle thrown undefined literal', function(t) {
     SERVER.get(
         {
@@ -2505,14 +2536,46 @@ test('uncaughtException should handle thrown undefined literal', function(t) {
         }
     );
 
-    CLIENT.get('/foo', function(err, _, res) {
+    CLIENT.get('/foo', function(err, _, res, data) {
+        t.ok(err);
+        t.equal(res.statusCode, 500);
+        t.equal(data.message, 'undefined');
+        t.end();
+    });
+});
+
+test('uncaughtException should handle thrown falsy number', function(t) {
+    SERVER.get(
+        {
+            name: 'foo',
+            path: '/foo'
+        },
+        function(req, res, next) {
+            throw 0; //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, data) {
         t.ok(err);
+        t.equal(data.message, '0');
         t.equal(res.statusCode, 500);
         t.end();
     });
 });
 
-test('uncaughtException should handle thrown Number', function(t) {
+test('uncaughtException should handle thrown non falsy number', function(t) {
     SERVER.get(
         {
             name: 'foo',
@@ -2535,8 +2598,71 @@ test('uncaughtException should handle thrown Number', function(t) {
         }
     );
 
-    CLIENT.get('/foo', function(err, _, res) {
+    CLIENT.get('/foo', function(err, _, res, data) {
+        t.ok(err);
+        t.equal(data.message, '1');
+        t.equal(res.statusCode, 500);
+        t.end();
+    });
+});
+
+test('uncaughtException should handle thrown boolean', function(t) {
+    SERVER.get(
+        {
+            name: 'foo',
+            path: '/foo'
+        },
+        function(req, res, next) {
+            throw true; //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, data) {
+        t.ok(err);
+        t.equal(data.message, 'true');
+        t.equal(res.statusCode, 500);
+        t.end();
+    });
+});
+
+test('uncaughtException should handle thrown falsy boolean', function(t) {
+    SERVER.get(
+        {
+            name: 'foo',
+            path: '/foo'
+        },
+        function(req, res, next) {
+            throw false; //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, data) {
         t.ok(err);
+        t.equal(data.message, 'false');
         t.equal(res.statusCode, 500);
         t.end();
     });