From 07be796ddf8df2ea0d0b6ee8a29e22fc802e984d Mon Sep 17 00:00:00 2001 From: David Murdoch <187813+davidmurdoch@users.noreply.github.com> Date: Fri, 31 Jul 2026 16:49:21 -0400 Subject: [PATCH 1/2] fix(core,browser): Handle errors from other realms Replace realm-sensitive Error constructor checks with the existing isError helper. Preserve the fetch TypeError restriction by checking the error name, and add cross-realm regression coverage. Co-Authored-By: OpenAI Codex --- packages/browser/src/eventbuilder.ts | 2 +- packages/browser/test/eventbuilder.test.ts | 17 ++++++ packages/core/src/instrument/fetch.ts | 3 +- packages/core/src/utils/aggregate-errors.ts | 8 +-- packages/core/src/utils/eventbuilder.ts | 2 +- .../core/test/lib/instrument/fetch.test.ts | 60 +++++++++++++------ .../test/lib/utils/aggregate-errors.test.ts | 19 ++++++ .../core/test/lib/utils/eventbuilder.test.ts | 17 ++++++ 8 files changed, 104 insertions(+), 24 deletions(-) diff --git a/packages/browser/src/eventbuilder.ts b/packages/browser/src/eventbuilder.ts index 948732738c70..824dd225d247 100644 --- a/packages/browser/src/eventbuilder.ts +++ b/packages/browser/src/eventbuilder.ts @@ -408,5 +408,5 @@ function getObjectClassName(obj: unknown): string | undefined | void { /** If a plain object has a property that is an `Error`, return this error. */ function getErrorPropertyFromObject(obj: Record): Error | undefined { - return Object.values(obj).find((v): v is Error => v instanceof Error); + return Object.values(obj).find(isError); } diff --git a/packages/browser/test/eventbuilder.test.ts b/packages/browser/test/eventbuilder.test.ts index 13386010cfdd..408f6e60a658 100644 --- a/packages/browser/test/eventbuilder.test.ts +++ b/packages/browser/test/eventbuilder.test.ts @@ -2,6 +2,7 @@ * @vitest-environment jsdom */ +import { runInNewContext } from 'node:vm'; import { addNonEnumerableProperty } from '@sentry/core/browser'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { defaultStackParser } from '../src'; @@ -140,6 +141,22 @@ describe('eventFromUnknownInput', () => { }); }); + it('handles object with error prop created in another realm', () => { + const error = runInNewContext(`new Error('Some error')`) as Error; + expect(error).not.toBeInstanceOf(Error); + + const event = eventFromUnknownInput(defaultStackParser, { + err: error, + }); + + expect(event.exception?.values?.[0]).toEqual( + expect.objectContaining({ + type: 'Error', + value: 'Some error', + }), + ); + }); + it('handles class with error prop', () => { const error = new Error('Some error'); diff --git a/packages/core/src/instrument/fetch.ts b/packages/core/src/instrument/fetch.ts index 88998a2dc6f6..443c71095e67 100644 --- a/packages/core/src/instrument/fetch.ts +++ b/packages/core/src/instrument/fetch.ts @@ -129,7 +129,8 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void): void { if ( shouldEnhance && - error instanceof TypeError && + isError(error) && + error.name === 'TypeError' && (error.message === 'Failed to fetch' || error.message === 'Load failed' || error.message === 'NetworkError when attempting to fetch resource.') diff --git a/packages/core/src/utils/aggregate-errors.ts b/packages/core/src/utils/aggregate-errors.ts index 947a998f4970..ef006d12e69d 100644 --- a/packages/core/src/utils/aggregate-errors.ts +++ b/packages/core/src/utils/aggregate-errors.ts @@ -2,7 +2,7 @@ import type { ExtendedError } from '../types/error'; import type { Event, EventHint } from '../types/event'; import type { Exception } from '../types/exception'; import type { StackParser } from '../types/stacktrace'; -import { isInstanceOf } from './is'; +import { isError } from './is'; /** * 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( event: Event, hint?: EventHint, ): void { - if (!event.exception?.values || !hint || !isInstanceOf(hint.originalException, Error)) { + if (!event.exception?.values || !hint || !isError(hint.originalException)) { return; } @@ -55,7 +55,7 @@ function aggregateExceptionsFromError( let newExceptions = [...prevExceptions]; // Recursively call this function in order to walk down a chain of errors - if (isInstanceOf(error[key], Error)) { + if (isError(error[key])) { applyExceptionGroupFieldsForParentException(exception, exceptionId, error); const newException = exceptionFromErrorImplementation(parser, error[key]); const newExceptionId = newExceptions.length; @@ -76,7 +76,7 @@ function aggregateExceptionsFromError( // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/AggregateError if (isExceptionGroup(error)) { error.errors.forEach((childError, i) => { - if (isInstanceOf(childError, Error)) { + if (isError(childError)) { applyExceptionGroupFieldsForParentException(exception, exceptionId, error); const newException = exceptionFromErrorImplementation(parser, childError); const newExceptionId = newExceptions.length; diff --git a/packages/core/src/utils/eventbuilder.ts b/packages/core/src/utils/eventbuilder.ts index 8c1525ef7e74..9b300c59c593 100644 --- a/packages/core/src/utils/eventbuilder.ts +++ b/packages/core/src/utils/eventbuilder.ts @@ -63,7 +63,7 @@ function getErrorPropertyFromObject(obj: Record): Error | undef for (const prop in obj) { if (Object.prototype.hasOwnProperty.call(obj, prop)) { const value = obj[prop]; - if (value instanceof Error) { + if (isError(value)) { return value; } } diff --git a/packages/core/test/lib/instrument/fetch.test.ts b/packages/core/test/lib/instrument/fetch.test.ts index fd6d1d294fe4..612f027d2ded 100644 --- a/packages/core/test/lib/instrument/fetch.test.ts +++ b/packages/core/test/lib/instrument/fetch.test.ts @@ -1,7 +1,6 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { addFetchInstrumentationHandler, parseFetchArgs } from '../../../src/instrument/fetch'; -import { resetInstrumentationHandlers } from '../../../src/instrument/handlers'; -import * as isBrowserModule from '../../../src/utils/isBrowser'; +import { runInNewContext } from 'node:vm'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { parseFetchArgs } from '../../../src/instrument/fetch'; import { GLOBAL_OBJ } from '../../../src/utils/worldwide'; describe('instrument > parseFetchArgs', () => { @@ -59,30 +58,57 @@ describe('instrument > parseFetchArgs', () => { describe('instrument > addFetchInstrumentationHandler', () => { const globalWithFetch = GLOBAL_OBJ as typeof GLOBAL_OBJ & { fetch?: (...args: unknown[]) => unknown }; + const originalFetchDescriptor = Object.getOwnPropertyDescriptor(globalWithFetch, 'fetch'); + + // `maybeInstrument` patches the global `fetch` only once per module instance, so each test needs a + // fresh copy of the instrumentation modules - otherwise only the first one actually wraps `fetch`. + async function loadFetchModule() { + vi.resetModules(); + const isBrowserModule = await import('../../../src/utils/isBrowser'); + // Non-browser runtime so we skip the native-fetch check and always patch + vi.spyOn(isBrowserModule, 'isBrowser').mockReturnValue(false); + return import('../../../src/instrument/fetch'); + } + + let addFetchInstrumentationHandler: Awaited>['addFetchInstrumentationHandler']; + + beforeEach(async () => { + ({ addFetchInstrumentationHandler } = await loadFetchModule()); + }); afterEach(() => { - resetInstrumentationHandlers(); + if (originalFetchDescriptor) { + Object.defineProperty(globalWithFetch, 'fetch', originalFetchDescriptor); + } else { + Reflect.deleteProperty(globalWithFetch, 'fetch'); + } + vi.restoreAllMocks(); }); it('preserves non-standard own properties on the global fetch (e.g. Bun `fetch.preconnect`)', () => { - // Non-browser runtime so we skip the native-fetch check and always patch - vi.spyOn(isBrowserModule, 'isBrowser').mockReturnValue(false); - const preconnect = vi.fn(); const originalFetch = vi.fn(() => Promise.resolve(new Response())); (originalFetch as unknown as { preconnect: unknown }).preconnect = preconnect; globalWithFetch.fetch = originalFetch as unknown as typeof globalWithFetch.fetch; - try { - addFetchInstrumentationHandler(() => {}); + addFetchInstrumentationHandler(() => {}); - // fetch was actually wrapped ... - expect(globalWithFetch.fetch).not.toBe(originalFetch); - // ... and the non-standard own property was carried over onto the wrapper - expect((globalWithFetch.fetch as unknown as { preconnect: unknown }).preconnect).toBe(preconnect); - } finally { - globalWithFetch.fetch = originalFetch as unknown as typeof globalWithFetch.fetch; - } + // fetch was actually wrapped ... + expect(globalWithFetch.fetch).not.toBe(originalFetch); + // ... and the non-standard own property was carried over onto the wrapper + expect((globalWithFetch.fetch as unknown as { preconnect: unknown }).preconnect).toBe(preconnect); + }); + + it('enhances a fetch TypeError created in another realm', async () => { + const error = runInNewContext(`new TypeError('Failed to fetch')`) as TypeError; + expect(error).not.toBeInstanceOf(TypeError); + + globalThis.fetch = vi.fn().mockRejectedValue(error); + addFetchInstrumentationHandler(() => undefined); + + await expect(globalThis.fetch('https://example.com/path')).rejects.toBe(error); + + expect(error.message).toBe('Failed to fetch (example.com)'); }); }); diff --git a/packages/core/test/lib/utils/aggregate-errors.test.ts b/packages/core/test/lib/utils/aggregate-errors.test.ts index ac9e0c4f3bc5..3d51bffe520d 100644 --- a/packages/core/test/lib/utils/aggregate-errors.test.ts +++ b/packages/core/test/lib/utils/aggregate-errors.test.ts @@ -1,3 +1,4 @@ +import { runInNewContext } from 'node:vm'; import { describe, expect, test } from 'vitest'; import type { ExtendedError } from '../../../src/types/error'; import type { Event, EventHint } from '../../../src/types/event'; @@ -115,6 +116,24 @@ describe('applyAggregateErrorsToEvent()', () => { }); }); + test('recursively walks errors created in another realm', () => { + const originalException = runInNewContext( + `new AggregateError([new Error('Aggregate child')], 'Root Error', { cause: new Error('Cause') })`, + ) as ExtendedError; + expect(originalException).not.toBeInstanceOf(Error); + + const event: Event = { exception: { values: [exceptionFromError(stackParser, originalException)] } }; + const eventHint: EventHint = { originalException }; + + applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint); + + expect(event.exception?.values?.map(exception => exception.value)).toStrictEqual([ + 'Aggregate child', + 'Cause', + 'Root Error', + ]); + }); + test('should not modify event if there are no attached errors', () => { const originalException: ExtendedError = new Error('Some Error'); diff --git a/packages/core/test/lib/utils/eventbuilder.test.ts b/packages/core/test/lib/utils/eventbuilder.test.ts index b882a4562b1c..2a08f073117e 100644 --- a/packages/core/test/lib/utils/eventbuilder.test.ts +++ b/packages/core/test/lib/utils/eventbuilder.test.ts @@ -1,3 +1,4 @@ +import { runInNewContext } from 'node:vm'; import { describe, expect, it, test } from 'vitest'; import type { Client } from '../../../src/client'; import { eventFromMessage, eventFromUnknownInput, exceptionFromError } from '../../../src/utils/eventbuilder'; @@ -106,6 +107,22 @@ describe('eventFromUnknownInput', () => { }); }); + test('object with error prop created in another realm', () => { + const error = runInNewContext(`new Error('Some error')`) as Error; + expect(error).not.toBeInstanceOf(Error); + + const event = eventFromUnknownInput(fakeClient, stackParser, { + err: error, + }); + + expect(event.exception?.values?.[0]).toEqual( + expect.objectContaining({ + type: 'Error', + value: 'Some error', + }), + ); + }); + it('handles class with error prop', () => { const error = new Error('Some error'); From 692150a39994ebd54d7448ef7501faf58d6e1d16 Mon Sep 17 00:00:00 2001 From: David Murdoch <187813+davidmurdoch@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:00:55 -0400 Subject: [PATCH 2/2] test(browser): Add cross-realm error cause integration test --- .../cross-realm-error-cause/subject.js | 15 +++++++++ .../cross-realm-error-cause/test.ts | 33 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 dev-packages/browser-integration-tests/suites/public-api/captureException/cross-realm-error-cause/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/captureException/cross-realm-error-cause/test.ts diff --git a/dev-packages/browser-integration-tests/suites/public-api/captureException/cross-realm-error-cause/subject.js b/dev-packages/browser-integration-tests/suites/public-api/captureException/cross-realm-error-cause/subject.js new file mode 100644 index 000000000000..8735e8b46a1b --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/captureException/cross-realm-error-cause/subject.js @@ -0,0 +1,15 @@ +const iframe = document.createElement('iframe'); + +iframe.srcdoc = ` +