notiz-dev / nestjs-prisma

Easy Prisma support for your NestJS application
https://nestjs-prisma.dev
MIT License
587 stars 50 forks source link

Simplify error handling flow #39

Closed Loskir closed 1 year ago

Loskir commented 2 years ago

Fixes #38

It maybe naive, but it should be enough to just throw a new error instead, right? Other exception filters will catch it and wrap it properly?

Or should we return it instead? Couldn't find any info in the docs

marcjulian commented 2 years ago

@Loskir I tested it in the express (basic) and fastify example. Fastify works fine, however for express the new exception is not handled correctly. The server is not reachable after the exception is thrown in the filter. I tried to return the exception instead, but then the request won't receive any response.

I guess thats the reason why response.send(..) is used to resolve correctly with express and fastify.

Loskir commented 2 years ago

Thank you for testing! I guess we can change it back to response but replacing .send() with .json() to have correct response type. We can also probably do super.catch(exception) to delegate the handling to the default handler

Loskir commented 2 years ago

@marcjulian hello! I've changed it to super.catch, it should work. Could you test it again please?

marcjulian commented 1 year ago

@Loskir thanks for cleaning up the exception filter. That is much better, tested it with Express and Fastify and it works the same except it turns now application/json.