-
-
Notifications
You must be signed in to change notification settings - Fork 799
[Fusion] Remove Debug.Assert around MoveNext() calls #8805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a critical bug where Debug.Assert calls were being used to advance enumerators in release builds, causing enumerators to never advance and resulting in exceptions for operations querying affected fields.
- Replaces Debug.Assert calls with direct MoveNext() calls in introspection code
- Fixes TypeNameField to correctly reference the selection set's type instead of the root operation type
- Adds a new test case to verify __typename functionality on introspection types
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
TypeNameField.cs | Fixed to use correct type reference for __typename field resolution |
__Type.cs | Replaced Debug.Assert with direct MoveNext() calls for enumerator advancement |
__Directive.cs | Replaced Debug.Assert with direct MoveNext() call for enumerator advancement |
IntrospectionTests.cs | Added test case and refactored existing test structure |
IntrospectionTests.Typename_On_Introspection_Types.yaml | Test snapshot for new introspection functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/Nodes/TypeNameField.cs
Show resolved
Hide resolved
src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/Introspection/__Type.cs
Outdated
Show resolved
Hide resolved
src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/Introspection/__Type.cs
Outdated
Show resolved
Hide resolved
src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/Introspection/__Type.cs
Outdated
Show resolved
Hide resolved
src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/Introspection/__Directive.cs
Outdated
Show resolved
Hide resolved
24dcec1
to
871c19c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8805 +/- ##
======================================
Coverage ? 0
======================================
Files ? 0
Lines ? 0
Branches ? 0
======================================
Hits ? 0
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Debug.Assert calls are removed in Release builds, so the enumerator was never advanced in those cases leading to an exception for operations querying affected fields.
Also fixes that
__typename
on any introspection type like__Schema
always returnedQuery
.