fix: prevent async iterable detection for sync execution in gql-core 3.3#4267
fix: prevent async iterable detection for sync execution in gql-core 3.3#4267
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a guarded is_async_iterable override for sync execution paths so graphql-core 3.3 does not mis-detect sync values (like Django QuerySets) as async iterables, wiring it through existing custom context kwargs only when supported. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider extracting the
lambda _x: Falseinto a named module-level function (e.g._never_async_iterable) to avoid recreating the lambda on each call and to make the intent clearer for future readers. - The new
syncflag on_get_custom_context_kwargsis a bit indirect in what it controls; renaming it to something likeforce_sync_iterables(or similar) would better communicate that it specifically toggles theis_async_iterablebehavior rather than generic sync/async execution.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the `lambda _x: False` into a named module-level function (e.g. `_never_async_iterable`) to avoid recreating the lambda on each call and to make the intent clearer for future readers.
- The new `sync` flag on `_get_custom_context_kwargs` is a bit indirect in what it controls; renaming it to something like `force_sync_iterables` (or similar) would better communicate that it specifically toggles the `is_async_iterable` behavior rather than generic sync/async execution.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Thanks for adding the Here's a preview of the changelog: Fix sync execution crash with graphql-core 3.3 where Now passes Note: graphql-core >= 3.3.0a12 is now the minimum required version for the 3.3.x series. Here's the tweet text: |
Greptile SummaryFixes a critical bug where The fix adds runtime detection using Key changes:
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: e6ca453 |
e6ca453 to
3f8df4a
Compare
Apollo Federation Subgraph Compatibility Results
Learn more: |
018a569 to
c0cabe7
Compare
|
@patrick91 should be ready now edit: not really, some tests are failing. Will check why =P edit2: there was another "breaking" change, fixed now |
There was a problem hiding this comment.
Pull request overview
This PR is a multi-version release (v0.308.0 through v0.308.3) bundling several fixes and one new feature. The primary changes address graphql-core 3.3 compatibility (preventing sync execution from incorrectly returning coroutines), Python 3.14 pydantic v1 warning suppression, ApolloTracingExtension crash fixes for invalid queries, lazy type resolution improvements for generic wrappers with from __future__ import annotations, and a new Annotated syntax for field definitions.
Changes:
- Adds
is_async_iterable=lambda _x: Falsetoexecute_sync()for graphql-core 3.3, preventing objects with__aiter__(like Django QuerySets) from being treated as async iterables, plus a minimum version guard requiring gql-core >= 3.3.0a12. - Introduces support for
Annotated[type, strawberry.field(...)]field definition syntax with proper error handling for duplicatestrawberry.fields, and extends the AST namespace resolver to handle lazy types inside generic subscripts. - Fixes ApolloTracingExtension crashes on invalid queries by initializing timing attributes in
__init__and wrapping lifecycle hooks withtry/finally, plus suppresses pydantic v1 warnings on Python 3.14+.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
strawberry/utils/__init__.py |
Adds minimum version guard for graphql-core 3.3 (>= 3.3.0a12) |
strawberry/schema/schema.py |
Passes is_async_iterable=lambda _x: False for sync execution in gql-core 3.3 |
strawberry/extensions/tracing/apollo.py |
Initializes timing attributes in __init__, wraps hooks with try/finally |
strawberry/utils/typing.py |
Adds already_resolved guard for lazy types + generic subscript fallback |
strawberry/types/type_resolver.py |
Adds _get_field_from_annotated() for Annotated field extraction |
strawberry/exceptions/multiple_strawberry_fields.py |
New MultipleStrawberryFieldsError exception class |
strawberry/exceptions/__init__.py |
Exports the new exception |
strawberry/experimental/pydantic/_compat.py |
Skips pydantic v1 imports on Python 3.14+ |
tests/types/test_object_types.py |
Comprehensive tests for Annotated field definitions |
tests/types/test_lazy_types_future_annotations.py |
Tests for TYPE_CHECKING guard with lazy types |
tests/schema/extensions/test_apollo.py |
Regression tests for invalid queries with tracing |
tests/experimental/pydantic/schema/test_1_and_2.py |
Python 3.14 pydantic warning test |
docs/guides/custom-extensions.md |
Fixes info type in resolve example to GraphQLResolveInfo |
docs/errors/multiple-strawberry-fields.md |
Error documentation for new exception |
CHANGELOG.md |
Changelog entries for v0.308.0–v0.308.3 |
pyproject.toml |
Version bump to 0.308.3 |
poetry.lock |
FastAPI dependency update |
.pre-commit-config.yaml |
Ruff pre-commit hook update to v0.15.4 |
.github/workflows/test.yml |
GitHub Actions version bumps |
.github/workflows/e2e-tests.yml |
upload-artifact action version bump |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
67122b6 to
390f827
Compare
graphql-core 3.3.0a12 requires tuples instead of lists for AST node collection fields (e.g. ObjectValueNode.fields). Convert the list comprehension in ast_from_leaf_type to a tuple.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4b120c5 to
be7936e
Compare
- Add upstream tracking comment to Strawberry printer patch (strawberry-graphql/strawberry#4267) - Clarify patch docstring - Add sed post-processing to normalize { } → {} in exported schema
…r patch - Upgrade strawberry-graphql from 0.287.3 to 0.312.2 across all pins - Remove _patch_strawberry_schema_printer() monkey-patch (fixed upstream in strawberry-graphql/strawberry#4267) - Fix deprecated union syntax in Secret.py - Remove test_schema.py (tested the now-removed patch)
- Add upstream tracking comment to Strawberry printer patch (strawberry-graphql/strawberry#4267) - Clarify patch docstring - Add sed post-processing to normalize { } → {} in exported schema
* chore: unblock graphql defer * chore: add tracking comment and normalize schema.graphql spacing - Add upstream tracking comment to Strawberry printer patch (strawberry-graphql/strawberry#4267) - Clarify patch docstring - Add sed post-processing to normalize { } → {} in exported schema * chore: revert sed post-processing, rebuild schema * chore: remove test_schema.py

Summary
graphql-core 3.3 added an
is_async_iterableparameter toexecute(). When not provided, it defaults to a check that returnsTruefor any object with__aiter__— including Django QuerySets.This causes
execute()to return a coroutine instead of anExecutionResultduring sync execution, which then crashes inexecute_sync()with:Fix: When
execute_sync()callsexecute(), passis_async_iterable=lambda _x: Falseto prevent graphql-core 3.3 from treating sync iterables as async. This is gated behind theIS_GQL_33version flag so it only applies to 3.3+, and a runtime version check ensures users on older 3.3 alphas (beforeis_async_iterableexisted) get a clear error directing them to upgrade to>= 3.3.0a12.Test plan
execute_syncworks with resolvers returning objects that have__aiter__