Repository: RobinTail/express-zod-api
Pull request #15 (2021-05-13 16:58:56)

Result handler refactoring


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1cb7651a..67a84bc8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - Readme file updates:
   - Concept description update.
   - Excess property check according to the new features of version 0.7.0.
+- Refactoring of `defaultResultHandler` and `ResultHandler` calls in `server.ts`.
 
 ### v0.7.0
 - Zod version is v3.0.0-beta.1
@@ -33,7 +34,7 @@ const endpoint = factory.build({
     value: z.string().transform((str) => str.length)
   }),
   handler: async ({input, options}) => ({
-    transform: 'test' // => in response: { transform: 4 }
+    value: 'test' // => in response: { value: 4 }
   })
 });
 ```
diff --git a/README.md b/README.md
index 5d9d87bf..bfa2e2db 100644
--- a/README.md
+++ b/README.md
@@ -41,13 +41,13 @@ Starting with version 0.7.0, union and intersection of object schemas are also s
 
 The object being validated is the `request.query` for GET request, the `request.body` for PUT, PATCH and POST requests, or their merging for DELETE requests.
 
-Middlewares can handle validated inputs and the original `request`, for example, to perform the authentication.
-The returns of middlewares are combined into `options` parameter available to the next middlewares and the endpoint's handler.
+Middlewares can handle validated inputs and the original `request`, for example, to perform the authentication or provide the endpoint's handler with some request properties like the actual method.
+The returns of middlewares are combined into the `options` parameter available to the next middlewares and the endpoint's handler.
 
-The endpoint parameter `input` combines the validated inputs of all connected middlewares along with its own.
+The handler's parameter `input` combines the validated inputs of all connected middlewares along with the handler's one.
 The result that the handler returns goes to the `ResultHandler` which is responsible for transmission of the final response or possible error.
 
-All inputs and outputs are validated and there are also advanced powerful features for schema validation like transformations and refinements.
+All inputs and outputs are validated and there are also advanced powerful features like transformations and refinements.
 The diagram below can give you a better idea of the dataflow.
 
 ![Dataflow](dataflow.svg)
diff --git a/src/helpers.ts b/src/helpers.ts
index b3a4fc46..48160a4e 100644
--- a/src/helpers.ts
+++ b/src/helpers.ts
@@ -1,4 +1,5 @@
 import {Request} from 'express';
+import {HttpError} from 'http-errors';
 import {z} from 'zod';
 import {LoggerConfig, loggerLevels} from './config-type';
 import {MiddlewareDefinition} from './middleware';
@@ -78,3 +79,20 @@ export function isLoggerConfig(logger: any): logger is LoggerConfig {
     Object.keys(loggerLevels).includes(logger.level) &&
     typeof logger.color === 'boolean';
 }
+
+export function getMessageFromError(error: Error): string {
+  return error instanceof z.ZodError
+    ? error.issues.map(({path, message}) =>
+      `${path.join('/')}: ${message}`).join('; ')
+    : error.message;
+}
+
+export function getStatusCodeFromError(error: Error): number {
+  if (error instanceof HttpError) {
+    return error.statusCode;
+  }
+  if (error instanceof z.ZodError) {
+    return 400;
+  }
+  return 500;
+}
diff --git a/src/result-handler.ts b/src/result-handler.ts
index c16e45d4..f823e885 100644
--- a/src/result-handler.ts
+++ b/src/result-handler.ts
@@ -1,7 +1,6 @@
 import {Request, Response} from 'express';
-import {HttpError} from 'http-errors';
 import {Logger} from 'winston';
-import {z} from 'zod';
+import {getStatusCodeFromError, getMessageFromError} from './helpers';
 
 type ApiResponse<T> = {
   status: 'success',
@@ -24,40 +23,24 @@ interface ResultHandlerParams {
 
 export type ResultHandler = (params: ResultHandlerParams) => void | Promise<void>;
 
-export const defaultResultHandler = ({error, request, response, input, output, logger}: ResultHandlerParams) => {
-  let resultJson: ApiResponse<any>;
-  if (error) {
-    let statusCode = 500;
-    if (error instanceof HttpError) {
-      statusCode = error.statusCode;
-    }
-    if (error instanceof z.ZodError) {
-      statusCode = 400;
-    }
-    if (statusCode === 500) {
-      logger.error(
-        'Internal server error\n' +
-        `${error.stack}\n` +
-        `URL: ${request.url}\n` +
-        `Payload: ${JSON.stringify(input, undefined, 2)}`,
-      );
-    }
-    response.status(statusCode);
-    resultJson = {
-      status: 'error',
-      error: {
-        message: error instanceof z.ZodError
-          ? error.issues.map(({path, message}) =>
-            `${path.join('/')}: ${message}`).join('; ')
-          : error.message,
-      }
-    };
-  } else {
-    response.status(200);
-    resultJson = {
-      status: 'success',
-      data: output
-    };
+export const defaultResultHandler: ResultHandler = ({error, request, response, input, output, logger}) => {
+  if (!error) {
+    const result: ApiResponse<typeof output> = { status: 'success', data: output };
+    response.status(200).json(result);
+    return;
   }
-  response.json(resultJson);
+  const statusCode = getStatusCodeFromError(error);
+  if (statusCode === 500) {
+    logger.error(
+      'Internal server error\n' +
+      `${error.stack}\n` +
+      `URL: ${request.url}\n` +
+      `Payload: ${JSON.stringify(input, undefined, 2)}`,
+    );
+  }
+  const result: ApiResponse<any> = {
+    status: 'error',
+    error: { message: getMessageFromError(error) }
+  };
+  response.status(statusCode).json(result);
 };
diff --git a/src/server.ts b/src/server.ts
index 0ae3eedd..166a81af 100644
--- a/src/server.ts
+++ b/src/server.ts
@@ -19,32 +19,29 @@ export function createServer(config: ConfigWithServer, routing: Routing): Server
   const logger = isLoggerConfig(config.logger) ? createLogger(config.logger) : config.logger;
   const app = express();
   const resultHandler = config.resultHandler || defaultResultHandler;
+  const jsonParser = config.server.jsonParser || express.json();
 
-  app.use([
-    config.server.jsonParser || express.json(),
-    (error, request, response, next) => {
-      if (error) {
-        resultHandler({
-          error, request, response, logger,
-          input: request.body,
-          output: null
-        });
-      } else {
-        next();
-      }
-    }
-  ]);
-
-  initRouting({app, routing, logger, config});
+  const jsonFailureHandler: express.ErrorRequestHandler = (error, request, response, next) => {
+    if (!error) { return next(); }
+    return resultHandler({
+      error, request, response, logger,
+      input: request.body,
+      output: null
+    });
+  };
 
-  app.use((request, response) => {
+  const lastResortHandler: express.RequestHandler = (request, response) => {
     resultHandler({
       request, response, logger,
       error: createHttpError(404, `Can not ${request.method} ${request.path}`),
       input: null,
       output: null
     });
-  });
+  };
+
+  app.use([jsonParser, jsonFailureHandler]);
+  initRouting({app, routing, logger, config});
+  app.use(lastResortHandler);
 
   return app.listen(config.server.listen, () => {
     logger.info(`Listening ${config.server.listen}`);


Test patch:
diff --git a/tests/unit/endpoint.spec.ts b/tests/unit/endpoint.spec.ts
index 7242b71a..e5e2d9f7 100644
--- a/tests/unit/endpoint.spec.ts
+++ b/tests/unit/endpoint.spec.ts
@@ -63,10 +63,10 @@ describe('Endpoint', () => {
           n: 453
         }
       };
-      const responseMock = {
-        set: jest.fn(),
-        status: jest.fn(),
-        json: jest.fn()
+      const responseMock: Record<string, jest.Mock> = {
+        set: jest.fn().mockImplementation(() => responseMock),
+        status: jest.fn().mockImplementation(() => responseMock),
+        json: jest.fn().mockImplementation(() => responseMock)
       };
       const configMock = {
         server: {
diff --git a/tests/unit/helpers.spec.ts b/tests/unit/helpers.spec.ts
index 28f5e1dc..c6cf8866 100644
--- a/tests/unit/helpers.spec.ts
+++ b/tests/unit/helpers.spec.ts
@@ -2,9 +2,11 @@ import {
   combineEndpointAndMiddlewareInputSchemas,
   extractObjectSchema,
   getInitialInput,
+  getMessageFromError,
+  getStatusCodeFromError,
   isLoggerConfig
 } from '../../src/helpers';
-import {createMiddleware, z} from '../../src';
+import {createMiddleware, z, createHttpError} from '../../src';
 import {Request} from 'express';
 
 describe('Helpers', () => {
@@ -217,4 +219,60 @@ describe('Helpers', () => {
       expect(subject.shape).toMatchSnapshot();
     });
   });
+
+  describe('getMessageFromError()', () => {
+    test('should compile a string from ZodError', () => {
+      const error = new z.ZodError([
+        {
+          code: 'invalid_type',
+          path: ['user', 'id'],
+          message: 'expected number, got string',
+          expected: 'number',
+          received: 'string'
+        },
+        {
+          code: 'invalid_type',
+          path: ['user', 'name'],
+          message: 'expected string, got number',
+          expected: 'string',
+          received: 'number'
+        }
+      ]);
+      expect(getMessageFromError(error)).toEqual(
+        'user/id: expected number, got string; user/name: expected string, got number'
+      );
+    });
+
+    test('should pass message from other error types', () => {
+      expect(getMessageFromError(createHttpError(502, 'something went wrong')))
+        .toEqual('something went wrong');
+      expect(getMessageFromError(new Error('something went wrong')))
+        .toEqual('something went wrong');
+    });
+  });
+
+  describe('getStatusCodeFromError()', () => {
+    test('should get status code from HttpError', () => {
+      expect(getStatusCodeFromError(createHttpError(403, 'Access denied')))
+        .toEqual(403);
+    });
+
+    test('should return 400 for ZodError', () => {
+      const error = new z.ZodError([
+        {
+          code: 'invalid_type',
+          path: ['user', 'id'],
+          message: 'expected number, got string',
+          expected: 'number',
+          received: 'string'
+        }
+      ]);
+      expect(getStatusCodeFromError(error)).toEqual(400);
+    });
+
+    test('should return 500 for other errors', () => {
+      expect(getStatusCodeFromError(new Error('something went wrong')))
+        .toEqual(500);
+    });
+  });
 });
diff --git a/tests/unit/result-handler.spec.ts b/tests/unit/result-handler.spec.ts
index 8cffd159..4c848135 100644
--- a/tests/unit/result-handler.spec.ts
+++ b/tests/unit/result-handler.spec.ts
@@ -15,8 +15,9 @@ describe('ResultHandler', () => {
         debug: jest.fn()
       };
       responseMock = {
-        status: jest.fn(),
-        json: jest.fn()
+        set: jest.fn().mockImplementation(() => responseMock),
+        status: jest.fn().mockImplementation(() => responseMock),
+        json: jest.fn().mockImplementation(() => responseMock)
       };
     });
 
diff --git a/tests/unit/routing.spec.ts b/tests/unit/routing.spec.ts
index b5dfd3a8..4b870f21 100644
--- a/tests/unit/routing.spec.ts
+++ b/tests/unit/routing.spec.ts
@@ -106,10 +106,10 @@ describe('Routing', () => {
           test: 123
         }
       };
-      const responseMock = {
-        set: jest.fn(),
-        status: jest.fn(),
-        json: jest.fn()
+      const responseMock: Record<string, jest.Mock> = {
+        set: jest.fn().mockImplementation(() => responseMock),
+        status: jest.fn().mockImplementation(() => responseMock),
+        json: jest.fn().mockImplementation(() => responseMock)
       };
       const nextMock = jest.fn();
       await routeHandler(requestMock as Request, responseMock as any as Response, nextMock);