Skip to content

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

Merged

Conversation

MarcEspiard
Copy link
Contributor

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

Context

While switching from @can to @canFind recently, we noticed that exceptions thrown from within a mutation class were caught and rethrown as generic AuthorizationException by the action: 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 hide ModelNotFoundException 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):

  1. Exceptions thrown in @canResolved are always caught / modified as resolving happens during the authorization.
    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

@MarcEspiard MarcEspiard changed the title Feat/dont conceal errors in resolvers for canFind, canModel and canQuery directives Don't conceal errors in resolvers for canFind, canModel and canQuery directives May 29, 2025
k0ka
k0ka previously requested changes May 30, 2025
@spawnia spawnia added the enhancement A feature or improvement label Jun 2, 2025
@MarcEspiard MarcEspiard requested a review from spawnia June 3, 2025 21:55
Copy link
Collaborator

@spawnia spawnia left a 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.

@spawnia
Copy link
Collaborator

spawnia commented Jun 4, 2025

@MarcEspiard
Copy link
Contributor Author

Errors should be fixed and added an extra test for CanRoot

@k0ka k0ka dismissed their stale review June 5, 2025 07:01

approved

@spawnia spawnia merged commit 2d7eb06 into nuwave:master Jun 5, 2025
62 checks passed
@spawnia
Copy link
Collaborator

spawnia commented Jun 5, 2025

Released with https://github.com/nuwave/lighthouse/releases/tag/v6.58.0, thank you. Please consider sponsoring.

@MarcEspiard MarcEspiard deleted the feat/dont-conceal-errors-in-resolvers branch June 5, 2025 08:01
@MarcEspiard
Copy link
Contributor Author

Thanks for the quick release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants