Skip to content

Commit

Permalink
fix(browser): Initialize default integration if `defaultIntegrations:…
Browse files Browse the repository at this point in the history
… undefined` (#13261)

If users or our
higher-level SDKs pass `defaultIntegrations: undefined` to the Browser
SDK's init options, it deactivates all default integrations. As per our
docs, this should only happen if you explicitly pass
`defaultIntegrations: false`.

This PR fixes this by removing the `defaultIntegrations` key from the
user options object before merging the options together.

---------

Co-authored-by: Francesco Novy <[email protected]>
  • Loading branch information
Lms24 and mydea authored Aug 7, 2024
1 parent 9ed2112 commit 061042a
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 1 deletion.
2 changes: 1 addition & 1 deletion .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ module.exports = [
path: 'packages/browser/build/npm/esm/index.js',
import: createImport('init', 'browserTracingIntegration', 'replayIntegration'),
gzip: true,
limit: '72 KB',
limit: '73 KB',
},
{
name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags',
Expand Down
8 changes: 8 additions & 0 deletions packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ function applyDefaultOptions(optionsArg: BrowserOptions = {}): BrowserOptions {
sendClientReports: true,
};

// TODO: Instead of dropping just `defaultIntegrations`, we should simply
// call `dropUndefinedKeys` on the entire `optionsArg`.
// However, for this to work we need to adjust the `hasTracingEnabled()` logic
// first as it differentiates between `undefined` and the key not being in the object.
if (optionsArg.defaultIntegrations == null) {
delete optionsArg.defaultIntegrations;
}

return { ...defaultOptions, ...optionsArg };
}

Expand Down
14 changes: 14 additions & 0 deletions packages/browser/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import type { Mock } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import * as SentryCore from '@sentry/core';
import { Scope, createTransport } from '@sentry/core';
import type { Client, Integration } from '@sentry/types';
import { resolvedSyncPromise } from '@sentry/utils';
Expand Down Expand Up @@ -79,6 +80,18 @@ describe('init', () => {
expect(DEFAULT_INTEGRATIONS[1]!.setupOnce as Mock).toHaveBeenCalledTimes(1);
});

it('installs default integrations if `defaultIntegrations: undefined`', () => {
// @ts-expect-error this is fine for testing
const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind').mockImplementationOnce(() => {});
const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, defaultIntegrations: undefined });
init(options);

expect(initAndBindSpy).toHaveBeenCalledTimes(1);

const optionsPassed = initAndBindSpy.mock.calls[0]?.[1];
expect(optionsPassed?.integrations?.length).toBeGreaterThan(0);
});

test("doesn't install default integrations if told not to", () => {
const DEFAULT_INTEGRATIONS: Integration[] = [
new MockIntegration('MockIntegration 0.3'),
Expand Down Expand Up @@ -150,6 +163,7 @@ describe('init', () => {
Object.defineProperty(WINDOW, 'browser', { value: undefined, writable: true });
Object.defineProperty(WINDOW, 'nw', { value: undefined, writable: true });
Object.defineProperty(WINDOW, 'window', { value: WINDOW, writable: true });
vi.clearAllMocks();
});

it('logs a browser extension error if executed inside a Chrome extension', () => {
Expand Down

0 comments on commit 061042a

Please sign in to comment.