Skip to content

Commit 7937002

Browse files
committed
fix(incremental): emit only single completion when multiple deferred grouped field sets error
Replicates graphql/graphql-js@03cc5dc
1 parent ba48968 commit 7937002

File tree

3 files changed

+165
-11
lines changed

3 files changed

+165
-11
lines changed

src/graphql/execution/incremental_graph.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,17 +254,18 @@ def complete_deferred_fragment(
254254
def remove_deferred_fragment(
255255
self,
256256
deferred_fragment_record: DeferredFragmentRecord,
257-
) -> None:
258-
"""Remove a deferred fragment."""
257+
) -> bool:
258+
"""Check if deferred fragment exists and remove it in that case."""
259+
deferred_fragment_nodes = self._deferred_fragment_nodes
259260
try:
260-
deferred_fragment_node = self._deferred_fragment_nodes[
261-
deferred_fragment_record
262-
]
261+
deferred_fragment_node = deferred_fragment_nodes[deferred_fragment_record]
263262
except KeyError: # pragma: no cover
264-
return
263+
return False
265264
self._remove_pending(deferred_fragment_node)
265+
del deferred_fragment_nodes[deferred_fragment_record]
266266
for child in deferred_fragment_node.children: # pragma: no cover
267267
self.remove_deferred_fragment(child.deferred_fragment_record)
268+
return True
268269

269270
def remove_stream(self, stream_record: StreamRecord) -> None:
270271
"""Remove a stream record as no longer pending."""

src/graphql/execution/incremental_publisher.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,17 @@ def _handle_completed_deferred_grouped_field_set(
221221
):
222222
remove_deferred = self._incremental_graph.remove_deferred_fragment
223223
for deferred_fragment_record in record.deferred_fragment_records:
224+
if not remove_deferred(deferred_fragment_record):
225+
# multiple deferred grouped field sets could error for a fragment
226+
continue
224227
id_ = deferred_fragment_record.id
225-
if id_ is not None: # pragma: no branch
226-
append_completed(
227-
CompletedResult(id_, deferred_grouped_field_set_result.errors)
228-
)
229-
remove_deferred(deferred_fragment_record)
228+
if id_ is None: # pragma: no cover
229+
msg = "Missing deferred fragment record identifier."
230+
raise RuntimeError(msg)
231+
append_completed(
232+
CompletedResult(id_, deferred_grouped_field_set_result.errors)
233+
)
234+
remove_deferred(deferred_fragment_record)
230235
return
231236

232237
deferred_grouped_field_set_result = cast(

tests/execution/test_defer.py

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,6 +1580,154 @@ async def nulls_cross_defer_boundaries_null_first():
15801580
},
15811581
]
15821582

1583+
async def handles_multiple_erroring_deferred_grouped_field_sets():
1584+
"""Handles multiple erroring deferred grouped field sets"""
1585+
document = parse(
1586+
"""
1587+
query {
1588+
... @defer {
1589+
a {
1590+
b {
1591+
c {
1592+
someError: nonNullErrorField
1593+
}
1594+
}
1595+
}
1596+
}
1597+
... @defer {
1598+
a {
1599+
b {
1600+
c {
1601+
anotherError: nonNullErrorField
1602+
}
1603+
}
1604+
}
1605+
}
1606+
}
1607+
"""
1608+
)
1609+
result = await complete(
1610+
document,
1611+
{"a": {"b": {"c": {"nonNullErrorField": None}}}},
1612+
)
1613+
assert result == [
1614+
{
1615+
"data": {},
1616+
"pending": [
1617+
{"id": "0", "path": []},
1618+
{"id": "1", "path": []},
1619+
],
1620+
"hasNext": True,
1621+
},
1622+
{
1623+
"completed": [
1624+
{
1625+
"id": "0",
1626+
"errors": [
1627+
{
1628+
"message": "Cannot return null"
1629+
" for non-nullable field c.nonNullErrorField.",
1630+
"locations": [{"line": 7, "column": 23}],
1631+
"path": ["a", "b", "c", "someError"],
1632+
},
1633+
],
1634+
},
1635+
{
1636+
"id": "1",
1637+
"errors": [
1638+
{
1639+
"message": "Cannot return null"
1640+
" for non-nullable field c.nonNullErrorField.",
1641+
"locations": [{"line": 16, "column": 23}],
1642+
"path": ["a", "b", "c", "anotherError"],
1643+
},
1644+
],
1645+
},
1646+
],
1647+
"hasNext": False,
1648+
},
1649+
]
1650+
1651+
async def handles_multiple_erroring_deferred_grouped_field_sets_for_same_fragment():
1652+
"""Handles multiple erroring deferred grouped field sets for same fragment"""
1653+
document = parse(
1654+
"""
1655+
query {
1656+
... @defer {
1657+
a {
1658+
b {
1659+
someC: c {
1660+
d: d
1661+
}
1662+
anotherC: c {
1663+
d: d
1664+
}
1665+
}
1666+
}
1667+
}
1668+
... @defer {
1669+
a {
1670+
b {
1671+
someC: c {
1672+
someError: nonNullErrorField
1673+
}
1674+
anotherC: c {
1675+
anotherError: nonNullErrorField
1676+
}
1677+
}
1678+
}
1679+
}
1680+
}
1681+
"""
1682+
)
1683+
result = await complete(
1684+
document,
1685+
{"a": {"b": {"c": {"d": "d", "nonNullErrorField": None}}}},
1686+
)
1687+
assert result == [
1688+
{
1689+
"data": {},
1690+
"pending": [
1691+
{"id": "0", "path": []},
1692+
{"id": "1", "path": []},
1693+
],
1694+
"hasNext": True,
1695+
},
1696+
{
1697+
"incremental": [
1698+
{
1699+
"data": {"a": {"b": {"someC": {}, "anotherC": {}}}},
1700+
"id": "0",
1701+
},
1702+
{
1703+
"data": {"d": "d"},
1704+
"id": "0",
1705+
"subPath": ["a", "b", "someC"],
1706+
},
1707+
{
1708+
"data": {"d": "d"},
1709+
"id": "0",
1710+
"subPath": ["a", "b", "anotherC"],
1711+
},
1712+
],
1713+
"completed": [
1714+
{
1715+
"id": "1",
1716+
"errors": [
1717+
{
1718+
"message": "Cannot return null"
1719+
" for non-nullable field c.nonNullErrorField.",
1720+
"locations": [{"line": 19, "column": 23}],
1721+
"path": ["a", "b", "someC", "someError"],
1722+
},
1723+
],
1724+
},
1725+
{"id": "0"},
1726+
],
1727+
"hasNext": False,
1728+
},
1729+
]
1730+
15831731
async def nulls_cross_defer_boundaries_value_first():
15841732
"""Nulls cross defer boundaries, value first"""
15851733
document = parse(

0 commit comments

Comments
 (0)