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
2 changes: 1 addition & 1 deletion packages/browser-utils/src/instrumentation/xhr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function instrumentXHR(): void {
const method = isString(xhrOpenArgArray[0]) ? xhrOpenArgArray[0].toUpperCase() : undefined;
const url = parseXhrUrlArg(xhrOpenArgArray[1]);

if (!method || !url) {
if (!method || url === undefined) {
return originalOpen.apply(xhrOpenThisArg, xhrOpenArgArray);
}

Expand Down
27 changes: 26 additions & 1 deletion packages/browser-utils/test/instrumentation/xhr.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,28 @@
import { afterEach, describe, expect, it, vi } from 'vitest';
import { addXhrInstrumentationHandler, instrumentXHR } from '../../src/instrumentation/xhr';
import type { HandlerDataXhr } from '../../src/types';
import type { HandlerDataXhr, SentryWrappedXMLHttpRequest } from '../../src/types';
import { WINDOW } from '../../src/types';

const win = WINDOW as typeof WINDOW & { XMLHttpRequest?: typeof XMLHttpRequest };
const originalXMLHttpRequest = win.XMLHttpRequest;

function openWithUrl(url: unknown): SentryWrappedXMLHttpRequest['__sentry_xhr_v3__'] {
class MockXMLHttpRequest {
public addEventListener(): void {}
public removeEventListener(): void {}
public open(_method: string, _url: string): void {}
public send(): void {}
public setRequestHeader(_header: string, _value: string): void {}
}

win.XMLHttpRequest = MockXMLHttpRequest as unknown as typeof XMLHttpRequest;
instrumentXHR();

const xhr = new MockXMLHttpRequest();
xhr.open('GET', url as string);
return (xhr as unknown as SentryWrappedXMLHttpRequest).__sentry_xhr_v3__;
}

describe('instrumentXHR', () => {
afterEach(() => {
win.XMLHttpRequest = originalXMLHttpRequest;
Expand All @@ -16,6 +33,14 @@ describe('instrumentXHR', () => {
expect(instrumentXHR).not.toThrow();
});

it('instruments requests with an empty URL', () => {
expect(openWithUrl('')).toMatchObject({ method: 'GET', url: '' });
});

it('does not instrument requests whose URL cannot be parsed', () => {
expect(openWithUrl(undefined)).toBeUndefined();
});

it('removes readystatechange event listener on readyState 4 when registered with addEventListener', () => {
const addEventListenerSpy = vi.fn();
const removeEventListenerSpy = vi.fn();
Expand Down
11 changes: 8 additions & 3 deletions packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ function _getXhrBreadcrumbHandler(client: Client): (handlerData: HandlerDataXhr)

const data: XhrBreadcrumbData = {
method,
url: filterCollectedUrl(url),
url: getBreadcrumbUrl(url, client),
status_code,
};

Expand Down Expand Up @@ -248,7 +248,7 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe

const breadcrumb = {
category: 'fetch',
data: { ...handlerData.fetchData, url: filterCollectedUrl(handlerData.fetchData.url) },
data: { ...handlerData.fetchData, url: getBreadcrumbUrl(handlerData.fetchData.url, client) },
level: 'error',
type: 'http',
} satisfies Breadcrumb;
Expand All @@ -260,7 +260,7 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe
const response = handlerData.response as Response | undefined;
const data: FetchBreadcrumbData = {
...handlerData.fetchData,
url: filterCollectedUrl(handlerData.fetchData.url),
url: getBreadcrumbUrl(handlerData.fetchData.url, client),
status_code: response?.status,
};

Expand Down Expand Up @@ -324,6 +324,11 @@ function _getHistoryBreadcrumbHandler(client: Client): (handlerData: HandlerData
};
}

function getBreadcrumbUrl(url: string, client?: Client): string {
const pageUrl = url === '' ? WINDOW.location?.href : undefined;
return filterCollectedUrl(pageUrl ? (pageUrl.split('#')[0] ?? '') : url, client);
}
Comment thread
cursor[bot] marked this conversation as resolved.
Resolved

function _isEvent(event: unknown): event is Event {
return !!event && !!(event as Record<string, unknown>).target;
}
126 changes: 123 additions & 3 deletions packages/browser/test/integrations/breadcrumbs.test.ts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some test nits:

  • consider using toHaveBeenCalledWith instead of mock.calls
  • restoreAllMocks instead of resetAllMocks, and unset the client after each test

Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
import * as SentryCore from '@sentry/core';
import { describe, expect, it, vi } from 'vitest';
import * as browserUtils from '@sentry/browser-utils';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { breadcrumbsIntegration, BrowserClient, flush } from '../../src';
import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';

describe('Breadcrumbs', () => {
afterEach(() => {
vi.unstubAllGlobals();
vi.restoreAllMocks();
SentryCore.getCurrentScope().setClient(undefined);
});

it('Should add sentry breadcrumb', async () => {
const client = new BrowserClient({
...getDefaultBrowserClientOptions(),
Expand All @@ -19,7 +26,120 @@ describe('Breadcrumbs', () => {
client.captureMessage('test');
await flush(2000);

expect(addBreadcrumbSpy.mock.calls[0]?.[0]?.category).toEqual('sentry.event');
expect(addBreadcrumbSpy.mock.calls[0]?.[0]?.message).toEqual('test');
expect(addBreadcrumbSpy).toHaveBeenCalledWith(
expect.objectContaining({
category: 'sentry.event',
message: 'test',
}),
expect.anything(),
);
});

describe('empty request URLs', () => {
function setup() {
let fetchHandler: ((data: SentryCore.HandlerDataFetch) => void) | undefined;
let xhrHandler: ((data: browserUtils.HandlerDataXhr) => void) | undefined;

vi.spyOn(SentryCore, 'addFetchInstrumentationHandler').mockImplementation(handler => {
fetchHandler = handler;
});
vi.spyOn(browserUtils, 'addXhrInstrumentationHandler').mockImplementation(handler => {
xhrHandler = handler;
});
const addBreadcrumbSpy = vi.spyOn(SentryCore, 'addBreadcrumb').mockImplementation(() => {});

const client = new BrowserClient({
...getDefaultBrowserClientOptions(),
dsn: 'https://username@domain/123',
integrations: [breadcrumbsIntegration()],
});
SentryCore.setCurrentClient(client);
client.init();

return { addBreadcrumbSpy, fetchHandler: fetchHandler!, xhrHandler: xhrHandler! };
}

const pageHref = 'https://app.example.com/users/42?token=secret#section';
const expectedBreadcrumbUrl = 'https://app.example.com/users/42?token=[Filtered]';

it('resolves an empty fetch URL against location.href, drops fragment, and filters query params', () => {
vi.stubGlobal('location', { href: pageHref });
const { addBreadcrumbSpy, fetchHandler } = setup();

fetchHandler({
fetchData: { method: 'GET', url: '' },
args: [''],
startTimestamp: 1,
endTimestamp: 2,
response: { status: 200 } as Response,
});

expect(addBreadcrumbSpy).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({ url: expectedBreadcrumbUrl }),
}),
expect.anything(),
);
});

it('resolves an empty failed fetch URL against location.href, drops fragment, and filters query params', () => {
vi.stubGlobal('location', { href: pageHref });
const { addBreadcrumbSpy, fetchHandler } = setup();

fetchHandler({
fetchData: { method: 'GET', url: '' },
args: [''],
startTimestamp: 1,
endTimestamp: 2,
error: new TypeError('Failed to fetch'),
});

expect(addBreadcrumbSpy).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({ url: expectedBreadcrumbUrl }),
}),
expect.anything(),
);
});

it('resolves an empty XHR URL against location.href, drops fragment, and filters query params', () => {
vi.stubGlobal('location', { href: pageHref });
const { addBreadcrumbSpy, xhrHandler } = setup();

xhrHandler({
xhr: {
[browserUtils.SENTRY_XHR_DATA_KEY]: { method: 'GET', url: '', status_code: 200, request_headers: {} },
},
startTimestamp: 1,
endTimestamp: 2,
} as browserUtils.HandlerDataXhr);

expect(addBreadcrumbSpy).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({ url: expectedBreadcrumbUrl }),
}),
expect.anything(),
);
});

it('leaves non-empty URLs unchanged', () => {
vi.stubGlobal('location', { href: pageHref });
const { addBreadcrumbSpy, fetchHandler } = setup();

fetchHandler({
fetchData: { method: 'GET', url: '/api/users' },
args: ['/api/users'],
startTimestamp: 1,
endTimestamp: 2,
response: { status: 200 } as Response,
});

expect(addBreadcrumbSpy).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({ url: '/api/users' }),
}),
expect.anything(),
);
});
});
});
Loading