Skip to content

Commit 7f90200

Browse files
ben-xoclaude
andauthored
fix: skip cancelled futures in DataLoader error handler (#4300)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b68998e commit 7f90200

File tree

3 files changed

+57
-0
lines changed

3 files changed

+57
-0
lines changed

RELEASE.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Release type: patch
2+
3+
This release fixes an `InvalidStateError` crash in the DataLoader when a batch
4+
load function raises an exception and some futures in the batch have already been
5+
cancelled (e.g. due to client disconnection).
6+
7+
The error handler in `dispatch_batch` now skips cancelled futures before calling
8+
`set_exception`, matching the guard that already exists in the success path
9+
(added in #2339).

strawberry/dataloader.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,8 @@ async def dispatch_batch(loader: DataLoader, batch: Batch) -> None:
260260
task.future.set_result(value)
261261
except Exception as e: # noqa: BLE001
262262
for task in batch.tasks:
263+
if task.future.cancelled():
264+
continue
263265
task.future.set_exception(e)
264266

265267

tests/test_dataloaders.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,52 @@ async def idx(keys: list[int]) -> list[int]:
359359
assert value_d == 4
360360

361361

362+
@pytest.mark.asyncio
363+
async def test_cancelled_future_with_failing_loader():
364+
"""When the load function raises and some futures are already cancelled,
365+
the error handler should skip cancelled futures instead of raising
366+
InvalidStateError.
367+
368+
This reproduces a real-world scenario where a client disconnects
369+
mid-request: some DataLoader futures get cancelled by the event loop,
370+
then the batch load function fails (e.g. because the executor is torn
371+
down). The error handler must not crash trying to set_exception on
372+
the already-cancelled futures.
373+
"""
374+
375+
async def failing_loader(keys: list[int]) -> list[int]:
376+
raise RuntimeError("executor is broken")
377+
378+
loader = DataLoader(load_fn=failing_loader, cache=False)
379+
380+
future_a = cast("Future[Any]", loader.load(1))
381+
future_b = cast("Future[Any]", loader.load(2))
382+
future_c = cast("Future[Any]", loader.load(3))
383+
384+
# Simulate a client disconnect cancelling one of the futures before
385+
# the batch dispatches
386+
future_a.cancel()
387+
388+
# Let the event loop dispatch the batch — this must not raise
389+
# InvalidStateError from trying to set_exception on the cancelled future.
390+
# Two yields: one for call_soon to fire create_task, one for the task to run.
391+
await asyncio.sleep(0)
392+
await asyncio.sleep(0)
393+
394+
# The cancelled future should remain cleanly cancelled — not broken by
395+
# an InvalidStateError inside dispatch_batch
396+
assert future_a.cancelled()
397+
with pytest.raises(asyncio.CancelledError):
398+
future_a.result()
399+
400+
# The non-cancelled futures should have received the loader's exception
401+
# (without the fix, dispatch_batch crashes on future_a before reaching these)
402+
with pytest.raises(RuntimeError, match="executor is broken"):
403+
future_b.result()
404+
with pytest.raises(RuntimeError, match="executor is broken"):
405+
future_c.result()
406+
407+
362408
@pytest.mark.asyncio
363409
async def test_cache_override():
364410
class TestCache(AbstractCache[int, int]):

0 commit comments

Comments
 (0)