Changing NotAuthorizedError to Be A Subclass of GraphQL::ExecutionError

[exaspark]

Changing NotAuthorizedError to Be A Subclass of GraphQL::ExecutionError

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[exAspArk]

Hey @sshaw,

Thank you for opening the issue!

with NotAuthorizedError as a superclass one gets proper error response formatting for free, as GraphQL Ruby will insert the exception's message into the "errors" property of the response.

These are 2 different behaviours: either fail the whole request with a Ruby exception (and return a 403 HTTP status, for example) or fail gracefully by returning a response with a GraphQL error. Although both of these use cases are being used, probably returning a GraphQL error is a more popular option (exactly what you suggested). For now, I'd prefer not making this breaking change since there is a way to customize the behaviour.

how do you feel about changing the default message of NotAuthorizedError from Query.queryName to Not authorized to access: Query.queryName (or similar)?

👍 good idea! Feel free to open a PR :)

[sshaw]

PR #32 opened.

These are 2 different behaviours: either fail the whole request with a Ruby exception (and return a > 403 HTTP status, for example) or fail gracefully by returning a response with a GraphQL error.

This is the problem (if I recall): a 403 is not returned. The exception is not handled and an HTTP 500 is returned.

[exAspArk]

Yeah, right. I usually use something like rescue_from from Rails to catch a specific exception (e.g. NotAuthorizedError) and return 403.

I merged #32. Thank you for your contribution! 🙌