Skip to content

Commit a0318d8

Browse files
mydeamydea
committed
feat(browser): Improve BrowserClient default data handling
1 parent 593d5b3 commit a0318d8

File tree

5 files changed

+180
-230
lines changed

5 files changed

+180
-230
lines changed

‎packages/browser/src/client.ts

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,24 @@ import { eventFromException, eventFromMessage } from './eventbuilder';
2121
import { WINDOW } from './helpers';
2222
import type { BrowserTransportOptions } from './transports/types';
2323

24+
/**
25+
* A magic string that build tooling can leverage in order to inject a release value into the SDK.
26+
*/
27+
declare const __SENTRY_RELEASE__: string | undefined;
28+
2429
const DEFAULT_FLUSH_INTERVAL = 5000;
2530

31+
type BrowserSpecificOptions = BrowserClientReplayOptions &
32+
BrowserClientProfilingOptions & {
33+
/** If configured, this URL will be used as base URL for lazy loading integration. */
34+
cdnBaseUrl?: string;
35+
};
2636
/**
2737
* Configuration options for the Sentry Browser SDK.
2838
* @see @sentry/core Options for more information.
2939
*/
3040
export type BrowserOptions = Options<BrowserTransportOptions> &
31-
BrowserClientReplayOptions &
32-
BrowserClientProfilingOptions & {
41+
BrowserSpecificOptions & {
3342
/**
3443
* Important: Only set this option if you know what you are doing!
3544
*
@@ -54,12 +63,7 @@ export type BrowserOptions = Options<BrowserTransportOptions> &
5463
* Configuration options for the Sentry Browser SDK Client class
5564
* @see BrowserClient for more information.
5665
*/
57-
export type BrowserClientOptions = ClientOptions<BrowserTransportOptions> &
58-
BrowserClientReplayOptions &
59-
BrowserClientProfilingOptions & {
60-
/** If configured, this URL will be used as base URL for lazy loading integration. */
61-
cdnBaseUrl?: string;
62-
};
66+
export type BrowserClientOptions = ClientOptions<BrowserTransportOptions> & BrowserSpecificOptions;
6367

6468
/**
6569
* The Sentry Browser SDK Client.
@@ -75,11 +79,7 @@ export class BrowserClient extends Client<BrowserClientOptions> {
7579
* @param options Configuration options for this SDK.
7680
*/
7781
public constructor(options: BrowserClientOptions) {
78-
const opts = {
79-
// We default this to true, as it is the safer scenario
80-
parentSpanIsAlwaysRootSpan: true,
81-
...options,
82-
};
82+
const opts = applyDefaultOptions(options);
8383
const sdkSource = WINDOW.SENTRY_SDK_SOURCE || getSDKSource();
8484
applySdkMetadata(opts, 'browser', ['browser'], sdkSource);
8585

@@ -155,3 +155,17 @@ export class BrowserClient extends Client<BrowserClientOptions> {
155155
return super._prepareEvent(event, hint, currentScope, isolationScope);
156156
}
157157
}
158+
159+
/** Exported only for tests. */
160+
export function applyDefaultOptions<T extends Partial<BrowserClientOptions>>(optionsArg: T): T {
161+
return {
162+
release:
163+
typeof __SENTRY_RELEASE__ === 'string' // This allows build tooling to find-and-replace __SENTRY_RELEASE__ to inject a release value
164+
? __SENTRY_RELEASE__
165+
: WINDOW.SENTRY_RELEASE?.id, // This supports the variable that sentry-webpack-plugin injects
166+
sendClientReports: true,
167+
// We default this to true, as it is the safer scenario
168+
parentSpanIsAlwaysRootSpan: true,
169+
...optionsArg,
170+
};
171+
}

‎packages/browser/src/sdk.ts

Lines changed: 10 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,14 @@
11
import type { Client, Integration, Options } from '@sentry/core';
22
import {
3-
consoleSandbox,
43
dedupeIntegration,
54
functionToStringIntegration,
65
getIntegrationsToSetup,
7-
getLocationHref,
86
inboundFiltersIntegration,
97
initAndBind,
108
stackParserFromStackParserOptions,
119
} from '@sentry/core';
1210
import type { BrowserClientOptions, BrowserOptions } from './client';
1311
import { BrowserClient } from './client';
14-
import { DEBUG_BUILD } from './debug-build';
15-
import { WINDOW } from './helpers';
1612
import { breadcrumbsIntegration } from './integrations/breadcrumbs';
1713
import { browserApiErrorsIntegration } from './integrations/browserapierrors';
1814
import { browserSessionIntegration } from './integrations/browsersession';
@@ -21,22 +17,7 @@ import { httpContextIntegration } from './integrations/httpcontext';
2117
import { linkedErrorsIntegration } from './integrations/linkederrors';
2218
import { defaultStackParser } from './stack-parsers';
2319
import { makeFetchTransport } from './transports/fetch';
24-
25-
type ExtensionProperties = {
26-
chrome?: Runtime;
27-
browser?: Runtime;
28-
nw?: unknown;
29-
};
30-
type Runtime = {
31-
runtime?: {
32-
id?: string;
33-
};
34-
};
35-
36-
/**
37-
* A magic string that build tooling can leverage in order to inject a release value into the SDK.
38-
*/
39-
declare const __SENTRY_RELEASE__: string | undefined;
20+
import { checkAndWarnIfIsEmbeddedBrowserExtension } from './utils/detectBrowserExtension';
4021

4122
/** Get the default integrations for the browser SDK. */
4223
export function getDefaultIntegrations(_options: Options): Integration[] {
@@ -59,40 +40,6 @@ export function getDefaultIntegrations(_options: Options): Integration[] {
5940
];
6041
}
6142

62-
/** Exported only for tests. */
63-
export function applyDefaultOptions(optionsArg: BrowserOptions = {}): BrowserOptions {
64-
const defaultOptions: BrowserOptions = {
65-
defaultIntegrations: getDefaultIntegrations(optionsArg),
66-
release:
67-
typeof __SENTRY_RELEASE__ === 'string' // This allows build tooling to find-and-replace __SENTRY_RELEASE__ to inject a release value
68-
? __SENTRY_RELEASE__
69-
: WINDOW.SENTRY_RELEASE?.id, // This supports the variable that sentry-webpack-plugin injects
70-
sendClientReports: true,
71-
};
72-
73-
return {
74-
...defaultOptions,
75-
...dropTopLevelUndefinedKeys(optionsArg),
76-
};
77-
}
78-
79-
/**
80-
* In contrast to the regular `dropUndefinedKeys` method,
81-
* this one does not deep-drop keys, but only on the top level.
82-
*/
83-
function dropTopLevelUndefinedKeys<T extends object>(obj: T): Partial<T> {
84-
const mutatetedObj: Partial<T> = {};
85-
86-
for (const k of Object.getOwnPropertyNames(obj)) {
87-
const key = k as keyof T;
88-
if (obj[key] !== undefined) {
89-
mutatetedObj[key] = obj[key];
90-
}
91-
}
92-
93-
return mutatetedObj;
94-
}
95-
9643
/**
9744
* The Sentry Browser SDK Client.
9845
*
@@ -139,19 +86,21 @@ function dropTopLevelUndefinedKeys<T extends object>(obj: T): Partial<T> {
13986
*
14087
* @see {@link BrowserOptions} for documentation on configuration options.
14188
*/
142-
export function init(browserOptions: BrowserOptions = {}): Client | undefined {
143-
if (!browserOptions.skipBrowserExtensionCheck && _checkForBrowserExtension()) {
144-
return;
145-
}
89+
export function init(options: BrowserOptions = {}): Client | undefined {
90+
const shouldDisableBecauseIsBrowserExtenstion =
91+
!options.skipBrowserExtensionCheck && checkAndWarnIfIsEmbeddedBrowserExtension();
14692

147-
const options = applyDefaultOptions(browserOptions);
14893
const clientOptions: BrowserClientOptions = {
94+
enabled: !shouldDisableBecauseIsBrowserExtenstion,
14995
...options,
15096
stackParser: stackParserFromStackParserOptions(options.stackParser || defaultStackParser),
151-
integrations: getIntegrationsToSetup(options),
97+
integrations: getIntegrationsToSetup({
98+
integrations: options.integrations,
99+
defaultIntegrations:
100+
options.defaultIntegrations == null ? getDefaultIntegrations(options) : options.defaultIntegrations,
101+
}),
152102
transport: options.transport || makeFetchTransport,
153103
};
154-
155104
return initAndBind(BrowserClient, clientOptions);
156105
}
157106

@@ -170,48 +119,3 @@ export function forceLoad(): void {
170119
export function onLoad(callback: () => void): void {
171120
callback();
172121
}
173-
174-
function _isEmbeddedBrowserExtension(): boolean {
175-
if (typeof WINDOW.window === 'undefined') {
176-
// No need to show the error if we're not in a browser window environment (e.g. service workers)
177-
return false;
178-
}
179-
180-
const _window = WINDOW as typeof WINDOW & ExtensionProperties;
181-
182-
// Running the SDK in NW.js, which appears like a browser extension but isn't, is also fine
183-
// see: https://github.com/getsentry/sentry-javascript/issues/12668
184-
if (_window.nw) {
185-
return false;
186-
}
187-
188-
const extensionObject = _window['chrome'] || _window['browser'];
189-
190-
if (!extensionObject?.runtime?.id) {
191-
return false;
192-
}
193-
194-
const href = getLocationHref();
195-
const extensionProtocols = ['chrome-extension', 'moz-extension', 'ms-browser-extension', 'safari-web-extension'];
196-
197-
// Running the SDK in a dedicated extension page and calling Sentry.init is fine; no risk of data leakage
198-
const isDedicatedExtensionPage =
199-
WINDOW === WINDOW.top && extensionProtocols.some(protocol => href.startsWith(`${protocol}://`));
200-
201-
return !isDedicatedExtensionPage;
202-
}
203-
204-
function _checkForBrowserExtension(): true | void {
205-
if (_isEmbeddedBrowserExtension()) {
206-
if (DEBUG_BUILD) {
207-
consoleSandbox(() => {
208-
// eslint-disable-next-line no-console
209-
console.error(
210-
'[Sentry] You cannot use Sentry.init() in a browser extension, see: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
211-
);
212-
});
213-
}
214-
215-
return true;
216-
}
217-
}

‎packages/browser/src/utils/detectBrowserExtension.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { consoleSandbox, getLocationHref } from '@sentry/core';
2+
import { DEBUG_BUILD } from '../debug-build';
3+
import { WINDOW } from '../helpers';
4+
5+
type ExtensionRuntime = {
6+
runtime?: {
7+
id?: string;
8+
};
9+
};
10+
type ExtensionProperties = {
11+
chrome?: ExtensionRuntime;
12+
browser?: ExtensionRuntime;
13+
nw?: unknown;
14+
};
15+
16+
/**
17+
* Returns true if the SDK is running in an embedded browser extension.
18+
* Stand-alone browser extensions (which do not share the same data as the main browser page) are fine.
19+
*/
20+
export function checkAndWarnIfIsEmbeddedBrowserExtension(): boolean {
21+
if (_isEmbeddedBrowserExtension()) {
22+
if (DEBUG_BUILD) {
23+
consoleSandbox(() => {
24+
// eslint-disable-next-line no-console
25+
console.error(
26+
'[Sentry] You cannot use Sentry.init() in a browser extension, see: https://docs.sentry.io/platforms/javascript/best-practices/browser-extensions/',
27+
);
28+
});
29+
}
30+
31+
return true;
32+
}
33+
34+
return false;
35+
}
36+
37+
function _isEmbeddedBrowserExtension(): boolean {
38+
if (typeof WINDOW.window === 'undefined') {
39+
// No need to show the error if we're not in a browser window environment (e.g. service workers)
40+
return false;
41+
}
42+
43+
const _window = WINDOW as typeof WINDOW & ExtensionProperties;
44+
45+
// Running the SDK in NW.js, which appears like a browser extension but isn't, is also fine
46+
// see: https://github.com/getsentry/sentry-javascript/issues/12668
47+
if (_window.nw) {
48+
return false;
49+
}
50+
51+
const extensionObject = _window['chrome'] || _window['browser'];
52+
53+
if (!extensionObject?.runtime?.id) {
54+
return false;
55+
}
56+
57+
const href = getLocationHref();
58+
const extensionProtocols = ['chrome-extension', 'moz-extension', 'ms-browser-extension', 'safari-web-extension'];
59+
60+
// Running the SDK in a dedicated extension page and calling Sentry.init is fine; no risk of data leakage
61+
const isDedicatedExtensionPage =
62+
WINDOW === WINDOW.top && extensionProtocols.some(protocol => href.startsWith(`${protocol}://`));
63+
64+
return !isDedicatedExtensionPage;
65+
}

‎packages/browser/test/client.test.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import * as sentryCore from '@sentry/core';
66
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
7-
import { BrowserClient } from '../src/client';
7+
import { applyDefaultOptions, BrowserClient } from '../src/client';
88
import { WINDOW } from '../src/helpers';
99
import { getDefaultBrowserClientOptions } from './helper/browser-client-options';
1010

@@ -118,3 +118,70 @@ describe('BrowserClient', () => {
118118
});
119119
});
120120
});
121+
122+
describe('applyDefaultOptions', () => {
123+
it('works with empty options', () => {
124+
const options = {};
125+
const actual = applyDefaultOptions(options);
126+
127+
expect(actual).toEqual({
128+
release: undefined,
129+
sendClientReports: true,
130+
parentSpanIsAlwaysRootSpan: true,
131+
});
132+
});
133+
134+
it('works with options', () => {
135+
const options = {
136+
tracesSampleRate: 0.5,
137+
release: '1.0.0',
138+
};
139+
const actual = applyDefaultOptions(options);
140+
141+
expect(actual).toEqual({
142+
release: '1.0.0',
143+
sendClientReports: true,
144+
tracesSampleRate: 0.5,
145+
parentSpanIsAlwaysRootSpan: true,
146+
});
147+
});
148+
149+
it('picks up release from WINDOW.SENTRY_RELEASE.id', () => {
150+
const releaseBefore = WINDOW.SENTRY_RELEASE;
151+
152+
WINDOW.SENTRY_RELEASE = { id: '1.0.0' };
153+
const options = {
154+
tracesSampleRate: 0.5,
155+
};
156+
const actual = applyDefaultOptions(options);
157+
158+
expect(actual).toEqual({
159+
release: '1.0.0',
160+
sendClientReports: true,
161+
tracesSampleRate: 0.5,
162+
parentSpanIsAlwaysRootSpan: true,
163+
});
164+
165+
WINDOW.SENTRY_RELEASE = releaseBefore;
166+
});
167+
168+
it('passed in release takes precedence over WINDOW.SENTRY_RELEASE.id', () => {
169+
const releaseBefore = WINDOW.SENTRY_RELEASE;
170+
171+
WINDOW.SENTRY_RELEASE = { id: '1.0.0' };
172+
const options = {
173+
release: '2.0.0',
174+
tracesSampleRate: 0.5,
175+
};
176+
const actual = applyDefaultOptions(options);
177+
178+
expect(actual).toEqual({
179+
release: '2.0.0',
180+
sendClientReports: true,
181+
tracesSampleRate: 0.5,
182+
parentSpanIsAlwaysRootSpan: true,
183+
});
184+
185+
WINDOW.SENTRY_RELEASE = releaseBefore;
186+
});
187+
});

0 commit comments

Comments
 (0)