Improve Node 16 abort handling further

The previous version could lose requests, where they were never
reported with a response or an abort, if the handler failed after
closing the writable response.

In that case, the error handler fires, but fails to write an error, and
so tries to mark the response as aborted. Because the writable is
closed, our previous logic assumed that a response was written OK.
That's not necessarily true. Now, we instead require that all handlers
return ASAP (promises OK, but not IO) after closing the socket, and as
long as they do then everything is OK.

It doesn't seem to be possible to directly reproduce that error case in
the tests here, but the new test at least get closer than the existing
ones.

Author: pimterry

src/server/mockttp-server.ts

@@ -389,16 +389,16 @@ export default class MockttpServer extends AbstractMockttp implements Mockttp {
 
         let result: 'responded' | 'aborted' | null = null;
         const abort = () => {
-            if (result === null && !rawResponse.writableEnded) {
+            if (result === null) {
                 result = 'aborted';
                 request.timingEvents.abortedTimestamp = now();
                 this.announceAbortAsync(request);
             }
         }
         request.once('aborted', abort);
-        // In Node 16+ we don't get an abort even in many cases, just closes, but we know
-        // it's aborted because the response is closed before we've written anything.
-        rawResponse.once('close', abort);
+        // In Node 16+ we don't get an abort event in many cases, just closes, but we know
+        // it's aborted because the response is closed with no other result being set.
+        rawResponse.once('close', () => setImmediate(abort));
 
         this.announceInitialRequestAsync(request);
 

test/integration/subscriptions/response-events.spec.ts

@@ -340,14 +340,74 @@ describe("Abort subscriptions", () => {
             expect(seenAbort.timingEvents.bodyReceivedTimestamp).to.equal(undefined);
             expect(wasRequestSeen).to.equal(false);
         });
+
+        describe("given a server that closes connections", () => {
+
+            const badServer = new http.Server((req, res) => {
+                // Forcefully close the socket with no response
+                req.socket!.destroy();
+            });
+
+            beforeEach(async () => {
+                await new Promise((resolve, reject) => {
+                    badServer.listen(8901);
+                    badServer.on('listening', resolve);
+                    badServer.on('error', reject);
+                });
+            });
+
+            afterEach(() => {
+                badServer.close();
+            });
+
+            it("should be sent when the remote server aborts the resopnse", async () => {
+                let seenAbortPromise = getDeferred<InitiatedRequest>();
+                await server.on('abort', (r) => seenAbortPromise.resolve(r));
+
+                let seenResponsePromise = getDeferred<CompletedResponse>();
+                await server.on('response', (r) => seenResponsePromise.resolve(r));
+
+                await server.anyRequest().thenForwardTo(`http://localhost:8901`);
+
+                fetch(server.urlFor("/mocked-endpoint")).catch(() => {});
+
+                await Promise.race([
+                    seenAbortPromise,
+                    seenResponsePromise.then(() => {
+                        throw new Error('Should not fire a response event');
+                    })
+                ]);
+            });
+
+            it("should be sent when a remote proxy aborts the response", async () => {
+                let seenAbortPromise = getDeferred<InitiatedRequest>();
+                await server.on('abort', (r) => seenAbortPromise.resolve(r));
+
+                let seenResponsePromise = getDeferred<CompletedResponse>();
+                await server.on('response', (r) => seenResponsePromise.resolve(r));
+
+                await server.anyRequest().thenPassThrough({
+                    proxyConfig: { proxyUrl: `http://localhost:8901` }
+                });
+
+                fetch(server.urlFor("/mocked-endpoint")).catch(() => {});
+
+                await Promise.race([
+                    seenAbortPromise,
+                    seenResponsePromise.then(() => {
+                        throw new Error('Should not fire a response event');
+                    })
+                ]);
+            });
+        });
     });
 
     it("should be sent in place of response notifications, not in addition", async () => {
         let seenRequestPromise = getDeferred<CompletedRequest>();
         await server.on('request', (r) => seenRequestPromise.resolve(r));
 
         let seenResponsePromise = getDeferred<CompletedResponse>();
-        await server.on('response', (r) => Promise.resolve(r));
+        await server.on('response', (r) => seenResponsePromise.resolve(r));
 
         await server.post('/mocked-endpoint').thenCallback((req) => delay(500).then(() => ({})));