Repository: RobinTail/express-zod-api
Pull request #134 (2021-09-19 07:48:41)

Coverage!
Me this morning

![image](https://user-images.githubusercontent.com/13189514/133919697-90bb3e3d-db03-4a9a-ae4e-89c211df454a.png)


RobinTail: There is an issue with detecting stream termination in Node 10.x
RobinTail: 9️⃣ 9️⃣ . 3️⃣ 
RobinTail: 9️⃣ 9️⃣ . 7️⃣ 7️⃣ 
RobinTail: I need a better test for lastResortHandler in case of an error in ResultHandler
RobinTail: 1️⃣ 0️⃣ 0️⃣ 🎉 
RobinTail: ![image](https://user-images.githubusercontent.com/13189514/133922344-f4454a38-10a1-45bd-a348-68a09b4f0633.png)


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0fb018bd..a55a96a9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 ## Version 2
 
+### v2.5.1
+
+- Fixed a bug due to which the execution of the code could continue despite the possible closing of the response 
+  stream by one of the middlewares.
+  - Affected Node versions: below 12.9.0.
+
 ### v2.5.0
 
 - New feature: file uploads!
diff --git a/coverage.svg b/coverage.svg
index 3d55684e..5bb55be2 100644
--- a/coverage.svg
+++ b/coverage.svg
@@ -1,1 +1,1 @@
-<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="116" height="20" role="img" aria-label="Coverage: 98.59%"><title>Coverage: 98.59%</title><linearGradient id="s" x2="0" y2="100%"><stop offset="0" stop-color="#bbb" stop-opacity=".1"/><stop offset="1" stop-opacity=".1"/></linearGradient><clipPath id="r"><rect width="116" height="20" rx="3" fill="#fff"/></clipPath><g clip-path="url(#r)"><rect width="63" height="20" fill="#555"/><rect x="63" width="53" height="20" fill="#4c1"/><rect width="116" height="20" fill="url(#s)"/></g><g fill="#fff" text-anchor="middle" font-family="Verdana,Geneva,DejaVu Sans,sans-serif" text-rendering="geometricPrecision" font-size="110"><text aria-hidden="true" x="325" y="150" fill="#010101" fill-opacity=".3" transform="scale(.1)" textLength="530">Coverage</text><text x="325" y="140" transform="scale(.1)" fill="#fff" textLength="530">Coverage</text><text aria-hidden="true" x="885" y="150" fill="#010101" fill-opacity=".3" transform="scale(.1)" textLength="430">98.59%</text><text x="885" y="140" transform="scale(.1)" fill="#fff" textLength="430">98.59%</text></g></svg>
\ No newline at end of file
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="106" height="20" role="img" aria-label="Coverage: 100%"><title>Coverage: 100%</title><linearGradient id="s" x2="0" y2="100%"><stop offset="0" stop-color="#bbb" stop-opacity=".1"/><stop offset="1" stop-opacity=".1"/></linearGradient><clipPath id="r"><rect width="106" height="20" rx="3" fill="#fff"/></clipPath><g clip-path="url(#r)"><rect width="63" height="20" fill="#555"/><rect x="63" width="43" height="20" fill="#4c1"/><rect width="106" height="20" fill="url(#s)"/></g><g fill="#fff" text-anchor="middle" font-family="Verdana,Geneva,DejaVu Sans,sans-serif" text-rendering="geometricPrecision" font-size="110"><text aria-hidden="true" x="325" y="150" fill="#010101" fill-opacity=".3" transform="scale(.1)" textLength="530">Coverage</text><text x="325" y="140" transform="scale(.1)" fill="#fff" textLength="530">Coverage</text><text aria-hidden="true" x="835" y="150" fill="#010101" fill-opacity=".3" transform="scale(.1)" textLength="330">100%</text><text x="835" y="140" transform="scale(.1)" fill="#fff" textLength="330">100%</text></g></svg>
\ No newline at end of file
diff --git a/src/endpoint.ts b/src/endpoint.ts
index 111f5c41..b31e4346 100644
--- a/src/endpoint.ts
+++ b/src/endpoint.ts
@@ -175,17 +175,21 @@ export class Endpoint<
     logger: Logger
   }) {
     const options: any = {};
+    let isStreamClosed = false;
     for (const def of this.middlewares) {
       input = {...input, ...def.input.parse(input)}; // middleware can transform the input types
       Object.assign(options, await def.middleware({
         input, options, request,
         response, logger
       }));
-      if (response.writableEnded) {
+      isStreamClosed = ('writableEnded' in response && response.writableEnded) ||
+        ('finished' in response && response.finished); // Node v10 and below
+      if (isStreamClosed) {
+        logger.warn(`The middleware ${def.middleware.name} has closed the stream. Accumulated options:`, options);
         break;
       }
     }
-    return {input, options, isStreamClosed: response.writableEnded};
+    return {input, options, isStreamClosed};
   }
 
   async #parseAndRunHandler({input, options, logger}: {input: any, options: any, logger: Logger}) {


Test patch:
diff --git a/tests/unit/endpoint.spec.ts b/tests/unit/endpoint.spec.ts
index 50d8c400..3cf702e3 100644
--- a/tests/unit/endpoint.spec.ts
+++ b/tests/unit/endpoint.spec.ts
@@ -1,3 +1,4 @@
+import http from 'http';
 import {expectType} from 'tsd';
 import {
   z,
@@ -6,7 +7,10 @@ import {
   defaultResultHandler,
   EndpointInput,
   EndpointOutput,
-  EndpointResponse
+  EndpointResponse,
+  defaultEndpointsFactory,
+  createResultHandler,
+  createApiResponse
 } from '../../src';
 import {CommonConfig} from '../../src/config-type';
 import {Endpoint} from '../../src/endpoint';
@@ -147,6 +151,46 @@ describe('Endpoint', () => {
       });
     });
 
+    test('should close the stream on OPTIONS request', async () => {
+      const handlerMock = jest.fn();
+      const endpoint = defaultEndpointsFactory.build({
+        method: 'get',
+        input: z.object({}),
+        output: z.object({}),
+        handler: handlerMock
+      });
+      const requestMock = {
+        method: 'OPTIONS',
+        header: jest.fn(() => mimeJson),
+      };
+      const responseMock: Record<string, jest.Mock> = {
+        end: jest.fn(),
+        set: jest.fn().mockImplementation(() => responseMock),
+        status: jest.fn().mockImplementation(() => responseMock),
+        json: jest.fn().mockImplementation(() => responseMock)
+      };
+      const configMock = {
+        cors: true
+      };
+      await endpoint.execute({
+        request: requestMock as unknown as Request,
+        response: responseMock as unknown as Response,
+        config: configMock as CommonConfig,
+        logger: loggerMock
+      });
+      expect(loggerMock.error).toBeCalledTimes(0);
+      expect(responseMock.status).toBeCalledTimes(0);
+      expect(responseMock.json).toBeCalledTimes(0);
+      expect(handlerMock).toBeCalledTimes(0);
+      expect(responseMock.set).toBeCalledTimes(3);
+      expect(responseMock.end).toBeCalledTimes(1);
+      expect(responseMock.set.mock.calls[0]).toEqual(['Access-Control-Allow-Origin', '*']);
+      expect(responseMock.set.mock.calls[1]).toEqual(['Access-Control-Allow-Methods', 'GET, OPTIONS']);
+      expect(responseMock.set.mock.calls[2]).toEqual(['Access-Control-Allow-Headers', 'content-type']);
+    });
+  });
+
+  describe('#parseOutput', () => {
     test('Should throw on output parsing non-Zod error', async () => {
       const factory = new EndpointsFactory(defaultResultHandler);
       const endpoint = factory.build({
@@ -191,6 +235,103 @@ describe('Endpoint', () => {
     });
   });
 
+  describe('#runMiddlewares', () => {
+    test('Should handle middleware closing the response stream', async () => {
+      const middlewareMock = jest.fn().mockImplementationOnce(async ({input, response}) => {
+        response.end('to hell with all that!');
+        return { inc: input.n + 1 };
+      });
+      const middlewareDefinitionMock = createMiddleware({
+        input: z.object({
+          n: z.number()
+        }),
+        middleware: middlewareMock
+      });
+      const factory = defaultEndpointsFactory.addMiddleware(middlewareDefinitionMock);
+      const handlerMock = jest.fn();
+      const endpoint = factory.build({
+        method: 'post',
+        input: z.object({}),
+        output: z.object({}),
+        handler: handlerMock
+      });
+      const configMock = {
+        cors: true
+      };
+      const requestMock = {
+        method: 'POST',
+        header: jest.fn(() => mimeJson),
+        body: {
+          n: 453
+        }
+      };
+      const responseMock: any = new http.ServerResponse(requestMock as unknown as Request);
+      responseMock.set = jest.fn().mockImplementation(() => responseMock);
+      responseMock.status = jest.fn().mockImplementation(() => responseMock);
+      responseMock.json = jest.fn().mockImplementation(() => responseMock);
+      await endpoint.execute({
+        request: requestMock as unknown as Request,
+        response: responseMock as unknown as Response,
+        config: configMock as CommonConfig,
+        logger: loggerMock
+      });
+      expect(handlerMock).toHaveBeenCalledTimes(0);
+      expect(middlewareMock).toHaveBeenCalledTimes(1);
+      expect(loggerMock.error).toBeCalledTimes(0);
+      expect(loggerMock.warn).toBeCalledTimes(1);
+      expect(loggerMock.warn.mock.calls[0][0]).toBe(
+        'The middleware mockConstructor has closed the stream. Accumulated options:'
+      );
+      expect(loggerMock.warn.mock.calls[0][1]).toEqual({inc: 454});
+      expect(responseMock.status).toBeCalledTimes(0);
+      expect(responseMock.json).toBeCalledTimes(0);
+      expect(responseMock.statusCode).toBe(200);
+      expect(responseMock.statusMessage).toBe('OK');
+    });
+  });
+
+  describe('#handleResult', () => {
+    test('Should handle errors within ResultHandler', async () => {
+      const factory = new EndpointsFactory(createResultHandler({
+        getPositiveResponse: () => createApiResponse(z.object({})),
+        getNegativeResponse: () => createApiResponse(z.object({})),
+        handler: () => {
+          throw new Error('Something unexpected happened');
+        }
+      }));
+      const endpoint = factory.build({
+        method: 'get',
+        input: z.object({}),
+        output: z.object({
+          test: z.string()
+        }),
+        handler: async () => ({test: 'OK'})
+      });
+      const requestMock = {
+        method: 'GET',
+        header: jest.fn(() => mimeJson),
+      };
+      const responseMock: Record<string, jest.Mock> = {
+        set: jest.fn().mockImplementation(() => responseMock),
+        status: jest.fn().mockImplementation(() => responseMock),
+        json: jest.fn().mockImplementation(() => responseMock)
+      };
+      const configMock = {
+        cors: true
+      };
+      await endpoint.execute({
+        request: requestMock as unknown as Request,
+        response: responseMock as unknown as Response,
+        config: configMock as CommonConfig,
+        logger: loggerMock
+      });
+      expect(loggerMock.error).toBeCalledTimes(1);
+      expect(loggerMock.error.mock.calls[0][0]).toBe('Result handler failure: Something unexpected happened.');
+      expect(responseMock.status).toBeCalledTimes(0);
+      expect(responseMock.json).toBeCalledTimes(0);
+    });
+  });
+
   describe('.getInputSchema()', () => {
     test('should return input schema', () => {
       const factory = new EndpointsFactory(defaultResultHandler);
diff --git a/tests/unit/server.spec.ts b/tests/unit/server.spec.ts
index e067157b..15e6833c 100644
--- a/tests/unit/server.spec.ts
+++ b/tests/unit/server.spec.ts
@@ -126,6 +126,37 @@ describe('Server', () => {
       expect(appMock.listen).toBeCalledTimes(1);
       expect(appMock.listen.mock.calls[0][0]).toBe(8054);
     });
+
+    test('parserFailureHandler should call next if there is no error', () => {
+      const configMock: ServerConfig & CommonConfig = {
+        server: {
+          listen: 8054,
+        },
+        cors: true,
+        logger: {
+          level: 'warn',
+          color: false
+        }
+      };
+      const routingMock = {
+        v1: {
+          test: new EndpointsFactory(defaultResultHandler)
+            .build({
+              method: 'get',
+              input: z.object({}),
+              output: z.object({}),
+              handler: jest.fn()
+            })
+        }
+      };
+      createServer(configMock, routingMock);
+      expect(appMock).toBeTruthy();
+      expect(appMock.use).toBeCalledTimes(3);
+      expect(typeof appMock.use.mock.calls[1][0]).toBe('function');
+      const next = jest.fn();
+      appMock.use.mock.calls[1][0](undefined, null, null, next);
+      expect(next).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('attachRouting()', () => {