[Issue/384] Better http errors from prisma

[makinox]

Description

• prismaErrorToHttpError function refactorized
• Added a custom error response for each prisma error in the current prisma documentation (link)
• Update utils tests

Fixes # (issue)

Type of change

Please select the option(s) that is more relevant.

• Code cleanup
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

[shayneczyzewski]

Hey there @makinox! Thanks for your efforts here! 🥳 Apologies for our delay- we recently had a team retreat last week. I will take a look at this PR this week though. Thanks and be in touch soon!

[shayneczyzewski]

Hi @makinox, thanks for taking on this issue! I can see a lot of time went into this, and I appreciate the thoroughness of all the error codes to messages. :) I had a couple of questions/comments:

1. What strategy did you use to map the Prisma error type to an HTTP response code?
2. I do worry that returning many of these messages to the client would result in a possible security vulnerability, and even of not exploitable, at the very least provide too much info either way.
   • Ref: https://owasp.org/www-community/Improper_Error_Handling
   • What if instead, we logged most of this information, but only returned to the user a detailed error message for a subset of them (only the ones in which the frontend actually had to tell the user something about)? I think from a security perspective it should all probably be opaque, and we just return a standard response for everything, but perhaps in some cases we can provide more?

Curious to hear your thoughts. Thanks again!

[makinox]

Hi @makinox, thanks for taking on this issue! I can see a lot of time went into this, and I appreciate the thoroughness of all the error codes to messages. :) I had a couple of questions/comments:

1. What strategy did you use to map the Prisma error type to an HTTP response code?

2. I do worry that returning many of these messages to the client would result in a possible security vulnerability, and even of not exploitable, at the very least provide too much info either way.

   • Ref: https://owasp.org/www-community/Improper_Error_Handling
   • What if instead, we logged most of this information, but only returned to the user a detailed error message for a subset of them (only the ones in which the frontend actually had to tell the user something about)? I think from a security perspective it should all probably be opaque, and we just return a standard response for everything, but perhaps in some cases we can provide more?

Curious to hear your thoughts. Thanks again!

Hello @shayneczyzewski, I hope you guys doing well.

1. I did not follow a particular strategy, I just did what I think it was the best, that is letting the user know what is happening instead of only responding general error, reading each prisma error and translating that to the wasp context; usually big frameworks always informs you what is happening, and personally I would like to add more details for some errors, but I think that is out of scope for now.
2. Yes I see your point, maybe for that case we could handle

• Common error (P1000 to P1017 errors)
• Prisma Client error (P2000 to P2034 errors)
• Prisma Migrate error (P3000 to P3022 errors)
• Prisma pull error (P3000 to P3002 errors)
  I think the problem with this general sectioned errors is that are less helpful, and it could save less time to the developer, from my personal experience I appreciate more detailed errors, but you have the last words guys

[shayneczyzewski]

Thanks for the reply, @makinox! I hope you are well too! :)

Yeah, how to handle how much info to provide back to the client is tricky. I will think on it a bit more/research it and we can also see if @Martinsos has any immediate thoughts on this too. My gut tells me less is more here (in terms of what info goes down the wire to the client), but certainly, we can make use of all your work so far with additional server-side logging. 🤔

Do we have any examples of how other frameworks handle this? I think I remember in Rails we get a ton of error info when in dev mode, but when in prod it just all becomes opaque. Maybe we can handle the errors differently depending on the mode?

[makinox]

Thanks for the reply, @makinox! I hope you are well too! :)

Yeah, how to handle how much info to provide back to the client is tricky. I will think on it a bit more/research it and we can also see if @Martinsos has any immediate thoughts on this too. My gut tells me less is more here (in terms of what info goes down the wire to the client), but certainly, we can make use of all your work so far with additional server-side logging. 🤔

Do we have any examples of how other frameworks handle this? I think I remember in Rails we get a ton of error info when in dev mode, but when in prod it just all becomes opaque. Maybe we can handle the errors differently depending on the mode?

Yeah, that solution sound very good, meaningful messages for developers and more generic responses for production users, in fact we could extend this implementation for other errors in the app.

[shayneczyzewski]

Hey again, @makinox! Hope things are good! @Martinsos and I chatted a bit yesterday about this PR and we were wondering what you think about the following.

What if instead of returning a unique HttpErrror per Prisma error code, we instead bucketed them into two broad categories: "validation" errors (to include DB constraints, invalid types, value too large, etc.) and everything else. I say validation in quotes since it may include a few that are not strictly validation, but this bucket would include anything that the frontend may want to notify the user about or act upon in some smart way.

Some examples of validation errors may include Prisma error codes like P2000, P2006, P2007, and many more. You can probably spot them quickly based on all your great work so far. :D For these, we could perhaps return 422 for most, if not all? Then, everything else that is not one of those whitelisted Prisma error codes is just a 500.

Additionally, we were wondering what information we could pass along from the Prisma error object as part of the response. Right now you have a String literal message (perhaps this can be pulled off the error object instead? maybe not if a security issue, but quote possibly 🤔 ), and maybe the response should also include other useful JSON metadata for the frontend to consume, like the column name(s) that the validation failed for, perhaps (e.g. meta.target from here: https://www.prisma.io/docs/reference/api-reference/error-reference#prismaclientknownrequesterror)?

Lastly, one idea that would give the caller more control was that we could perhaps pass a list of codes into the function to modify the whitelist somehow (like add or remove Prisma codes to include for special processing). So maybe in some context they always want code Pxxxx to be a 500, or maybe one code we left out of special handling they want included, etc.

Anyways, this may not make 100% sense (apologies), because I think we too have only a foggy idea of what it could look like, but maybe this is helpful and a decent direction to refactor towards in your mind? Curious to hear your thoughts. Thanks!

[makinox]

Hello @shayneczyzewski, yes jeje this is turning a little big confusing, maybe I can help you to simplify the problem.
I have 3 questions:

• What do you guys want?
• What do you guys expect to response back?
• These errors are for the final users or the developers?

[shayneczyzewski]

Hello @shayneczyzewski, yes jeje this is turning a little big confusing, maybe I can help you to simplify the problem. I have 3 questions:

• What do you guys want?
• What do you guys expect to response back?
• These errors are for the final users or the developers?

Hi @makinox, great questions! :) In short, we don't have concrete answers as to how this should be implemented and probably are not in a position right now to define a complete spec for this issue. As we were discussing it all together in the comments, we have come to realize it was probably only half-baked to start and not the best first issue for someone since it required a bit more research, experimentation, and discussion to arrive at a solution. We knew something was off with the current implementation, plus where we wanted to go in vague/general terms, but did not have a concrete interface in mind we can provide someone when we created the issue. Apologies for that. Would you be ok with hitting pause on this one until we have time to provide a more well-thought-out spec for this? At that time, we should be able to resume.

[shayneczyzewski]

And it also goes without saying, but I want to reiterate, we really do appreciate all your efforts on this so far @makinox! It will not be in vain. 🙏🏻

[Valetek]

Hello, I wanted to mention this issue, is it still planned to be integrated to Prisma yet ? or is it already in production ?

[Martinsos]

Hello, I wanted to mention this issue, is it still planned to be integrated to Prisma yet ? or is it already in production ?

Sorry what do you mean by that? This issue will need some revisitng to figure out what we should do with it, but it hasn't been much of a priority. I didn't really understand your question though, what do you mean "integrated to Prisma"?

[Valetek]

Sorry, I meant to ask if it was already released

Okay so for now this will probably not be implemented in the near future I guess, do you plan to have a discussion about it ?

[Martinsos]

Sorry, I meant to ask if it was already released

Okay so for now this will probably not be implemented in the near future I guess, do you plan to have a discussion about it ?

Not yet released! We kind of put it aside for now, since nobody complained about it, and I would have to remind myself a bit to have a valuable discussion. Do you have any thoughts on it you would like to share, why are you asking?

[Valetek]

Okay so it's almost ready ! No real thought on it, I just faced this case on my side, and this PR would give some help !

[Martinsos]

Okay so it's almost ready ! No real thought on it, I just faced this case on my side, and this PR would give some help !

No worries and thanks for letting us know, that helps us prioritize tasks like this more!