Skip to content
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

Handle error in resolve_type #5274

Merged

Conversation

xuanduc987
Copy link
Contributor

Without GraphQL::Dataloader installed, the error is handled by graphql-ruby but the location and path of the error is not set.
With GraphQL::Dataloader installed, the error is not handled by graphql-ruby

As I am not familiar with this codebase, this PR based on the assumption that continue_value will handle GraphQL::ExecutionError and GraphQL::UnauthorizedError value.

@xuanduc987 xuanduc987 changed the title Hanlde error in resolve_type Handle error in resolve_type Mar 13, 2025
@xuanduc987 xuanduc987 force-pushed the push-c23315765c0e7ae94f164f369e5d0729 branch from 2dff0da to 88cb872 Compare March 13, 2025 10:15
@rmosolgo
Copy link
Owner

Hey, thanks for proposing this fix. I'd like to understand better what went wrong. Could you please share an example backtrace from your application?

That would also help us include a test for this fix so that we can be sure it will keep working in future GraphQL-Ruby versions.

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
@xuanduc987 xuanduc987 force-pushed the push-c23315765c0e7ae94f164f369e5d0729 branch from 88cb872 to f850c9e Compare March 13, 2025 11:56
@xuanduc987
Copy link
Contributor Author

xuanduc987 commented Mar 13, 2025

I'm not sure where to put the test, so I added test cases to spec/graphql/execution_error_spec.rb

Here is the test result before the fix, the backtrace from our application is similar to this:

bundle exec rake test TEST=spec/graphql/execution_error_spec.rb

# Running tests with run options --seed 51684:

..FE.........

Finished tests in 0.138061s, 94.1613 tests/s, 108.6476 assertions/s.


Failure:
GraphQL::ExecutionError::when ExecutionError is raised in resolve_type#test_0001_return execution error with location and path [/Users/<username>/ghq/github.com/rmosolgo/graphql-ruby/spec/graphql/execution_error_spec.rb:446]
Minitest::Assertion: --- expected
+++ actual
@@ -1,5 +1 @@
-{"errors"=>
-  [{"message"=>"resolve_type",
-    "locations"=>[{"line"=>1, "column"=>3}],
-    "path"=>["test"]}],
- "data"=>{"test"=>nil}}
+{"errors"=>[{"message"=>"resolve_type"}]}


Error:
GraphQL::ExecutionError::when ExecutionError is raised in resolve_type::when using DataLoaders#test_0001_return execution error with location and path:
GraphQL::ExecutionError: resolve_type
    spec/graphql/execution_error_spec.rb:475:in `block (6 levels) in <top (required)>'
    lib/graphql/schema.rb:1142:in `resolve_type'
    lib/graphql/query.rb:157:in `block (2 levels) in initialize'
    lib/graphql/query.rb:393:in `resolve_type'
    lib/graphql/execution/interpreter/runtime.rb:859:in `block in resolve_type'
    lib/graphql/tracing/trace.rb:139:in `resolve_type'
    lib/graphql/execution/interpreter/runtime.rb:858:in `resolve_type'
    lib/graphql/execution/interpreter/runtime.rb:590:in `continue_field'
    lib/graphql/execution/interpreter/runtime.rb:399:in `block (2 levels) in evaluate_selection_with_resolved_keyword_args'
    lib/graphql/execution/interpreter/runtime.rb:832:in `after_lazy'
    lib/graphql/execution/interpreter/runtime.rb:392:in `block in evaluate_selection_with_resolved_keyword_args'
    lib/graphql/execution/interpreter/runtime.rb:712:in `call_method_on_directives'
    lib/graphql/execution/interpreter/runtime.rb:367:in `evaluate_selection_with_resolved_keyword_args'
    lib/graphql/execution/interpreter/runtime.rb:277:in `evaluate_selection'
    lib/graphql/execution/interpreter/runtime.rb:230:in `block (3 levels) in evaluate_selections'
    lib/graphql/dataloader.rb:300:in `block in spawn_job_fiber'
    lib/graphql/dataloader.rb:255:in `block in spawn_fiber'

13 tests, 15 assertions, 1 failures, 1 errors, 0 skips

@rmosolgo rmosolgo merged commit fcbfa86 into rmosolgo:master Mar 18, 2025
13 of 14 checks passed
@rmosolgo rmosolgo added this to the 2.4.15 milestone Mar 18, 2025
@rmosolgo
Copy link
Owner

Thanks for this improvement!

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

Successfully merging this pull request may close these issues.

None yet

2 participants