feat(core): consolidate fetch integrations - #24346
Conversation
Consolidate the nearly (but not exactly!) identical fetch integrations used by deno, bun, cloudflare, and vercel-edge. The functionality is placed in `@sentry/core` rather than `@sentry/server-utils` in order to avoid leaking node internals where they don't belong. closes JS-3667 closes #24344
isaacs
changed the title
|
Added a second commit to bound the number of spans that can potentially be hanging. It's a preexisting hazard that all 4 implementations had, but noticed in the refactor. If others feel like it'd be better to put off, that can be a separate issue for discussion (or ditched entirely if it's not a relevant concern), but it seemed convenient to just fix it while in the area. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| // oxlint-disable-next-line typescript/no-deprecated | ||
| if (!_options.fetch) { | ||
| debug.warn( | ||
| 'breadcrumbsIntegration({ fetch: false }) no longer has any effect. Fetch breadcrumbs are recorded by fetchIntegration; disable them with fetchIntegration({ breadcrumbs: false }).', | ||
| ); |
There was a problem hiding this comment.
Bug: The deprecated fetch: false option in breadcrumbsIntegration no longer suppresses fetch breadcrumbs, causing them to be generated by the new default fetchIntegration.
Severity: MEDIUM
Suggested Fix
To avoid this breaking change for existing users, consider respecting the fetch: false option for a deprecation period. Alternatively, elevate the notification from a debug log to a console.warn to ensure users are aware that they need to update their configuration to fetchIntegration({ breadcrumbs: false }) to restore the previous behavior.
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/deno/src/integrations/breadcrumbs.ts#L46-L50
Potential issue: A breaking change was introduced for users of the Deno SDK. Previously,
fetch-related breadcrumbs could be disabled by initializing the SDK with
`breadcrumbsIntegration({ fetch: false })`. After this change, this option no longer
prevents fetch breadcrumbs from being generated. Instead, the new `fetchIntegration`,
which is enabled by default, will create them. The old option now only logs a
debug-level warning, which may not be visible to users, causing fetch breadcrumbs to be
unexpectedly recorded for those who had explicitly opted out.
Did we get this right? 👍 / 👎 to inform future reviews.
![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 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b931e56. Configure here.
| export type { DenoServeIntegrationOptions } from './integrations/deno-serve'; | ||
| export { fetchIntegration } from './integrations/fetch'; | ||
| export type { FetchOptions } from './integrations/fetch'; | ||
| export type { FetchIntegrationOptions } from '@sentry/core'; |
There was a problem hiding this comment.
Public FetchOptions type removed
Medium Severity
@sentry/deno no longer exports the public FetchOptions type and now exports FetchIntegrationOptions instead, with no deprecated alias. Existing type-only imports will fail typecheck. Flagged because the review rules call out public type removals without a deprecation path.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit b931e56. Configure here.
| if (!_options.fetch) { | ||
| debug.warn( | ||
| 'breadcrumbsIntegration({ fetch: false }) no longer has any effect. Fetch breadcrumbs are recorded by fetchIntegration; disable them with fetchIntegration({ breadcrumbs: false }).', | ||
| ); |
There was a problem hiding this comment.
Deprecation warning hidden from users
Low Severity
The breadcrumbsIntegration({ fetch: false }) deprecation notice uses debug.warn, so it only appears when Sentry debug logging is on. Users who disabled fetch breadcrumbs this way will keep getting them with no visible warning. Flagged because the review rules reserve consoleSandbox for user-facing init warnings.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit b931e56. Configure here.
size-limit report 📦
|
1 participant
Consolidate the nearly (but not exactly!) identical fetch integrations used by deno, bun, cloudflare, and vercel-edge.
The deno
breadcrumbsintegration is folded in, and the separate integration is deprecated.The functionality is placed in
@sentry/corerather than@sentry/server-utilsin order to avoid leaking node internals where they don't belong.Node and Browser fetch integrations are not touched, because they use completely different mechanisms.
closes JS-3667
closes #24344