ref(strawberry): Simplify span creation#5647
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Pydantic Ai
Other
Bug Fixes 🐛
Documentation 📚
Internal Changes 🔧Openai Agents
Other
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 13.39s All tests are passing successfully. ❌ Patch coverage is 0.00%. Project has 14066 uncovered lines. Files with missing lines (1)
Generated by Codecov Action |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| transaction.op = op | ||
|
|
||
| self.graphql_span.finish() | ||
| self.graphql_span.__exit__(None, None, None) |
There was a problem hiding this comment.
Missing try/finally around yield leaks scope state
Medium Severity
__enter__() modifies scope.span and stores the old span in _context_manager_state, but the yield between __enter__() and __exit__() is not wrapped in a try/finally. If the Strawberry framework throws an exception into the generator or calls close() on it, the code after yield (including __exit__) is never reached. This leaves scope.span permanently pointing to the now-stale graphql_span, and the previous span is never restored. The old code using finish() didn't have this issue because it never called __enter__() and therefore never modified scope state.


The integration is doing manual scope management when it doesn't have to. Also, it's not setting the span on the scope.
(Another thing that popped out at me while looking at mypy issues in span first.)