Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const iframe = document.createElement('iframe');

iframe.srcdoc = `
<script>
try {
throw new Error('iframe root error', {
cause: new Error('iframe cause error'),
});
} catch (error) {
parent.Sentry.captureException(error);
}
<\/script>
`;

document.body.appendChild(iframe);
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('captures causes from errors thrown in an iframe @firefox', async ({ getLocalTestUrl, page }) => {
const url = await getLocalTestUrl({ testDir: __dirname });
const req = await waitForErrorRequestOnUrl(page, url);
const eventData = envelopeRequestParser(req);

expect(eventData.exception?.values).toHaveLength(2);
expect(eventData.exception?.values).toEqual([
expect.objectContaining({
type: 'Error',
value: 'iframe cause error',
mechanism: {
exception_id: 1,
handled: true,
parent_id: 0,
source: 'cause',
type: 'chained',
},
}),
expect.objectContaining({
type: 'Error',
value: 'iframe root error',
mechanism: {
exception_id: 0,
handled: true,
type: 'generic',
},
}),
]);
});
2 changes: 1 addition & 1 deletion packages/browser/src/eventbuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>): Error | undefined {
return Object.values(obj).find((v): v is Error => v instanceof Error);
return Object.values(obj).find(isError);
}
17 changes: 17 additions & 0 deletions packages/browser/test/eventbuilder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');

Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/instrument/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/utils/aggregate-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils/eventbuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function getErrorPropertyFromObject(obj: Record<string, unknown>): 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;
}
}
Expand Down
60 changes: 43 additions & 17 deletions packages/core/test/lib/instrument/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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<ReturnType<typeof loadFetchModule>>['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<typeof fetch>().mockRejectedValue(error);
addFetchInstrumentationHandler(() => undefined);

await expect(globalThis.fetch('https://example.com/path')).rejects.toBe(error);

expect(error.message).toBe('Failed to fetch (example.com)');
});
});
19 changes: 19 additions & 0 deletions packages/core/test/lib/utils/aggregate-errors.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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');

Expand Down
17 changes: 17 additions & 0 deletions packages/core/test/lib/utils/eventbuilder.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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');

Expand Down
Loading