Skip to content

Commit 9be181c

Browse files
committed
Fix RuntimeError "Invalid state while adding deferred fragment node"
`IncrementalGraph._add_deferred_fragment_node` raised an internal RuntimeError while executing a spec-valid query when two sibling top-level `@defer` fragments — each resolving a field that errors and each containing its own nested `@defer` — were executed with async resolvers. When a deferred field errors, that fragment's execution group fails and the (parentless, root) deferred fragment is removed from the root nodes. A sibling execution group completing afterwards then discovers its nested `@defer` and, in `_add_incremental_data_records`, recurses up to the now-removed parentless fragment, reaching the `# pragma: no cover` branch and raising. Return instead of raising when reaching a parentless fragment that is no longer a root node: its subtree is no longer being delivered, so there is nothing to attach it to. This mirrors the `future.cancelled()` guard added to `_enqueue` (a race with a stopping/removed consumer). Adds a regression test that reproduces the crash prior to this change. Fixes #271
1 parent 56d4338 commit 9be181c

2 files changed

Lines changed: 85 additions & 3 deletions

File tree

‎src/graphql/execution/incremental_graph.py‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,16 @@ def _add_deferred_fragment_node(
257257
return
258258
parent = deferred_fragment_record.parent
259259
if parent is None:
260-
if initial_result_children is None: # pragma: no cover
261-
msg = "Invalid state while adding deferred fragment node."
262-
raise RuntimeError(msg)
260+
if initial_result_children is None:
261+
# The fragment has already been removed from the root nodes
262+
# (e.g. its execution group failed on a resolver error) while a
263+
# sibling execution group that references its subtree is still
264+
# completing. There is nothing to attach the record to and its
265+
# subtree is no longer being delivered, so drop it rather than
266+
# treating this as an invalid state. Mirrors the
267+
# `future.cancelled()` guard in `_enqueue` — a race with a
268+
# stopping/removed consumer.
269+
return
263270
initial_result_children[deferred_fragment_record] = None
264271
return
265272
parent.children[deferred_fragment_record] = None

‎tests/execution/test_defer.py‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3163,3 +3163,78 @@ async def author(_info):
31633163
with pytest.raises(AbortError, match="This operation was aborted"):
31643164
await anext_task
31653165
assert items_source.aclose_finished
3166+
3167+
3168+
def describe_defer_directive_with_errors_and_nested_defer():
3169+
"""Regression tests for graphql-python/graphql-core#271."""
3170+
3171+
async def does_not_reach_invalid_state_when_sibling_fragments_error():
3172+
"""Sibling erroring ``@defer`` fragments with nested ``@defer`` must not crash.
3173+
3174+
Two sibling top-level ``@defer`` fragments that each resolve a distinct
3175+
erroring field and each contain a nested ``@defer`` used to crash the
3176+
incremental graph with "Invalid state while adding deferred fragment
3177+
node": a failing
3178+
execution group removes its parentless (root) deferred fragment, and a
3179+
sibling execution group completing afterwards then recursed up into the
3180+
removed record and hit the invariant. See issue #271.
3181+
"""
3182+
3183+
async def resolve_fast(_source, _info) -> str:
3184+
await sleep(0)
3185+
return "fast"
3186+
3187+
def resolve_error(_source, _info) -> str:
3188+
raise RuntimeError("bad")
3189+
3190+
async def resolve_obj(_source, _info) -> dict:
3191+
await sleep(0)
3192+
return {}
3193+
3194+
obj_type: GraphQLObjectType = GraphQLObjectType(
3195+
"Obj",
3196+
lambda: {
3197+
"fast": GraphQLField(GraphQLString, resolve=resolve_fast),
3198+
"boom": GraphQLField(GraphQLString, resolve=resolve_error),
3199+
"boomNonNull": GraphQLField(
3200+
GraphQLNonNull(GraphQLString), resolve=resolve_error
3201+
),
3202+
"child": GraphQLField(obj_type, resolve=resolve_obj),
3203+
},
3204+
)
3205+
local_schema = GraphQLSchema(
3206+
GraphQLObjectType(
3207+
"Query", {"obj": GraphQLField(obj_type, resolve=resolve_obj)}
3208+
)
3209+
)
3210+
# The two siblings must error on DISTINCT fields (`boom` vs `boomNonNull`):
3211+
# erroring on the same field in both does not reproduce the crash. The
3212+
# nested `@defer` selections may be identical.
3213+
document = parse(
3214+
"""
3215+
query {
3216+
obj {
3217+
... @defer(label: "a") {
3218+
boom
3219+
child { ... @defer(label: "b") { fast } }
3220+
}
3221+
... @defer(label: "c") {
3222+
boomNonNull
3223+
child { ... @defer(label: "d") { fast } }
3224+
}
3225+
}
3226+
}
3227+
"""
3228+
)
3229+
3230+
for early_execution in (False, True):
3231+
result = experimental_execute_incrementally(
3232+
local_schema, document, {}, enable_early_execution=early_execution
3233+
)
3234+
assert is_awaitable(result)
3235+
result = await result
3236+
assert isinstance(result, ExperimentalIncrementalExecutionResults)
3237+
# Draining the stream must terminate without raising
3238+
# "Invalid state while adding deferred fragment node".
3239+
async for _patch in result.subsequent_results:
3240+
pass

0 commit comments

Comments
 (0)