-
-
Notifications
You must be signed in to change notification settings - Fork 452
Don't conceal errors in resolvers for canFind, canModel and canQuery directives #2686
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
Don't conceal errors in resolvers for canFind, canModel and canQuery directives #2686
Conversation
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.
Nicely done. What about adding tests for @canRoot
? There is an existing CanRootDirectiveTest
.
Errors should be fixed and added an extra test for CanRoot |
Released with https://github.com/nuwave/lighthouse/releases/tag/v6.58.0, thank you. Please consider sponsoring. |
Thanks for the quick release! |
Context
While switching from
@can
to@canFind
recently, we noticed that exceptions thrown from within a mutation class were caught and rethrown as genericAuthorizationException
by theaction: EXCEPTION_NOT_AUTHORIZED
argument on can* directives.This is an issue for our use case as we perform sanity checks (business logic) in the mutation code and throw specific exception / error messages back to the user in some cases.
We do want to use
action: EXCEPTION_NOT_AUTHORIZED
to hideModelNotFoundException
from end users, but we still need to return specific exception / message / error codes to the user manually (when the@can*
directive actually passed) from the mutation class.Changes
With this PR, I moved resolving the field to outside of the authorization try-catch block, when applicable.
I kept the changes non breaking.
Note (unchanged behavior):
2.Exceptions throwing in @canRoot are never caught / modified as resolving happens before the authorization, outside of the try-catch
Unsure if this needs to be documented, happy to update the docs if needed?
Breaking changes
None