-
-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support custom error message mappings #73
support custom error message mappings #73
Conversation
Thanks for the PR @megane42. I like the approach, but it seems that we need to duplicate the status codes to the HttpStatus and again for the message. What if we could include the custom message in the first object. Just an idea to either set the status or object app.useGlobalFilters(
new PrismaClientExceptionFilter(
httpAdapter,
// Prisma Error Code: HTTP Status Response
{
P2000: HttpStatus.BAD_REQUEST,
P2002: HttpStatus.CONFLICT,
P2025: { status: HttpStatus.NOT_FOUND, message: 'Resource not found' }
}
),
); |
0ba7767
to
3448272
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcjulian Thanks for your reply and your idea sounds better! So I've implemented it. Now it supports the config like this:
app.useGlobalFilters(
new PrismaClientExceptionFilter(httpAdapter, {
// Current way
P2000: HttpStatus.BAD_REQUEST,
// New way
P2001: {
statusCode: HttpStatus.BAD_REQUEST,
errorMessage: 'something went wrong',
},
// New way: either statusCode or errorMessage can be omitted
P2002: { statusCode: HttpStatus.BAD_REQUEST },
P2003: { errorMessage: 'something went wrong' },
}),
);
BTW I think it's better to see the code diff from current main
rather than from my last commit because it's easier to read what I did. I'm still leaving the old commits now just in case, but will squash them if the new code is OK.
@@ -12,7 +12,12 @@ import { Prisma } from '@prisma/client'; | |||
export declare type GqlContextType = 'graphql' | ContextType; | |||
|
|||
export type ErrorCodesStatusMapping = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I concern that this type is no longer ErrorCodesStatusMapping
strictly, because it handles not only "Status" but also "Message" now. Currently I haven't renamed it now, because it's hard to rename the already exported type.
3448272
to
786cbe4
Compare
@marcjulian Hi, let me know if there is anything I can do to help merge this PR. Sorry to rush you like this 🙏 |
Hi @megane42, I haven't forgotten about your PR. I will try to find time to review and merge your PR soon. |
Thanks for this PR, I really hope to get this finished ASAP because it's really important feature. Greeting to you Devs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the PR and keeping it backwards compatible!
Really appreciate your help @megane42
The custom error message can be testet in the latest dev release. npm i [email protected] |
resolves #72
Examples
useGlobalFilters()
APP_FILTER
or
Results
Before
After
Note
Only status mappings are used for judging whether or not to handle Prisma errors. Message mappings are not used for it. So if the app setting is like this:
the nest app returns
500 Internal server error
whenP2028
happens. Note thatP2000
P2002
P2025
are always handled implicitly because they are hard-coded.