Skip to content

Commit 13660f1

Browse files
authored
fix(core,browser): Handle errors from other realms (#22926)
Some event-processing paths failed to recognize errors created in another JavaScript realm. This preserves linked causes and AggregateError children, recognizes error-valued object properties, and applies fetch TypeError hostname enhancement across realm boundaries. These paths now use the SDK’s existing realm-tolerant `isError` helper. The fetch path additionally checks `error.name === "TypeError"` to preserve its existing type restriction.
1 parent 86170ab commit 13660f1

10 files changed

Lines changed: 152 additions & 24 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
const iframe = document.createElement('iframe');
2+
3+
iframe.srcdoc = `
4+
<script>
5+
try {
6+
throw new Error('iframe root error', {
7+
cause: new Error('iframe cause error'),
8+
});
9+
} catch (error) {
10+
parent.Sentry.captureException(error);
11+
}
12+
<\/script>
13+
`;
14+
15+
document.body.appendChild(iframe);
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../../utils/fixtures';
3+
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
4+
5+
sentryTest('captures causes from errors thrown in an iframe @firefox', async ({ getLocalTestUrl, page }) => {
6+
const url = await getLocalTestUrl({ testDir: __dirname });
7+
const req = await waitForErrorRequestOnUrl(page, url);
8+
const eventData = envelopeRequestParser(req);
9+
10+
expect(eventData.exception?.values).toHaveLength(2);
11+
expect(eventData.exception?.values).toEqual([
12+
expect.objectContaining({
13+
type: 'Error',
14+
value: 'iframe cause error',
15+
mechanism: {
16+
exception_id: 1,
17+
handled: true,
18+
parent_id: 0,
19+
source: 'cause',
20+
type: 'chained',
21+
},
22+
}),
23+
expect.objectContaining({
24+
type: 'Error',
25+
value: 'iframe root error',
26+
mechanism: {
27+
exception_id: 0,
28+
handled: true,
29+
type: 'generic',
30+
},
31+
}),
32+
]);
33+
});

packages/browser/src/eventbuilder.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,5 +408,5 @@ function getObjectClassName(obj: unknown): string | undefined | void {
408408

409409
/** If a plain object has a property that is an `Error`, return this error. */
410410
function getErrorPropertyFromObject(obj: Record<string, unknown>): Error | undefined {
411-
return Object.values(obj).find((v): v is Error => v instanceof Error);
411+
return Object.values(obj).find(isError);
412412
}

packages/browser/test/eventbuilder.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment jsdom
33
*/
44

5+
import { runInNewContext } from 'node:vm';
56
import { addNonEnumerableProperty } from '@sentry/core/browser';
67
import { afterEach, describe, expect, it, vi } from 'vitest';
78
import { defaultStackParser } from '../src';
@@ -140,6 +141,22 @@ describe('eventFromUnknownInput', () => {
140141
});
141142
});
142143

144+
it('handles object with error prop created in another realm', () => {
145+
const error = runInNewContext(`new Error('Some error')`) as Error;
146+
expect(error).not.toBeInstanceOf(Error);
147+
148+
const event = eventFromUnknownInput(defaultStackParser, {
149+
err: error,
150+
});
151+
152+
expect(event.exception?.values?.[0]).toEqual(
153+
expect.objectContaining({
154+
type: 'Error',
155+
value: 'Some error',
156+
}),
157+
);
158+
});
159+
143160
it('handles class with error prop', () => {
144161
const error = new Error('Some error');
145162

packages/core/src/instrument/fetch.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void): void {
129129

130130
if (
131131
shouldEnhance &&
132-
error instanceof TypeError &&
132+
isError(error) &&
133+
error.name === 'TypeError' &&
133134
(error.message === 'Failed to fetch' ||
134135
error.message === 'Load failed' ||
135136
error.message === 'NetworkError when attempting to fetch resource.')

packages/core/src/utils/aggregate-errors.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { ExtendedError } from '../types/error';
22
import type { Event, EventHint } from '../types/event';
33
import type { Exception } from '../types/exception';
44
import type { StackParser } from '../types/stacktrace';
5-
import { isInstanceOf } from './is';
5+
import { isError } from './is';
66

77
/**
88
* Creates exceptions inside `event.exception.values` for errors that are nested on properties based on the `key` parameter.
@@ -15,7 +15,7 @@ export function applyAggregateErrorsToEvent(
1515
event: Event,
1616
hint?: EventHint,
1717
): void {
18-
if (!event.exception?.values || !hint || !isInstanceOf(hint.originalException, Error)) {
18+
if (!event.exception?.values || !hint || !isError(hint.originalException)) {
1919
return;
2020
}
2121

@@ -55,7 +55,7 @@ function aggregateExceptionsFromError(
5555
let newExceptions = [...prevExceptions];
5656

5757
// Recursively call this function in order to walk down a chain of errors
58-
if (isInstanceOf(error[key], Error)) {
58+
if (isError(error[key])) {
5959
applyExceptionGroupFieldsForParentException(exception, exceptionId, error);
6060
const newException = exceptionFromErrorImplementation(parser, error[key]);
6161
const newExceptionId = newExceptions.length;
@@ -76,7 +76,7 @@ function aggregateExceptionsFromError(
7676
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/AggregateError
7777
if (isExceptionGroup(error)) {
7878
error.errors.forEach((childError, i) => {
79-
if (isInstanceOf(childError, Error)) {
79+
if (isError(childError)) {
8080
applyExceptionGroupFieldsForParentException(exception, exceptionId, error);
8181
const newException = exceptionFromErrorImplementation(parser, childError);
8282
const newExceptionId = newExceptions.length;

packages/core/src/utils/eventbuilder.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ function getErrorPropertyFromObject(obj: Record<string, unknown>): Error | undef
6363
for (const prop in obj) {
6464
if (Object.prototype.hasOwnProperty.call(obj, prop)) {
6565
const value = obj[prop];
66-
if (value instanceof Error) {
66+
if (isError(value)) {
6767
return value;
6868
}
6969
}

packages/core/test/lib/instrument/fetch.test.ts

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
import { afterEach, describe, expect, it, vi } from 'vitest';
2-
import { addFetchInstrumentationHandler, parseFetchArgs } from '../../../src/instrument/fetch';
3-
import { resetInstrumentationHandlers } from '../../../src/instrument/handlers';
4-
import * as isBrowserModule from '../../../src/utils/isBrowser';
1+
import { runInNewContext } from 'node:vm';
2+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
3+
import { parseFetchArgs } from '../../../src/instrument/fetch';
54
import { GLOBAL_OBJ } from '../../../src/utils/worldwide';
65

76
describe('instrument > parseFetchArgs', () => {
@@ -59,30 +58,57 @@ describe('instrument > parseFetchArgs', () => {
5958

6059
describe('instrument > addFetchInstrumentationHandler', () => {
6160
const globalWithFetch = GLOBAL_OBJ as typeof GLOBAL_OBJ & { fetch?: (...args: unknown[]) => unknown };
61+
const originalFetchDescriptor = Object.getOwnPropertyDescriptor(globalWithFetch, 'fetch');
62+
63+
// `maybeInstrument` patches the global `fetch` only once per module instance, so each test needs a
64+
// fresh copy of the instrumentation modules - otherwise only the first one actually wraps `fetch`.
65+
async function loadFetchModule() {
66+
vi.resetModules();
67+
const isBrowserModule = await import('../../../src/utils/isBrowser');
68+
// Non-browser runtime so we skip the native-fetch check and always patch
69+
vi.spyOn(isBrowserModule, 'isBrowser').mockReturnValue(false);
70+
return import('../../../src/instrument/fetch');
71+
}
72+
73+
let addFetchInstrumentationHandler: Awaited<ReturnType<typeof loadFetchModule>>['addFetchInstrumentationHandler'];
74+
75+
beforeEach(async () => {
76+
({ addFetchInstrumentationHandler } = await loadFetchModule());
77+
});
6278

6379
afterEach(() => {
64-
resetInstrumentationHandlers();
80+
if (originalFetchDescriptor) {
81+
Object.defineProperty(globalWithFetch, 'fetch', originalFetchDescriptor);
82+
} else {
83+
Reflect.deleteProperty(globalWithFetch, 'fetch');
84+
}
85+
6586
vi.restoreAllMocks();
6687
});
6788

6889
it('preserves non-standard own properties on the global fetch (e.g. Bun `fetch.preconnect`)', () => {
69-
// Non-browser runtime so we skip the native-fetch check and always patch
70-
vi.spyOn(isBrowserModule, 'isBrowser').mockReturnValue(false);
71-
7290
const preconnect = vi.fn();
7391
const originalFetch = vi.fn(() => Promise.resolve(new Response()));
7492
(originalFetch as unknown as { preconnect: unknown }).preconnect = preconnect;
7593
globalWithFetch.fetch = originalFetch as unknown as typeof globalWithFetch.fetch;
7694

77-
try {
78-
addFetchInstrumentationHandler(() => {});
95+
addFetchInstrumentationHandler(() => {});
7996

80-
// fetch was actually wrapped ...
81-
expect(globalWithFetch.fetch).not.toBe(originalFetch);
82-
// ... and the non-standard own property was carried over onto the wrapper
83-
expect((globalWithFetch.fetch as unknown as { preconnect: unknown }).preconnect).toBe(preconnect);
84-
} finally {
85-
globalWithFetch.fetch = originalFetch as unknown as typeof globalWithFetch.fetch;
86-
}
97+
// fetch was actually wrapped ...
98+
expect(globalWithFetch.fetch).not.toBe(originalFetch);
99+
// ... and the non-standard own property was carried over onto the wrapper
100+
expect((globalWithFetch.fetch as unknown as { preconnect: unknown }).preconnect).toBe(preconnect);
101+
});
102+
103+
it('enhances a fetch TypeError created in another realm', async () => {
104+
const error = runInNewContext(`new TypeError('Failed to fetch')`) as TypeError;
105+
expect(error).not.toBeInstanceOf(TypeError);
106+
107+
globalThis.fetch = vi.fn<typeof fetch>().mockRejectedValue(error);
108+
addFetchInstrumentationHandler(() => undefined);
109+
110+
await expect(globalThis.fetch('https://example.com/path')).rejects.toBe(error);
111+
112+
expect(error.message).toBe('Failed to fetch (example.com)');
87113
});
88114
});

packages/core/test/lib/utils/aggregate-errors.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { runInNewContext } from 'node:vm';
12
import { describe, expect, test } from 'vitest';
23
import type { ExtendedError } from '../../../src/types/error';
34
import type { Event, EventHint } from '../../../src/types/event';
@@ -115,6 +116,24 @@ describe('applyAggregateErrorsToEvent()', () => {
115116
});
116117
});
117118

119+
test('recursively walks errors created in another realm', () => {
120+
const originalException = runInNewContext(
121+
`new AggregateError([new Error('Aggregate child')], 'Root Error', { cause: new Error('Cause') })`,
122+
) as ExtendedError;
123+
expect(originalException).not.toBeInstanceOf(Error);
124+
125+
const event: Event = { exception: { values: [exceptionFromError(stackParser, originalException)] } };
126+
const eventHint: EventHint = { originalException };
127+
128+
applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint);
129+
130+
expect(event.exception?.values?.map(exception => exception.value)).toStrictEqual([
131+
'Aggregate child',
132+
'Cause',
133+
'Root Error',
134+
]);
135+
});
136+
118137
test('should not modify event if there are no attached errors', () => {
119138
const originalException: ExtendedError = new Error('Some Error');
120139

packages/core/test/lib/utils/eventbuilder.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { runInNewContext } from 'node:vm';
12
import { describe, expect, it, test } from 'vitest';
23
import type { Client } from '../../../src/client';
34
import { eventFromMessage, eventFromUnknownInput, exceptionFromError } from '../../../src/utils/eventbuilder';
@@ -106,6 +107,22 @@ describe('eventFromUnknownInput', () => {
106107
});
107108
});
108109

110+
test('object with error prop created in another realm', () => {
111+
const error = runInNewContext(`new Error('Some error')`) as Error;
112+
expect(error).not.toBeInstanceOf(Error);
113+
114+
const event = eventFromUnknownInput(fakeClient, stackParser, {
115+
err: error,
116+
});
117+
118+
expect(event.exception?.values?.[0]).toEqual(
119+
expect.objectContaining({
120+
type: 'Error',
121+
value: 'Some error',
122+
}),
123+
);
124+
});
125+
109126
it('handles class with error prop', () => {
110127
const error = new Error('Some error');
111128

0 commit comments

Comments
 (0)