ref(server-utils): Streamline graphql integration - #23329
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
Reviewed by Cursor Bugbot for commit 89a3910. Configure here.
| genericPoolIntegration, | ||
| googleGenAIIntegration, | ||
| graphqlDiagnosticsIntegration, | ||
| graphqlIntegration, |
There was a problem hiding this comment.
Public GraphQL API renamed without deprecation
Medium Severity
graphqlDiagnosticsIntegration is removed from public Deno and @sentry/server-utils/orchestrion exports and replaced with graphqlIntegration without a deprecated alias. The main @sentry/server-utils entry also drops its previous graphqlIntegration export. This is a public API breaking change without a deprecation notice, which the PR review guidelines ask to flag. Nearby Deno renames keep deprecated aliases for the same reason.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 89a3910. Configure here.
There was a problem hiding this comment.
this was not exposed before at all, so all good.
size-limit report 📦
|
6c4ec00 to
8960fd0
Compare
8960fd0 to
8b49cb9
Compare
8b49cb9 to
760292b
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
|
||
| const _graphqlIntegration = ((options: GraphqlDiagnosticChannelsOptions = {}) => { | ||
| const _graphqlIntegration = ((options: GraphQLOptions = {}) => { | ||
| const config = getOptionsWithDefaults(options); | ||
| const getConfig = (): GraphqlResolvedConfig => config; |
There was a problem hiding this comment.
Bug: setupOnce calls waitForTracingChannelBinding which may fail if the async context isn't ready during SDK init. The native GraphQL instrumentation will then silently fail without any recovery mechanism.
Severity: HIGH
Suggested Fix
Implement a recovery mechanism for the native GraphQL instrumentation path, similar to the one used by the orchestrion instrumentation. Instead of a single, immediate retry, the code should listen for an event indicating the async context is ready or use a more robust retry strategy to ensure the subscription to diagnostic channels eventually succeeds.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/server-utils/src/integrations/graphql/index.ts#L37-L40
Potential issue: The `graphqlIntegration.setupOnce()` method calls
`setupNativeGraphQLInstrumentation()`, which uses `waitForTracingChannelBinding()` to
subscribe to native GraphQL diagnostic channels. This subscription can fail if the async
context strategy is not yet configured when `setupOnce()` is executed during SDK
initialization. The `waitForTracingChannelBinding()` function only retries once before
failing silently. Unlike the orchestrion instrumentation path, this new native path
lacks a recovery mechanism to re-attempt the subscription later. Consequently, for
GraphQL v17+, instrumentation may be silently disabled without any indication to the
user, leading to a loss of tracing data.
Did we get this right? 👍 / 👎 to inform future reviews.
1 participant
Streamline graphql integration files into a single folder, rename it to
graphqlIntegrationfor consistency, combine into a singular integration for consistency.