Improve default InvalidNullError handling #5257
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ravangen
reviewed
Feb 27, 2025
| end | ||
|
|
||
| # @deprecated always false | ||
| def parent_error? |
`InvalidNullError` was modeled as a `RuntimeTypeError` but also implemented `to_h` to act like an `ExecutionError`. It was a strange hybrid which made it harder for applications to change how the errors were handled. It also caused limitations such as not being easy to add `extensions` or `path` to the error in the response. This changes `InvalidNullError` to inherit from the base `GraphQL::Error` instead of `RuntimeTypeError`. Instead of adding the instance of `InvalidNullError` directly to `context.errors`, the default implementation is changed to create a new `ExecutionError` instead. This provides a simpler middle ground where applications can still choose to handle `InvalidNullError` exceptions differently (such as reporting them to their exception handler app) but keep the proper spec compliant field error response.
swalkinshaw
force-pushed
the
fix-invalid-null-error-handling
branch
from
dac5d2a to
b74c41b
Compare
|
Thanks for this improvement! |
|
Derp, sorry I overlooked #5256 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #5239
InvalidNullErrorwas modeled as aRuntimeTypeErrorbut also implementedto_hto act like anExecutionError. It was a strange hybrid which made it harder for applications to change how the errors were handled.It also caused limitations such as not being easy to add
extensionsorpathto the error in the response.This changes
InvalidNullErrorto inherit from the baseGraphQL::Errorinstead ofRuntimeTypeError. Instead of adding the instance ofInvalidNullErrordirectly tocontext.errors, the default implementation is changed to create a newExecutionErrorinstead.This provides a simpler middle ground where applications can still choose to handle
InvalidNullErrorexceptions differently (such as reporting them to their exception handler app) but keep the proper spec compliant field error response.Note: assuming this solution is good, I'll rebase this after #5256