Skip to content

Commit 908fa35

Browse files
isaacsclaude
andauthored
fix(cloudflare): Do not add back channel integrations the user removed (#25585)
The `@sentry/cloudflare/vite` plugin registers an integration factory for each instrumented package when the package first runs. A package that runs after `init()`, for example a driver loaded with a dynamic `import()`, emits `orchestrion.module-injected`, and the SDK then added its integration without checking the user's options. So an `integrations` function that filtered the integration out, `defaultIntegrations: false`, or a custom `defaultIntegrations` list did not keep it out. For the AI integrations, that can turn prompt and response capture back on, and whether it happened depended on module load order. The handler now applies the rules that `init()` applies to the defaults: - If `defaultIntegrations` is set, the SDK defaults are off, so the handler adds nothing. - If `integrations` is a function, the handler passes the late integration through it and installs the result with the same name, if there is one. This calls the function again, once for each late module. If the function throws, the handler skips the integration, so the module load that emitted the event still works. - If `integrations` is an array, nothing changes. An array cannot remove a default, and `addIntegration` already skips a name the user configured. The Nuxt and SvelteKit Cloudflare SDKs always pass a `defaultIntegrations` list, which they build from the Cloudflare defaults. Under the first rule they lost every late integration, so the `nuxt-4-cloudflare` and `sveltekit-cloudflare-pages` E2E tests got no `mysql` spans. These SDKs now set the internal option `_extendsDefaultIntegrations` when they build the list, so the handler still adds late integrations for them. A list the user passes still turns them off. We kept this option over moving the handler into a default integration, because a user who builds a list from `getDefaultIntegrations()` and filters it would then still get the filtered integration back. Fixes #25576 Fixes JS-3974 --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent ad71ef6 commit 908fa35

7 files changed

Lines changed: 230 additions & 7 deletions

File tree

‎packages/cloudflare/src/baseSdk.ts‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Integration } from '@sentry/core';
2-
import { _INTERNAL_isClientClosed, debug, getCurrentScope, setCurrentClient } from '@sentry/core';
2+
import { _INTERNAL_isClientClosed, debug, getCurrentScope, safeCallback, setCurrentClient } from '@sentry/core';
33
import {
44
consoleIntegration,
55
conversationIdIntegration,
@@ -107,6 +107,9 @@ export function initWithDefaultIntegrations(
107107
return cached;
108108
}
109109

110+
// `false` or a custom list turns off the SDK defaults, late-loaded ones
111+
// included, unless an SDK built the list from the defaults.
112+
const usesSdkDefaults = options.defaultIntegrations === undefined || !!options._extendsDefaultIntegrations;
110113
if (options.defaultIntegrations === undefined) {
111114
options.defaultIntegrations = getDefaultIntegrationsImpl(options);
112115
}
@@ -153,12 +156,36 @@ export function initWithDefaultIntegrations(
153156
// lazily required on first use) stores its subscriber factory on the global
154157
// marker too late for the default-integrations snapshot above. Its injected
155158
// snippet emits this event right after storing the factory, so install the
156-
// integration on the live client here. `addIntegration` dedupes by
157-
// integration name, so already-installed integrations are no-ops.
159+
// integration on the live client here, with the same rules as the defaults.
160+
// `addIntegration` dedupes by integration name, so an integration the user
161+
// configured stays in place.
162+
const userIntegrations = options.integrations;
158163
client.on('orchestrion.module-injected', moduleName => {
159164
const factory = GLOBAL_OBJ.__SENTRY_ORCHESTRION__?.integrations?.get(moduleName);
160-
if (factory) {
161-
client.addIntegration(factory());
165+
if (!usesSdkDefaults || !factory) {
166+
return;
167+
}
168+
169+
const integration = factory();
170+
if (typeof userIntegrations !== 'function') {
171+
client.addIntegration(integration);
172+
return;
173+
}
174+
175+
// Pass the late default through the user's `integrations` function, so a
176+
// filter that removed it at init removes it here too. This calls the
177+
// function again, once for each late module. If the function throws, skip
178+
// the integration, so the module load that emitted the event still works.
179+
const kept = safeCallback(
180+
DEBUG_BUILD ? 'Error in the `integrations` function, skipping a late-loaded integration:' : '',
181+
() =>
182+
getIntegrationsToSetup({ defaultIntegrations: [integration], integrations: userIntegrations }).find(
183+
({ name }) => name === integration.name,
184+
),
185+
() => undefined,
186+
);
187+
if (kept) {
188+
client.addIntegration(kept);
162189
}
163190
});
164191

‎packages/cloudflare/src/client.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,16 @@ interface BaseCloudflareOptions {
351351
*/
352352
enableDedupe?: boolean;
353353

354+
/**
355+
* @ignore Used internally by SDKs that build `defaultIntegrations` from the Cloudflare defaults.
356+
* @hidden Used internally by SDKs that build `defaultIntegrations` from the Cloudflare defaults.
357+
*
358+
* Marks the `defaultIntegrations` list as an extension of the SDK defaults, so the SDK also adds
359+
* the integrations of instrumented modules that load after `init()`.
360+
* @default false
361+
*/
362+
_extendsDefaultIntegrations?: boolean;
363+
354364
/**
355365
* The Cloudflare SDK is not OpenTelemetry native. By default (`false`) it does not set up a tracer
356366
* provider; spans are emitted via the SDK's own instrumentation and scopes are isolated with

‎packages/cloudflare/test/sdk.test.ts‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,108 @@ describe('getDefaultIntegrations', () => {
458458
expect(client?.getIntegrationByName('Mysql')).toBe(userInstance);
459459
});
460460

461+
describe('when a channel integration module loads after init', () => {
462+
const DSN = 'https://public@dsn.ingest.sentry.io/1337';
463+
464+
async function loadMysqlLate(client: CloudflareClient | undefined): Promise<void> {
465+
const { mysqlIntegration } = await import('@sentry/server-utils');
466+
globalThis.__SENTRY_ORCHESTRION__ = {
467+
bundler: new Set(['mysql']),
468+
integrations: new Map([['mysql', mysqlIntegration]]),
469+
};
470+
client?.emit('orchestrion.module-injected', 'mysql');
471+
}
472+
473+
test('does not add it when the integrations function filtered it out', async () => {
474+
resetSdk();
475+
const client = init({
476+
dsn: DSN,
477+
integrations: defaults => defaults.filter(integration => integration.name !== 'Mysql'),
478+
});
479+
480+
await loadMysqlLate(client);
481+
482+
expect(client?.getIntegrationByName('Mysql')).toBeUndefined();
483+
});
484+
485+
test('adds it when the integrations function keeps it', async () => {
486+
resetSdk();
487+
const client = init({ dsn: DSN, integrations: defaults => defaults });
488+
489+
await loadMysqlLate(client);
490+
491+
expect(client?.getIntegrationByName('Mysql')).toBeDefined();
492+
});
493+
494+
test('keeps the instance the integrations function configured', async () => {
495+
resetSdk();
496+
const client = init({
497+
dsn: DSN,
498+
integrations: defaults => [...defaults, exportedMysqlIntegration()],
499+
});
500+
const configured = client?.getIntegrationByName('Mysql');
501+
expect(configured).toBeDefined();
502+
503+
await loadMysqlLate(client);
504+
505+
expect(client?.getIntegrationByName('Mysql')).toBe(configured);
506+
});
507+
508+
test('does not add it and does not throw when the integrations function throws', async () => {
509+
resetSdk();
510+
let calls = 0;
511+
const client = init({
512+
dsn: DSN,
513+
integrations: defaults => {
514+
calls++;
515+
if (calls > 1) {
516+
throw new Error('integrations failed');
517+
}
518+
return defaults;
519+
},
520+
});
521+
522+
await expect(loadMysqlLate(client)).resolves.toBeUndefined();
523+
524+
expect(client?.getIntegrationByName('Mysql')).toBeUndefined();
525+
});
526+
527+
test('adds it when a custom defaultIntegrations list extends the SDK defaults', async () => {
528+
resetSdk();
529+
const client = init({ dsn: DSN, defaultIntegrations: [], _extendsDefaultIntegrations: true });
530+
531+
await loadMysqlLate(client);
532+
533+
expect(client?.getIntegrationByName('Mysql')).toBeDefined();
534+
});
535+
536+
test('does not add it when the integrations function filtered it out of a list that extends the SDK defaults', async () => {
537+
resetSdk();
538+
const client = init({
539+
dsn: DSN,
540+
defaultIntegrations: [],
541+
_extendsDefaultIntegrations: true,
542+
integrations: defaults => defaults.filter(integration => integration.name !== 'Mysql'),
543+
});
544+
545+
await loadMysqlLate(client);
546+
547+
expect(client?.getIntegrationByName('Mysql')).toBeUndefined();
548+
});
549+
550+
test.each([
551+
['false', false as const],
552+
['a custom list', [] as Integration[]],
553+
])('does not add it when defaultIntegrations is %s', async (_label, defaultIntegrations) => {
554+
resetSdk();
555+
const client = init({ dsn: DSN, defaultIntegrations });
556+
557+
await loadMysqlLate(client);
558+
559+
expect(client?.getIntegrationByName('Mysql')).toBeUndefined();
560+
});
561+
});
562+
461563
test('keeps a channel integration the user configured when its module loads after init', async () => {
462564
resetSdk();
463565
const { mysqlIntegration } = await import('@sentry/server-utils');

‎packages/nuxt/src/runtime/plugins/sentry-cloudflare.server.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,11 @@ export const sentryCloudflareNitroPlugin =
7171
const requestHandlerOptions = {
7272
// `wrapRequestHandler` only sets up the integrations that work without `nodejs_compat`.
7373
// Nitro's Cloudflare presets require the flag anyway, so opt into the full set.
74-
options: { defaultIntegrations: getDefaultIntegrations(cloudflareOptions), ...cloudflareOptions },
74+
options: {
75+
defaultIntegrations: getDefaultIntegrations(cloudflareOptions),
76+
_extendsDefaultIntegrations: cloudflareOptions.defaultIntegrations === undefined,
77+
...cloudflareOptions,
78+
},
7579
request,
7680
context: getCloudflareProperties(event).context,
7781
};
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import type { CloudflareOptions } from '@sentry/cloudflare';
2+
import { wrapRequestHandler } from '@sentry/cloudflare/request';
3+
import type { NitroApp } from 'nitropack';
4+
import { beforeEach, describe, expect, it, vi } from 'vitest';
5+
import { sentryCloudflareNitroPlugin } from '../../../src/runtime/plugins/sentry-cloudflare.server';
6+
7+
vi.mock('@sentry/cloudflare/request', () => ({
8+
wrapRequestHandler: vi.fn((_options: unknown, callback: () => unknown) => callback()),
9+
}));
10+
11+
const DSN = 'https://public@dsn.ingest.sentry.io/1337';
12+
13+
async function handleRequest(options: CloudflareOptions): Promise<void> {
14+
const nitroApp = { localFetch: vi.fn(), hooks: { hook: vi.fn() } } as unknown as NitroApp;
15+
sentryCloudflareNitroPlugin(options)(nitroApp);
16+
17+
const event = {
18+
protocol: 'https:',
19+
host: 'example.com',
20+
method: 'GET',
21+
headers: {},
22+
context: { cf: {}, cloudflare: { context: {} } },
23+
};
24+
await nitroApp.localFetch('/api/db-mysql', event as unknown as RequestInit);
25+
}
26+
27+
describe('sentryCloudflareNitroPlugin', () => {
28+
beforeEach(() => {
29+
vi.mocked(wrapRequestHandler).mockClear();
30+
});
31+
32+
it('passes a defaultIntegrations list that extends the SDK defaults', async () => {
33+
await handleRequest({ dsn: DSN });
34+
35+
expect(wrapRequestHandler).toHaveBeenCalledWith(
36+
expect.objectContaining({
37+
options: expect.objectContaining({
38+
defaultIntegrations: expect.any(Array),
39+
_extendsDefaultIntegrations: true,
40+
}),
41+
}),
42+
expect.any(Function),
43+
);
44+
});
45+
46+
it('does not mark a defaultIntegrations list the user passed as extending the SDK defaults', async () => {
47+
await handleRequest({ dsn: DSN, defaultIntegrations: [] });
48+
49+
expect(wrapRequestHandler).toHaveBeenCalledWith(
50+
expect.objectContaining({
51+
options: expect.objectContaining({ defaultIntegrations: [], _extendsDefaultIntegrations: false }),
52+
}),
53+
expect.any(Function),
54+
);
55+
});
56+
});

‎packages/sveltekit/src/worker/cloudflare.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export function initCloudflareSentryHandle(options: CloudflareOptions): Handle {
2323
rewriteFramesIntegration(),
2424
svelteKitSpansIntegration(),
2525
],
26+
_extendsDefaultIntegrations: options.defaultIntegrations === undefined,
2627
// SvelteKit emits its own OpenTelemetry spans (Kit tracing), so, like the Node SvelteKit SDK, it
2728
// defaults to registering the tracer provider instead of inheriting Cloudflare's no-provider default.
2829
// Only the `init`-backed wrapper from the main entry point honors this; `@sentry/cloudflare/request`

‎packages/sveltekit/test/worker/cloudflare.test.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ describe('initCloudflareSentryHandle', () => {
6363
{
6464
// SvelteKit emits its own OpenTelemetry spans, so it opts into the tracer provider rather than
6565
// inheriting Cloudflare's no-provider default.
66-
options: expect.objectContaining({ dsn: options.dsn, enableOpenTelemetrySetup: true }),
66+
options: expect.objectContaining({
67+
dsn: options.dsn,
68+
enableOpenTelemetrySetup: true,
69+
_extendsDefaultIntegrations: true,
70+
}),
6771
request,
6872
context,
6973
captureErrors: false,
@@ -74,6 +78,25 @@ describe('initCloudflareSentryHandle', () => {
7478
expect(resolve).toHaveBeenCalledTimes(1);
7579
});
7680

81+
it('does not mark a defaultIntegrations list the user passed as extending the SDK defaults', async () => {
82+
const { options, event, resolve } = getHandlerInput();
83+
84+
// @ts-expect-error - resolving an empty object is enough for this test
85+
vi.mocked(wrapRequestHandler).mockImplementationOnce((_, cb) => cb());
86+
87+
const handle = initCloudflareSentryHandle({ ...options, defaultIntegrations: [] });
88+
89+
// @ts-expect-error - only passing a partial event object
90+
await handle({ event, resolve });
91+
92+
expect(wrapRequestHandler).toHaveBeenCalledWith(
93+
expect.objectContaining({
94+
options: expect.objectContaining({ defaultIntegrations: [], _extendsDefaultIntegrations: false }),
95+
}),
96+
expect.any(Function),
97+
);
98+
});
99+
77100
it('adds flag to skip request isolation in subsequent sentry handler', async () => {
78101
const { options, event, resolve } = getHandlerInput();
79102
const locals = {};

0 commit comments

Comments
 (0)