Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 8.15s All tests are passing successfully. ❌ Patch coverage is 14.29%. Project has 14372 uncovered lines. Files with missing lines (1)
Generated by Codecov Action |
| try: | ||
| yield | ||
| finally: | ||
| _graphql_span.__exit__(None, None, None) | ||
| if isinstance(_graphql_span, StreamedSpan): | ||
| _graphql_span.end() | ||
| else: | ||
| _graphql_span.__exit__(None, None, None) |
There was a problem hiding this comment.
StreamedSpan exceptions not marked as errors due to bypassing exit
When has_span_streaming_enabled is True, the span is created but never entered as a context manager (no __enter__ call), and ended directly via .end() instead of __exit__. This skips the exception handling logic in StreamedSpan.__exit__ that sets status = SpanStatus.ERROR when an exception occurs. If a GraphQL operation throws an exception, the streaming span will be marked as 'ok' instead of 'error', losing important error information.
Verification
Read traces.py:310-323 confirming exit handles exception status via should_be_treated_as_error, while end() (lines 325-332) just calls _end() without exception handling. Read asgi.py:254,290 showing that ASGI uses streaming spans correctly via with span_ctx as span: context manager pattern.
Identified by Warden code-review · G3U-L27
Description
Issues
Reminders
tox -e linters.feat:,fix:,ref:,meta:)