Defer is not working on fragments
alexus37 opened this issue · 12 comments
Describe the bug
If I put the @defer
directive on fragment the fragment gets removed from the initial response but the context does not have a defer key to continue the execution.
Versions
graphql
version: 2.2
graphql-pro
version: 1.27
Steps to reproduce
Steps to reproduce the behavior
query = <<~'GRAPHQL'
query($id: ID!) {
node(id: $id) {
... on Issue {
title
...IssueBody @defer(label: "body")
}
}
}
fragment IssueBody on Issue {
body
}
GRAPHQL
response = execute_query(
query,
:$id => @issue.global_relay_id,
:viewer => @author
)
debugger
initial_data = response.data
deferals = []
# initial response only includes the title
assert_equal initial_data, { "node" => { "title" => @issue.title } }
assert response.query.context[:defer].present? # fails
Expected behavior
A clear and concise description of what you expected to happen.
response.query.context[:defer]
is present and can compute the remaining chunks
Actual behavior
response.query.context[:defer]
is nil
Hey, thanks for reporting this. I definitely expect that example to work!
I wrote up a basic script here, but it worked fine: https://gist.github.com/rmosolgo/2d25ec6be3ad20476a978dcbc9eb46d1
To debug further:
- Are there other cases that
@defer
does work in your system? For example, if you put it directly onbody
(egbody @defer
), or on an inline fragment (... @defer { body }
), do those work? - Could you share the code that hooks up
GraphQL::Pro::Defer
to your schema? (I assume it's there, or else the query would fail with a validation error, but maybe the setup code will give some clue.) - If you copy my script into your test suite (with the brand new schema definition), does it work for you? (If it doesn't, then maybe there are some Ruby-level moving parts in your environment to investigate.)
Let me know what you find!
Are there other cases that
@defer
does work in your system? For example, if you put it directly on body (eg body@defer
), or on an inline fragment (... @defer { body }
), do those work?
Putting the directive directly on a field works as expected. However, inline fragments do not work.
Could you share the code that hooks up GraphQL::Pro::Defer to your schema? (I assume it's there, or else the query would fail with a validation error, but maybe the setup code will give some clue.)
Sure, the code that hooks it up is just calling directive(Directives::Defer)
. The directive itself is basically what is in the docs with the addition to run defer only if run_defer_directive
is set in the context.
class Defer < GraphQL::Pro::Defer
def self.resolve(obj, arguments, context, &block)
# only use defer if the run_defer_directive flag is set on the context
if context[:run_defer_directive]
# While the query is running, store the batch executor to re-use later
context[:graphql_batch_executor] ||= GraphQL::Batch::Executor.current
super
else
yield
end
end
class Deferral < GraphQL::Pro::Defer::Deferral
def resolve
# Before calling the deferred execution,
# set GraphQL-Batch back up:
prev_executor = GraphQL::Batch::Executor.current
GraphQL::Batch::Executor.current ||= @context[:graphql_batch_executor]
super
ensure
# Clean up afterward:
GraphQL::Batch::Executor.current = prev_executor
end
end
end
If you copy my script into your test suite (with the brand new schema definition), does it work for you? (If it doesn't, then maybe there are some Ruby-level moving parts in your environment to investigate.)
If I add your script into my test env it works, even when using my Defer directive. :/
If I adjust your script to have nested fragments it fails for me:
require "bundler/inline"
gemfile do
gem "graphql", "~>1.13.22"
gem "graphql-pro", "~>1.27.0"
end
class Schema < GraphQL::Schema
class Issue < GraphQL::Schema::Object
field :title, String
field :body, String
end
class Repo < GraphQL::Schema::Object
field :issue, Issue
field :name, String
end
class Query < GraphQL::Schema::Object
field :repo, Repo
def repo
{
name: "my repo",
issue: {
title: "Failed Reticulated Splines",
body: "It encountered an error while reticulating splines"
}
}
end
end
query(Query)
use GraphQL::Pro::Defer
end
query_str = <<-GRAPHQL
{
repo {
...RepoFrag
}
}
fragment RepoFrag on Repo {
name
issue {
... IssueFrag
}
}
fragment IssueFrag on Issue {
title
... IssueBody @defer
}
fragment IssueBody on Issue {
body
}
GRAPHQL
res = Schema.execute(query_str)
if res.context[:defer].present?
puts "Defer found"
else
puts "Defer not found"
end
res.context[:defer].each do |deferral|
pp [:deferred, deferral.to_h]
end
Thanks for sharing those details! A couple of followup thoughts:
I noticed your original report uses GraphQL v2.2, but the second script in your latest comment uses GraphQL v1.13.22. Which one are you currently using in your app? (I wouldn't be surprised if some cases aren't handled well in v1.13.22... I'm open to addressing those bugs if you're currently using that version, but I want to make sure!)
I see your Defer
directive only takes effect when context[:run_defer_directive]
is present. I updated the gist with those modifications (https://gist.github.com/rmosolgo/2d25ec6be3ad20476a978dcbc9eb46d1) and it works alright for me (using v2.2 or v1.13.22), but only when I pass context: { run_defer_directive: true}
. In your original report, I don't see run_defer_directive: true
being added; does it work when you pass that flag?
Which one are you currently using in your app?
Sorry for any confusion! We are currently in the process of upgrading to the latest version. After testing both, it appears that there are issues with both 1.13 and 2.2.
I see your Defer directive only takes effect when context[:run_defer_directive] is present. I updated the gist with those modifications (https://gist.github.com/rmosolgo/2d25ec6be3ad20476a978dcbc9eb46d1) and it works alright for me (using v2.2 or v1.13.22), but only when I pass context: { run_defer_directive: true}. In your original report, I don't see run_defer_directive: true being added; does it work when you pass that flag?
That's a helpful tip! I took out that flag from the original post since it wasn't part of the default directive, but I included it during my testing. Additionally, the script I mentioned in my earlier comment simply utilizes use GraphQL::Pro::Defer
and is still encountering the issue with both versions.
Ok, thanks for clarifying! I also got your second script to fail on 2.2. I tracked it down to a bug in GraphQL-Ruby's runtime and replicated it in the test suite. I'll keep working up a fix in #4959
Thank you so much for the fix! Any idea when we can expect the release of version 2.3.4?
I just released it 🚢 !
How much work would it be to back port it to version 2.0?
I don't know, but I can give it a try if you're on that version. Would a 2.0.x backport be useful for you?
Yes that would help. We are currently on the latest 2.0 version.