Skip to content

Commit ef51993

Browse files
SorsOpslforst
andauthored
fix(browser): Check for existence of instrumentation targets (#8939)
There are cases when global objects such as the window object are shimmed that they define properties such as `document`, but the actual value is undefined. This exact situation has been occurring and is causing the instrumentDOM function to throw an error as `'document' in WINDOW` is technically true though the value is falsey. We should rather attempt an actual check of the value being truthy rather than if the property is defined Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
1 parent b57e139 commit ef51993

File tree

2 files changed

+22
-5
lines changed

2 files changed

+22
-5
lines changed

packages/utils/src/instrument.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,9 @@ export function parseFetchArgs(fetchArgs: unknown[]): { method: string; url: str
247247
}
248248

249249
/** JSDoc */
250-
function instrumentXHR(): void {
251-
if (!('XMLHttpRequest' in WINDOW)) {
250+
export function instrumentXHR(): void {
251+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
252+
if (!(WINDOW as any).XMLHttpRequest) {
252253
return;
253254
}
254255

@@ -539,8 +540,8 @@ type InstrumentedElement = Element & {
539540
};
540541

541542
/** JSDoc */
542-
function instrumentDOM(): void {
543-
if (!('document' in WINDOW)) {
543+
export function instrumentDOM(): void {
544+
if (!WINDOW.document) {
544545
return;
545546
}
546547

packages/utils/test/instrument.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,22 @@
1-
import { parseFetchArgs } from '../src/instrument';
1+
import { instrumentDOM, instrumentXHR, parseFetchArgs } from '../src/instrument';
2+
3+
jest.mock('../src/worldwide', () => ({
4+
// Return an empty object with undefined properties
5+
getGlobalObject: () => ({
6+
document: undefined,
7+
XMLHttpRequest: undefined,
8+
}),
9+
}));
210

311
describe('instrument', () => {
12+
it('instrumentXHR() does not throw if XMLHttpRequest is a key on window but not defined', () => {
13+
expect(instrumentXHR).not.toThrow();
14+
});
15+
16+
it('instrumentDOM() does not throw if XMLHttpRequest is a key on window but not defined', () => {
17+
expect(instrumentDOM).not.toThrow();
18+
});
19+
420
describe('parseFetchArgs', () => {
521
it.each([
622
['string URL only', ['http://example.com'], { method: 'GET', url: 'http://example.com' }],

0 commit comments

Comments
 (0)