overblog / GraphQLBundle

This bundle provides tools to build a complete GraphQL API server in your Symfony App.
MIT License
788 stars 222 forks source link

Non-UserError/UserWarning exceptions are considered as internal #1194

Open theofidry opened 1 month ago

theofidry commented 1 month ago
Q A
Bug report? not sure
Feature request? not sure
BC Break report? not sure
RFC? no
Version/Branch 1.6.0

If I understand the error handling correctly based on Overblog\GraphQLBundle\Error\ErrorHandler, we have:

I am wondering a bit of the usage of rethrow_internal_exceptions as I assume on a production system you would not want to return a non-GraphQL response to a GraphQL query, so I was under the impression this was either for exception specific use cases or for dev/tests, as it is easier to deal with the original exception.

However, based on the implementation of the error handler, this cannot be because as soon as you throw an exception, if it is not a UserError or UserWarning, with rethrow_internal_exceptions = true, it will rethrow. As a result, something like a validation error (a Overblog\GraphQLBundle\Validator\Exception\ArgumentsValidationException) will bubble up instead of being formatted nicely.

Wouldn't it make more sense to consider as "internal" exceptions that are not ClientAware or ClientAware with isClientSafe = false instead?