RobinTail / express-zod-api

A Typescript library to help you get an API server up and running with I/O schema validation and custom middlewares in minutes.
https://ez.robintail.cz
MIT License
628 stars 33 forks source link

Catch errors in testMiddleware function #2153

Open williamgcampbell opened 8 hours ago

williamgcampbell commented 8 hours ago

It's common for middleware to throw exceptions when validating auth tokens or other request information. When this happens, the mocks available within testMiddleware won't be returned and can't be used to confirm behavior. This makes it difficult to test behavior of anything that happened up to the point of failure. For example:

import { z } from "zod";
import { Middleware, testMiddleware } from "express-zod-api";

const middleware = new Middleware({
  input: z.object({ test: z.string() }),
  handler: async ({ options, input: { test }, logger }) => {
     logger.info("logging something");
     throw new Error("something went wrong");
  },
});

// EXCEPTION THROWN HERE
const { output, responseMock, loggerMock } = await testMiddleware({
  middleware,
  requestProps: { method: "POST"},
  options: { prev: "accumulated" }, 
});
expect(loggerMock._getLogs().info).toHaveLength(1);  

To implement this we could provide an extra field to the function to indicate the intention of getting an error response back. Something like:

const { output, responseMock, loggerMock, error } = await testMiddleware({ 
  middleware,
  requestProps: { method: "POST"},
  options: { prev: "accumulated" }, 
  catchErrors: true, // when true will catch errors thrown by middleware and return them in the response
});
expect(loggerMock._getLogs().info).toHaveLength(1);
RobinTail commented 8 hours ago

This makes it difficult to test behavior

Is there anything why you can not do this:

await expect(
  () => testMiddleware( /* ... */ )
).rejects.toThrow("something went wrong");

// and then check logs too
expect(loggerMock._getLogs().info).toHaveLength(1);

https://vitest.dev/api/expect.html#rejects https://vitest.dev/api/expect.html#tothrowerror

@williamgcampbell ?

RobinTail commented 7 hours ago

My point is that you don't need catchErrors: true because there are already established methods of testing frameworks for asserting expectations on thrown errors.

testMiddleware() just calls Middleware::execute() that calls the specified handler. Therefore, if that one throws, testMiddleware() throws as well, so you could use standard testing approach here.

williamgcampbell commented 7 hours ago
await expect(
  () => testMiddleware( /* ... */ )
).rejects.toThrow("something went wrong");

// and then check logs too
expect(loggerMock._getLogs().info).toHaveLength(1);

In this example how do you gain access to loggerMock? It is initialized and returned by testMiddleware. The same goes for the request mock and response mock, which are all lost to the calling test if an exception is thrown.

RobinTail commented 7 hours ago

Ah! I get what you mean now, thank you for clarifying the problem, @williamgcampbell

RobinTail commented 6 hours ago

and return them in the response

it's a good idea, but I'd recommend to devote to consumer to decide how exactly that should happen. So that it would be configurable.

Therefore, I'm suggesting to extend your idea slightly, @williamgcampbell :

- catchErrors?: boolean
+ errorHandler?: (error: Error, response: Response) => void;

So, that errorHandler will act BOTH as a flag for catching AND as an implementation of responding with the error somehow. In basic scenario it can be:

(error, response) => response.end(error.message)

What do you think?

RobinTail commented 5 hours ago

Here is my final implementation, @williamgcampbell in #2154 :

https://github.com/RobinTail/express-zod-api/blob/4193ecaf740146d8ae5767b8f1d5ede426ffa486/src/testing.ts#L147-L150

I believe it should solve the issue for your case. Let me know how you feel about it.