Repository: RobinTail/express-zod-api
Pull request #819 (2023-02-14 00:12:20)

The proposed solution to problem #787
Related to the discussion #787 

Should enable the developer using `zod` as a tool within Endpoint's handler without considering its possible parsing/validation errors as IO related ones, leading to the status code `500` instead of `400` in that case.

@TheWisestOne 

### The approach

1. I am making an `InputValidationError`, consistent with `OutputValidationError`.
2. I separate the input parsing from the execution.
3. Validation errors (ZodError) catched before running middlewares and Endpoint handlers are transformed into `InputValidationError`
  - It also contains `originalError` prop for the further processing if needed.
  - However the `message` prop is already prepared for being human readable.
5. Thus, your `ResultHandler` now can be sure that any ZodError is actually NOT input validation error and can respond with `500`.

Inspired by #794.

### ToDo

- needs more tests
- need to expose error processing helpers in index ✅ 

RobinTail: @TheWisestOne , please review 
RobinTail: > EndpointHandlerError is a very generic name, and might be better renamed EndpointHandlerZodError or something like that to clarify that any other errors thrown by the endpoint don't get wrapped in this error class.

@TheWisestOne, yes, agree. That need to be fixed.
RobinTail: > and in the catch block we would throw a new `InputValidationError`

there was a reason why I try to refrain from making a validation error — it will be a breaking change for existing custom `ResultHandler`s made by developers.
Since `ZodError` is for a long time works as a synonym for the I/O validation error. 

But I also with your point on consistency.

Need to think on it a little bit.
Sorry that it takes a bit long.
TheWisestOne: > > and in the catch block we would throw a new `InputValidationError`
> 
> there was a reason why I tried to refrain from making another validation error — it would be a breaking change for existing custom `ResultHandler`s made by developers. Since `ZodError` is for a long time works as a synonym for the input validation error.
> 
> But I also agree with your point on consistency.
> 
> Need to think on it a little bit. Sorry that it takes a bit long.

ah, an interesting point. One thing to note is that my attempt at this fix (https://github.com/RobinTail/express-zod-api/pull/794) actually is non-breaking for people using an `instanceof ZodError` check in their ResultHandler. 
This is because:
```     throw createHttpError(400, makeErrorFromAnything(e)); ``` still creates an error that is an instance of a `ZodError` if `e` was a zod error. I still see some of the benifits of your approach, but just wanted to point out that we might be able to do something non-breaking for existing `ResultHandler`s
RobinTail: I'm going to tackle this problem once again keeping in mind that the breaking change is acceptable, since I'm going to make v9 with another one.
RobinTail: I wanna go with the `InputValidationError`, for of consistency.
RobinTail: I wanna leave the utilization of `createHttpError` for the developer's decision on the final implementation and customizations.
RobinTail: OK. Now I'm happy with it.
I hope it will also fulfills your needs, @TheWisestOne 

Anyway, I tested that throwing ZodError inside the Endpoint is not considered as IO validation issue.
So you could use zod for your database validations.

Patch:
diff --git a/src/common-helpers.ts b/src/common-helpers.ts
index 3da71b5e..f3d13a45 100644
--- a/src/common-helpers.ts
+++ b/src/common-helpers.ts
@@ -8,6 +8,7 @@ import {
   LoggerConfig,
   loggerLevels,
 } from "./config-type";
+import { InputValidationError, OutputValidationError } from "./errors";
 import { IOSchema } from "./io-schema";
 import { getMeta } from "./metadata";
 import { AuxMethod, Method } from "./method";
@@ -99,6 +100,10 @@ export function getMessageFromError(error: Error): string {
       )
       .join("; ");
   }
+  if (error instanceof OutputValidationError) {
+    const hasFirstField = error.originalError.issues[0].path.length > 0;
+    return `output${hasFirstField ? "/" : ": "}${error.message}`;
+  }
   return error.message;
 }
 
@@ -106,7 +111,7 @@ export function getStatusCodeFromError(error: Error): number {
   if (error instanceof HttpError) {
     return error.statusCode;
   }
-  if (error instanceof z.ZodError) {
+  if (error instanceof InputValidationError) {
     return 400;
   }
   return 500;
diff --git a/src/endpoint.ts b/src/endpoint.ts
index 0023fddf..0d03be74 100644
--- a/src/endpoint.ts
+++ b/src/endpoint.ts
@@ -5,6 +5,7 @@ import { ApiResponse } from "./api-response";
 import { CommonConfig } from "./config-type";
 import {
   IOSchemaError,
+  InputValidationError,
   OutputValidationError,
   ResultHandlerError,
 } from "./errors";
@@ -12,7 +13,6 @@ import {
   FlatObject,
   getActualMethod,
   getInput,
-  getMessageFromError,
   hasTopLevelTransformingEffect,
   makeErrorFromAnything,
 } from "./common-helpers";
@@ -222,16 +222,10 @@ export class Endpoint<
     try {
       return await this.outputSchema.parseAsync(output);
     } catch (e) {
-      const error =
-        e instanceof z.ZodError
-          ? new z.ZodError(
-              e.issues.map(({ path, ...rest }) => ({
-                ...rest,
-                path: (["output"] as typeof path).concat(path),
-              }))
-            )
-          : makeErrorFromAnything(e);
-      throw new OutputValidationError(getMessageFromError(error));
+      if (e instanceof z.ZodError) {
+        throw new OutputValidationError(e);
+      }
+      throw e;
     }
   }
 
@@ -254,10 +248,19 @@ export class Endpoint<
       if (method === "options" && def.type === "proprietary") {
         continue;
       }
+      let finalInput: any;
+      try {
+        finalInput = await def.input.parseAsync(input);
+      } catch (e) {
+        if (e instanceof z.ZodError) {
+          throw new InputValidationError(e);
+        }
+        throw e;
+      }
       Object.assign(
         options,
         await def.middleware({
-          input: await def.input.parseAsync(input),
+          input: finalInput,
           options,
           request,
           response,
@@ -285,9 +288,17 @@ export class Endpoint<
     options: any;
     logger: Logger;
   }) {
+    let finalInput: z.output<IN>; // final input types transformations for handler
+    try {
+      finalInput = (await this.inputSchema.parseAsync(input)) as z.output<IN>;
+    } catch (e) {
+      if (e instanceof z.ZodError) {
+        throw new InputValidationError(e);
+      }
+      throw e;
+    }
     return this.handler({
-      // final input types transformations for handler
-      input: (await this.inputSchema.parseAsync(input)) as z.output<IN>,
+      input: finalInput,
       options,
       logger,
     });
diff --git a/src/errors.ts b/src/errors.ts
index 1c36ef26..5181a82b 100644
--- a/src/errors.ts
+++ b/src/errors.ts
@@ -1,23 +1,49 @@
+import { ZodError } from "zod";
+import { getMessageFromError } from "./common-helpers";
+
+/** @desc An error related to the wrong Routing declaration */
 export class RoutingError extends Error {
   public override name = "RoutingError";
 }
 
+/** @desc An error related to the issues of using DependsOnMethod class */
 export class DependsOnMethodError extends RoutingError {
   public override name = "DependsOnMethodError";
 }
 
+/** @desc An error related to the generating of the documentation */
 export class OpenAPIError extends Error {
   public override name = "OpenAPIError";
 }
 
+/** @desc An error related to the input and output schemas declaration */
 export class IOSchemaError extends Error {
   public override name = "IOSchemaError";
 }
 
+/** @desc An error of validating the Endpoint handler's returns against the Endpoint output schema */
 export class OutputValidationError extends IOSchemaError {
   public override name = "OutputValidationError";
+  public readonly originalError: ZodError;
+
+  constructor(originalError: ZodError) {
+    super(getMessageFromError(originalError));
+    this.originalError = originalError;
+  }
+}
+
+/** @desc An error of validating the input sources against the Middleware or Endpoint input schema */
+export class InputValidationError extends IOSchemaError {
+  public override name = "InputValidationError";
+  public readonly originalError: ZodError;
+
+  constructor(originalError: ZodError) {
+    super(getMessageFromError(originalError));
+    this.originalError = originalError;
+  }
 }
 
+/** @desc An error related to the execution of ResultHandler */
 export class ResultHandlerError extends Error {
   public override name = "ResultHandlerError";
   public readonly originalError: Error | undefined;
diff --git a/src/index.ts b/src/index.ts
index d81b6ee3..db5dc1a7 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -3,7 +3,11 @@ export { AbstractEndpoint } from "./endpoint";
 export { Method } from "./method";
 export { EndpointsFactory, defaultEndpointsFactory } from "./endpoints-factory";
 export { IOSchema } from "./io-schema";
-export { FlatObject } from "./common-helpers";
+export {
+  FlatObject,
+  getMessageFromError,
+  getStatusCodeFromError,
+} from "./common-helpers";
 export { createApiResponse } from "./api-response";
 export { createLogger } from "./logger";
 export { createMiddleware } from "./middleware";
@@ -13,7 +17,13 @@ export { ServeStatic } from "./serve-static";
 export { Routing } from "./routing";
 export { createServer, attachRouting } from "./server";
 export { OpenAPI } from "./open-api";
-export { OpenAPIError, DependsOnMethodError, RoutingError } from "./errors";
+export {
+  OpenAPIError,
+  DependsOnMethodError,
+  RoutingError,
+  OutputValidationError,
+  InputValidationError,
+} from "./errors";
 export { withMeta } from "./metadata";
 export { testEndpoint } from "./mock";
 export { Client } from "./client";


Test patch:
diff --git a/tests/system/__snapshots__/system.spec.ts.snap b/tests/system/__snapshots__/system.spec.ts.snap
index 45c178ac..09b9d1bc 100644
--- a/tests/system/__snapshots__/system.spec.ts.snap
+++ b/tests/system/__snapshots__/system.spec.ts.snap
@@ -27,6 +27,15 @@ exports[`App Protocol Should fail when missing content type header 1`] = `
 }
 `;
 
+exports[`App Validation Problem 787: Should NOT treat ZodError thrown from within the handler as IOSchema validation error 1`] = `
+{
+  "error": {
+    "message": "Expected number, received string",
+  },
+  "status": "error",
+}
+`;
+
 exports[`App Validation Should fail on handler input type mismatch 1`] = `
 {
   "error": {
diff --git a/tests/system/system.spec.ts b/tests/system/system.spec.ts
index 73efd3de..af202cf9 100644
--- a/tests/system/system.spec.ts
+++ b/tests/system/system.spec.ts
@@ -45,16 +45,45 @@ describe("App", () => {
               throw new Error("I am faulty");
             },
           })
-        ).build({
-          method: "get",
-          input: z.object({}),
-          output: z.object({
-            test: z.string(),
-          }),
-          handler: async () => ({
-            test: "Should not work",
+        )
+          .addMiddleware(
+            createMiddleware({
+              input: z.object({
+                mwError: z
+                  .any()
+                  .optional()
+                  .transform((value) => {
+                    if (value) {
+                      throw new Error(
+                        "Custom error in the Middleware input validation"
+                      );
+                    }
+                  }),
+              }),
+              middleware: async () => ({}),
+            })
+          )
+          .build({
+            method: "get",
+            input: z.object({
+              epError: z
+                .any()
+                .optional()
+                .transform((value) => {
+                  if (value) {
+                    throw new Error(
+                      "Custom error in the Endpoint input validation"
+                    );
+                  }
+                }),
+            }),
+            output: z.object({
+              test: z.string(),
+            }),
+            handler: async () => ({
+              test: "Should not work",
+            }),
           }),
-        }),
         test: new EndpointsFactory(defaultResultHandler)
           .addMiddleware(
             createMiddleware({
@@ -90,13 +119,19 @@ describe("App", () => {
             handler: async ({
               input: { key, something },
               options: { user, permissions, method },
-            }) => ({
-              anything: something === "joke" ? 300 : -100500,
-              doubleKey: key.repeat(2),
-              userId: user.id,
-              permissions,
-              method,
-            }),
+            }) => {
+              // Problem 787: should lead to ZodError that is NOT considered as the IOSchema validation error
+              if (something === "internal_zod_error") {
+                z.number().parse("");
+              }
+              return {
+                anything: something === "joke" ? 300 : -100500,
+                doubleKey: key.repeat(2),
+                userId: user.id,
+                permissions,
+                method,
+              };
+            },
           }),
       },
     };
@@ -250,6 +285,42 @@ describe("App", () => {
         "An error occurred while serving the result: I am faulty."
       );
     });
+
+    test("Should treat custom errors in middleware input validations as they are", async () => {
+      const response = await fetch(
+        "http://127.0.0.1:8055/v1/faulty?mwError=1",
+        {
+          method: "GET",
+          headers: {
+            "Content-Type": "application/json",
+          },
+        }
+      );
+      expect(response.status).toBe(500);
+      const text = await response.text();
+      expect(text).toBe(
+        "An error occurred while serving the result: I am faulty.\n" +
+          "Original error: Custom error in the Middleware input validation."
+      );
+    });
+
+    test("Should treat custom errors in middleware input validations as they are", async () => {
+      const response = await fetch(
+        "http://127.0.0.1:8055/v1/faulty?epError=1",
+        {
+          method: "GET",
+          headers: {
+            "Content-Type": "application/json",
+          },
+        }
+      );
+      expect(response.status).toBe(500);
+      const text = await response.text();
+      expect(text).toBe(
+        "An error occurred while serving the result: I am faulty.\n" +
+          "Original error: Custom error in the Endpoint input validation."
+      );
+    });
   });
 
   describe("Protocol", () => {
@@ -367,5 +438,21 @@ describe("App", () => {
       const json = await response.json();
       expect(json).toMatchSnapshot();
     });
+
+    test("Problem 787: Should NOT treat ZodError thrown from within the handler as IOSchema validation error", async () => {
+      const response = await fetch("http://127.0.0.1:8055/v1/test", {
+        method: "POST",
+        headers: {
+          "Content-Type": "application/json",
+        },
+        body: JSON.stringify({
+          key: "123",
+          something: "internal_zod_error",
+        }),
+      });
+      expect(response.status).toBe(500);
+      const json = await response.json();
+      expect(json).toMatchSnapshot();
+    });
   });
 });
diff --git a/tests/unit/common-helpers.spec.ts b/tests/unit/common-helpers.spec.ts
index 3aa6033b..0ce7f9d2 100644
--- a/tests/unit/common-helpers.spec.ts
+++ b/tests/unit/common-helpers.spec.ts
@@ -15,7 +15,7 @@ import {
   isValidDate,
   makeErrorFromAnything,
 } from "../../src/common-helpers";
-import { createHttpError, withMeta, z } from "../../src";
+import { InputValidationError, createHttpError, withMeta, z } from "../../src";
 import { Request } from "express";
 
 describe("Common Helpers", () => {
@@ -271,8 +271,24 @@ describe("Common Helpers", () => {
       ).toEqual(403);
     });
 
-    test("should return 400 for ZodError", () => {
-      const error = new z.ZodError([
+    test("should return 400 for InputValidationError", () => {
+      const error = new InputValidationError(
+        new z.ZodError([
+          {
+            code: "invalid_type",
+            path: ["user", "id"],
+            message: "expected number, got string",
+            expected: "number",
+            received: "string",
+          },
+        ])
+      );
+      expect(getStatusCodeFromError(error)).toEqual(400);
+    });
+
+    test.each([
+      new Error("something went wrong"),
+      new z.ZodError([
         {
           code: "invalid_type",
           path: ["user", "id"],
@@ -280,14 +296,9 @@ describe("Common Helpers", () => {
           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
-      );
+      ]),
+    ])("should return 500 for other errors %#", (error) => {
+      expect(getStatusCodeFromError(error)).toEqual(500);
     });
   });
 
diff --git a/tests/unit/errors.spec.ts b/tests/unit/errors.spec.ts
index d565f7ac..23f6406f 100644
--- a/tests/unit/errors.spec.ts
+++ b/tests/unit/errors.spec.ts
@@ -1,6 +1,8 @@
+import { ZodError } from "zod";
 import { DependsOnMethodError, OpenAPIError, RoutingError } from "../../src";
 import {
   IOSchemaError,
+  InputValidationError,
   OutputValidationError,
   ResultHandlerError,
 } from "../../src/errors";
@@ -50,16 +52,45 @@ describe("Errors", () => {
   });
 
   describe("OutputValidationError", () => {
+    const zodError = new ZodError([]);
+
     test("should be an instance of IOSchemaError and Error", () => {
-      expect(new OutputValidationError("test")).toBeInstanceOf(IOSchemaError);
-      expect(new OutputValidationError("test")).toBeInstanceOf(Error);
+      expect(new OutputValidationError(zodError)).toBeInstanceOf(IOSchemaError);
+      expect(new OutputValidationError(zodError)).toBeInstanceOf(Error);
     });
 
     test("should have the name matching its class", () => {
-      expect(new OutputValidationError("test").name).toBe(
+      expect(new OutputValidationError(zodError).name).toBe(
         "OutputValidationError"
       );
     });
+
+    test("should have .originalError property matching the one used for constructing", () => {
+      expect(new OutputValidationError(zodError).originalError).toEqual(
+        zodError
+      );
+    });
+  });
+
+  describe("InputValidationError", () => {
+    const zodError = new ZodError([]);
+
+    test("should be an instance of IOSchemaError and Error", () => {
+      expect(new InputValidationError(zodError)).toBeInstanceOf(IOSchemaError);
+      expect(new InputValidationError(zodError)).toBeInstanceOf(Error);
+    });
+
+    test("should have the name matching its class", () => {
+      expect(new InputValidationError(zodError).name).toBe(
+        "InputValidationError"
+      );
+    });
+
+    test("should have .originalError property matching the one used for constructing", () => {
+      expect(new InputValidationError(zodError).originalError).toEqual(
+        zodError
+      );
+    });
   });
 
   describe("ResultHandlerError", () => {
diff --git a/tests/unit/result-handler.spec.ts b/tests/unit/result-handler.spec.ts
index a790de36..da2e6fc7 100644
--- a/tests/unit/result-handler.spec.ts
+++ b/tests/unit/result-handler.spec.ts
@@ -1,5 +1,11 @@
 import { Request, Response } from "express";
-import { createHttpError, defaultResultHandler, withMeta, z } from "../../src";
+import {
+  InputValidationError,
+  createHttpError,
+  defaultResultHandler,
+  withMeta,
+  z,
+} from "../../src";
 import { metaProp } from "../../src/metadata";
 
 let loggerMock: any;
@@ -61,15 +67,17 @@ describe("ResultHandler", () => {
         url: "http://something/v1/anything",
       };
       defaultResultHandler.handler({
-        error: new z.ZodError([
-          {
-            code: "invalid_type",
-            message: "Expected string, got number",
-            path: ["something"],
-            expected: "string",
-            received: "number",
-          },
-        ]),
+        error: new InputValidationError(
+          new z.ZodError([
+            {
+              code: "invalid_type",
+              message: "Expected string, got number",
+              path: ["something"],
+              expected: "string",
+              received: "number",
+            },
+          ])
+        ),
         input: { something: 453 },
         output: { anything: 118 },
         request: requestMock as Request,