throwing error in route handler after request aborted leaks inflight requests

[misterdjules]

• Used appropriate template for the issue type
• Searched both open and closed issues for duplicates of this issue
• Title adequately and concisely reflects the feature or the bug

Bug Report

Restify Version

Latest master.

Node.js Version

10.15.1.

Expected behaviour

When throwing an error in a route handler and not listening on the 'uncaughtException' event, the 'after' event is emitted.

This is similar to the issue described at #1765, except for the fact that in this case the application's code is not setting an event listener for the 'uncaughtException' event.

Actual behaviour

When throwing an error in a route handler and not listening on the 'uncaughtException' event, the 'after' event is not emitted.

Repro case

The following test added to test/server.test.js fails:

diff --git a/test/server.test.js b/test/server.test.js
index ba19cbb..31e580c 100644
--- a/test/server.test.js
+++ b/test/server.test.js
@@ -2051,6 +2051,40 @@ test(
     }
 );
 
+// eslint-disable-next-line max-len
+test("should emit 'after' on uncaughtException after response closed without custom uncaughtException listener", function(t) {
+    var ERR_MSG = 'foo';
+    var gotAfter = false;
+    var gotReqCallback = false;
+
+    SERVER.on('after', function(req, res, route, err) {
+        gotAfter = true;
+        t.ok(err);
+        t.equal(req.connectionState(), 'close');
+        t.equal(res.statusCode, 444);
+        t.equal(err.name, 'Error');
+        t.equal(err.message, ERR_MSG);
+        if (gotReqCallback) {
+            t.end();
+        }
+    });
+
+    SERVER.get('/foobar', function(req, res, next) {
+        res.on('close', function onResClose() {
+            throw new Error(ERR_MSG);
+        });
+    });
+
+    FAST_CLIENT.get('/foobar', function(err, _, res) {
+        gotReqCallback = true;
+        t.ok(err);
+        t.equal(err.name, 'RequestTimeoutError');
+        if (gotAfter) {
+            t.end();
+        }
+    });
+});
+
 test('should increment/decrement inflight request count', function(t) {
     SERVER.get('/foo', function(req, res, next) {
         t.equal(SERVER.inflightRequests(), 1);

Cause

When the request/response actual lifecycle is complete before an exception is thrown and before the next callback of a route handler is called, it means that the route handlers are not marked as finished when Restify attempts to mark the request/response lifecycle as finished.

When an exception is eventually thrown, and as a result the next() callback of the route handler is never called, Restify's default behavior is to send a response, but it still doesn't mark route handlers as finished.

Thus, the res._handlersFinished property is always false, and no 'after' event is emitted. This also causes the number of inflight requests to not be decremented.

Are you willing and able to fix this?

Yes.