Skip to content

Commit 6516bc9

Browse files
s1gr1ds1gr1d
committed
less mutation on options; review comments
1 parent 4ec7a7a commit 6516bc9

File tree

4 files changed

+78
-77
lines changed
  • dev-packages/browser-integration-tests/suites/manual-client
  • packages/browser

4 files changed

+78
-77
lines changed

‎dev-packages/browser-integration-tests/suites/manual-client/skip-init-browser-extension/test.ts

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,34 @@
11
import { expect } from '@playwright/test';
22
import { sentryTest } from '../../../utils/fixtures';
33

4-
sentryTest('should not initialize when inside a Chrome browser extension', async ({ getLocalTestUrl, page }) => {
5-
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
6-
return route.fulfill({
7-
status: 200,
8-
contentType: 'application/json',
9-
body: JSON.stringify({ id: 'test-id' }),
4+
sentryTest(
5+
'should not initialize when inside a Firefox/Safari browser extension',
6+
async ({ getLocalTestUrl, page }) => {
7+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
8+
return route.fulfill({
9+
status: 200,
10+
contentType: 'application/json',
11+
body: JSON.stringify({ id: 'test-id' }),
12+
});
1013
});
11-
});
1214

13-
const errorLogs: string[] = [];
15+
const errorLogs: string[] = [];
1416

15-
page.on('console', message => {
16-
if (message.type() === 'error') errorLogs.push(message.text());
17-
});
17+
page.on('console', message => {
18+
if (message.type() === 'error') errorLogs.push(message.text());
19+
});
1820

19-
const url = await getLocalTestUrl({ testDir: __dirname });
20-
await page.goto(url);
21+
const url = await getLocalTestUrl({ testDir: __dirname });
22+
await page.goto(url);
2123

22-
const isInitialized = await page.evaluate(() => {
23-
return !!(window as any).Sentry.isInitialized();
24-
});
24+
const isInitialized = await page.evaluate(() => {
25+
return !!(window as any).Sentry.isInitialized();
26+
});
2527

26-
expect(isInitialized).toEqual(false);
27-
expect(errorLogs.length).toEqual(1);
28-
expect(errorLogs[0]).toEqual(
29-
'[Sentry] You cannot run Sentry this way in an extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
30-
);
31-
});
28+
expect(isInitialized).toEqual(false);
29+
expect(errorLogs.length).toEqual(1);
30+
expect(errorLogs[0]).toEqual(
31+
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
32+
);
33+
},
34+
);

‎dev-packages/browser-integration-tests/suites/manual-client/skip-init-chrome-extension/test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ sentryTest('should not initialize when inside a Chrome browser extension', async
2626
expect(isInitialized).toEqual(false);
2727
expect(errorLogs.length).toEqual(1);
2828
expect(errorLogs[0]).toEqual(
29-
'[Sentry] You cannot run Sentry this way in an extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
29+
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
3030
);
3131
});

‎packages/browser/src/sdk.ts

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,40 @@ export function getDefaultIntegrations(_options: Options): Integration[] {
4444
];
4545
}
4646

47+
function applyDefaultOptions(options: BrowserOptions = {}): BrowserOptions {
48+
const defaultOptions: BrowserOptions = {
49+
defaultIntegrations: getDefaultIntegrations(options),
50+
release:
51+
typeof __SENTRY_RELEASE__ === 'string' // This allows build tooling to find-and-replace __SENTRY_RELEASE__ to inject a release value
52+
? __SENTRY_RELEASE__
53+
: WINDOW.SENTRY_RELEASE && WINDOW.SENTRY_RELEASE.id // This supports the variable that sentry-webpack-plugin injects
54+
? WINDOW.SENTRY_RELEASE.id
55+
: undefined,
56+
autoSessionTracking: true,
57+
sendClientReports: true,
58+
};
59+
60+
return { ...defaultOptions, ...options };
61+
}
62+
63+
function shouldShowBrowserExtensionError(): boolean {
64+
const windowWithMaybeChrome = WINDOW as typeof WINDOW & { chrome?: { runtime?: { id?: string } } };
65+
const isInsideChromeExtension =
66+
windowWithMaybeChrome &&
67+
windowWithMaybeChrome.chrome &&
68+
windowWithMaybeChrome.chrome.runtime &&
69+
windowWithMaybeChrome.chrome.runtime.id;
70+
71+
const windowWithMaybeBrowser = WINDOW as typeof WINDOW & { browser?: { runtime?: { id?: string } } };
72+
const isInsideBrowserExtension =
73+
windowWithMaybeBrowser &&
74+
windowWithMaybeBrowser.browser &&
75+
windowWithMaybeBrowser.browser.runtime &&
76+
windowWithMaybeBrowser.browser.runtime.id;
77+
78+
return !!isInsideBrowserExtension || !!isInsideChromeExtension;
79+
}
80+
4781
/**
4882
* A magic string that build tooling can leverage in order to inject a release value into the SDK.
4983
*/
@@ -95,48 +129,14 @@ declare const __SENTRY_RELEASE__: string | undefined;
95129
*
96130
* @see {@link BrowserOptions} for documentation on configuration options.
97131
*/
98-
export function init(options: BrowserOptions = {}): void {
99-
if (options.defaultIntegrations === undefined) {
100-
options.defaultIntegrations = getDefaultIntegrations(options);
101-
}
102-
if (options.release === undefined) {
103-
// This allows build tooling to find-and-replace __SENTRY_RELEASE__ to inject a release value
104-
if (typeof __SENTRY_RELEASE__ === 'string') {
105-
options.release = __SENTRY_RELEASE__;
106-
}
107-
108-
// This supports the variable that sentry-webpack-plugin injects
109-
if (WINDOW.SENTRY_RELEASE && WINDOW.SENTRY_RELEASE.id) {
110-
options.release = WINDOW.SENTRY_RELEASE.id;
111-
}
112-
}
113-
if (options.autoSessionTracking === undefined) {
114-
options.autoSessionTracking = true;
115-
}
116-
if (options.sendClientReports === undefined) {
117-
options.sendClientReports = true;
118-
}
119-
120-
// inside Chrome extension (chrome.* API)
121-
const windowWithMaybeChrome = WINDOW as typeof WINDOW & { chrome?: { runtime?: { id?: string } } };
122-
123-
// inside Firefox/Safari extension (browser.* API)
124-
const windowWithMaybeBrowser = WINDOW as typeof WINDOW & { browser?: { runtime?: { id?: string } } };
132+
export function init(browserOptions: BrowserOptions = {}): void {
133+
const options = applyDefaultOptions(browserOptions);
125134

126-
if (
127-
(windowWithMaybeBrowser &&
128-
windowWithMaybeBrowser.browser &&
129-
windowWithMaybeBrowser.browser.runtime &&
130-
windowWithMaybeBrowser.browser.runtime.id) ||
131-
(windowWithMaybeChrome &&
132-
windowWithMaybeChrome.chrome &&
133-
windowWithMaybeChrome.chrome.runtime &&
134-
windowWithMaybeChrome.chrome.runtime.id)
135-
) {
135+
if (shouldShowBrowserExtensionError()) {
136136
consoleSandbox(() => {
137137
// eslint-disable-next-line no-console
138138
console.error(
139-
'[Sentry] You cannot run Sentry this way in an extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
139+
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
140140
);
141141
});
142142
return;

‎packages/browser/test/unit/sdk.test.ts

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,9 @@ describe('init', () => {
137137

138138
const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, defaultIntegrations: DEFAULT_INTEGRATIONS });
139139

140-
it('should not log a browser extension error if executed inside regular browser environment', () => {
141-
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
142-
143-
init(options);
144-
145-
expect(consoleErrorSpy).toBeCalledTimes(0);
146-
147-
consoleErrorSpy.mockRestore();
140+
afterEach(() => {
141+
Object.defineProperty(WINDOW, 'chrome', { value: undefined, writable: true });
142+
Object.defineProperty(WINDOW, 'browser', { value: undefined, writable: true });
148143
});
149144

150145
it('should log a browser extension error if executed inside a Chrome extension', () => {
@@ -159,15 +154,10 @@ describe('init', () => {
159154

160155
expect(consoleErrorSpy).toBeCalledTimes(1);
161156
expect(consoleErrorSpy).toHaveBeenCalledWith(
162-
'[Sentry] You cannot run Sentry this way in an extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
157+
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
163158
);
164159

165160
consoleErrorSpy.mockRestore();
166-
167-
Object.defineProperty(WINDOW, 'chrome', {
168-
value: null,
169-
writable: true,
170-
});
171161
});
172162

173163
it('should log a browser extension error if executed inside a Firefox/Safari extension', () => {
@@ -179,12 +169,20 @@ describe('init', () => {
179169

180170
expect(consoleErrorSpy).toBeCalledTimes(1);
181171
expect(consoleErrorSpy).toHaveBeenCalledWith(
182-
'[Sentry] You cannot run Sentry this way in an extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
172+
'[Sentry] You cannot run Sentry this way in a browser extension, check: https://docs.sentry.io/platforms/javascript/troubleshooting/#setting-up-sentry-in-shared-environments-eg-browser-extensions',
183173
);
184174

185175
consoleErrorSpy.mockRestore();
176+
});
177+
178+
it('should not log a browser extension error if executed inside regular browser environment', () => {
179+
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
186180

187-
Object.defineProperty(WINDOW, 'browser', { value: null, writable: true });
181+
init(options);
182+
183+
expect(consoleErrorSpy).toBeCalledTimes(0);
184+
185+
consoleErrorSpy.mockRestore();
188186
});
189187
});
190188
});

0 commit comments

Comments
 (0)