fix(bun): Keep diagnostics channel subscriptions alive - #24632
Conversation
33f7607 to
77ca363
Compare
size-limit report 📦
|
77ca363 to
72e8bec
Compare
72e8bec to
373d5ab
Compare
373d5ab to
49333f3
Compare
7452887 to
27c4421
Compare
27c4421 to
ff5d855
Compare
b0e5f06 to
21f68ca
Compare
763a9ee to
0b4a065
Compare
b3a3cf2 to
ef10728
Compare
7fae1f6 to
2882826
Compare
b2d38bb to
f0392e0
Compare
6d127e5 to
8d1438b
Compare
0ed13e4 to
d5ae9f0
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d5ae9f0. Configure here.
d5ae9f0 to
dc8b84d
Compare
dc8b84d to
78e269f
Compare
Bun garbage-collects a diagnostics channel that no code references, together with its subscribers, so a subscription made at init() could stop receiving messages after the next GC. Node and Deno keep a subscribed channel alive. On Bun, the channel wrappers in @sentry/server-utils now keep a reference to every channel they return, and all channel subscriptions in server-utils and @sentry/node use them, including Prisma and pino. Other runtimes get the original node:diagnostics_channel functions. A no-restricted-imports lint rule in server-utils, node and bun rejects value imports of node:diagnostics_channel outside the wrappers, so new integrations use them too. graphql-tracing-channel, which the GC broke, now runs on Bun. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
78e269f to
984e443
Compare
| export const subscribe: typeof nodeDiagnosticsChannel.subscribe = keepChannelsReferenced | ||
| ? (name, onMessage) => { | ||
| channel(name).subscribe(onMessage); | ||
| } | ||
| : nodeDiagnosticsChannel.subscribe; |
There was a problem hiding this comment.
Bug: The Bun-specific implementation of subscribe returns undefined, but its type annotation incorrectly claims it matches nodeDiagnosticsChannel.subscribe, which should return a subscription object.
Severity: LOW
Suggested Fix
Modify the Bun-specific implementation of the subscribe function to return the result of channel(name).subscribe(onMessage). This will align the function's behavior with its type annotation and the standard Node.js implementation, ensuring it correctly returns a subscription object.
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/utils/diagnosticsChannel.ts#L28-L32
Potential issue: The `subscribe` function in `diagnosticsChannel.ts` has a conditional
implementation for Bun environments. This implementation does not return a value,
resulting in an `undefined` return. However, the function is typed as `typeof
nodeDiagnosticsChannel.subscribe`, which implies it should return a subscription object
with an `unsubscribe()` method. While no current call sites in the codebase utilize this
return value, this type discrepancy could lead to runtime errors in the future if the
function is used in a context that expects a subscription object to be returned.
…ntegration (#24797) ## What The MCP server integration now uses the shared diagnostics channel wrappers, so its subscription stays alive on Bun. ## Why #24529 and #24632 merged at the same time, and the direct `node:diagnostics_channel` import now fails lint on `develop` and on the 11.1.0 release PR. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Bun garbage-collects a diagnostics channel that no code references, and its subscribers with it. Node and Deno keep subscribed channels alive. The channel integrations kept no reference, so on Bun they could silently stop producing spans after the first GC. They now create their channels through wrappers in
@sentry/server-utilsthat keep one.@sentry/bunuses these integrations, and alsohttpIntegrationfrom@sentry/node, whose channel subscriptions now go through the same wrappers.diagnosticsChannelGc.test.tsreproduces the GC on Bun.Running the shared Node integration suites on Bun found this. Also an oxlint rule has been added to prevent any issues in the future regarding this
Part of #23889.
🤖 Generated with Claude Code