Skip to content

Commit

Permalink
feat(browser): Attach virtual stack traces to HttpClient events (#1…
Browse files Browse the repository at this point in the history
  • Loading branch information
onurtemizkan authored Dec 12, 2024
1 parent 5fe6345 commit 63476fa
Show file tree
Hide file tree
Showing 11 changed files with 95 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ sentryTest(
type: 'http.client',
handled: false,
},
stacktrace: {
frames: expect.arrayContaining([
expect.objectContaining({
filename: 'http://sentry-test.io/subject.bundle.js',
function: '?',
in_app: true,
}),
]),
},
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ sentryTest(
type: 'http.client',
handled: false,
},
stacktrace: {
frames: expect.arrayContaining([
expect.objectContaining({
filename: 'http://sentry-test.io/subject.bundle.js',
function: '?',
in_app: true,
}),
]),
},
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ sentryTest('works with a Request passed in', async ({ getLocalTestUrl, page }) =
type: 'http.client',
handled: false,
},
stacktrace: {
frames: expect.arrayContaining([
expect.objectContaining({
filename: 'http://sentry-test.io/subject.bundle.js',
function: '?',
in_app: true,
}),
]),
},
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ sentryTest(
type: 'http.client',
handled: false,
},
stacktrace: {
frames: expect.arrayContaining([
expect.objectContaining({
filename: 'http://sentry-test.io/subject.bundle.js',
function: '?',
in_app: true,
}),
]),
},
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ sentryTest('works with a Request (without body) & options passed in', async ({ g
type: 'http.client',
handled: false,
},
stacktrace: {
frames: expect.arrayContaining([
expect.objectContaining({
filename: 'http://sentry-test.io/subject.bundle.js',
function: '?',
in_app: true,
}),
]),
},
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ sentryTest(
type: 'http.client',
handled: false,
},
stacktrace: {
frames: expect.arrayContaining([
expect.objectContaining({
filename: 'http://sentry-test.io/subject.bundle.js',
function: '?',
in_app: true,
}),
]),
},
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ class SentryScenarioGenerationPlugin {
}
: {};

// Checking if the current scenario has imported `@sentry/integrations`.
compiler.hooks.normalModuleFactory.tap(this._name, factory => {
factory.hooks.parser.for('javascript/auto').tap(this._name, parser => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
Expand Down
8 changes: 8 additions & 0 deletions packages/browser-utils/src/instrument/xhr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ export function instrumentXHR(): void {
// eslint-disable-next-line @typescript-eslint/unbound-method
xhrproto.open = new Proxy(xhrproto.open, {
apply(originalOpen, xhrOpenThisArg: XMLHttpRequest & SentryWrappedXMLHttpRequest, xhrOpenArgArray) {
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
// it means the error, that was caused by your XHR call did not
// have a stack trace. If you are using HttpClient integration,
// this is the expected behavior, as we are using this virtual error to capture
// the location of your XHR call, and group your HttpClient events accordingly.
const virtualError = new Error();

const startTimestamp = timestampInSeconds() * 1000;

// open() should always be called with two or more arguments
Expand Down Expand Up @@ -74,6 +81,7 @@ export function instrumentXHR(): void {
endTimestamp: timestampInSeconds() * 1000,
startTimestamp,
xhr: xhrOpenThisArg,
virtualError,
};
triggerHandlers('xhr', handlerData);
}
Expand Down
20 changes: 16 additions & 4 deletions packages/browser/src/integrations/httpclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ function _fetchResponseHandler(
requestInfo: RequestInfo,
response: Response,
requestInit?: RequestInit,
error?: unknown,
): void {
if (_shouldCaptureResponse(options, response.status, response.url)) {
const request = _getRequest(requestInfo, requestInit);
Expand All @@ -92,6 +93,7 @@ function _fetchResponseHandler(
responseHeaders,
requestCookies,
responseCookies,
error,
});

captureEvent(event);
Expand Down Expand Up @@ -130,6 +132,7 @@ function _xhrResponseHandler(
xhr: XMLHttpRequest,
method: string,
headers: Record<string, string>,
error?: unknown,
): void {
if (_shouldCaptureResponse(options, xhr.status, xhr.responseURL)) {
let requestHeaders, responseCookies, responseHeaders;
Expand Down Expand Up @@ -162,6 +165,7 @@ function _xhrResponseHandler(
// Can't access request cookies from XHR
responseHeaders,
responseCookies,
error,
});

captureEvent(event);
Expand Down Expand Up @@ -291,15 +295,15 @@ function _wrapFetch(client: Client, options: HttpClientOptions): void {
return;
}

const { response, args } = handlerData;
const { response, args, error, virtualError } = handlerData;
const [requestInfo, requestInit] = args as [RequestInfo, RequestInit | undefined];

if (!response) {
return;
}

_fetchResponseHandler(options, requestInfo, response as Response, requestInit);
});
_fetchResponseHandler(options, requestInfo, response as Response, requestInit, error || virtualError);
}, false);
}

/**
Expand All @@ -315,6 +319,8 @@ function _wrapXHR(client: Client, options: HttpClientOptions): void {
return;
}

const { error, virtualError } = handlerData;

const xhr = handlerData.xhr as SentryWrappedXMLHttpRequest & XMLHttpRequest;

const sentryXhrData = xhr[SENTRY_XHR_DATA_KEY];
Expand All @@ -326,7 +332,7 @@ function _wrapXHR(client: Client, options: HttpClientOptions): void {
const { method, request_headers: headers } = sentryXhrData;

try {
_xhrResponseHandler(options, xhr, method, headers);
_xhrResponseHandler(options, xhr, method, headers, error || virtualError);
} catch (e) {
DEBUG_BUILD && logger.warn('Error while extracting response event form XHR response', e);
}
Expand Down Expand Up @@ -361,7 +367,12 @@ function _createEvent(data: {
responseCookies?: Record<string, string>;
requestHeaders?: Record<string, string>;
requestCookies?: Record<string, string>;
error?: unknown;
}): SentryEvent {
const client = getClient();
const virtualStackTrace = client && data.error && data.error instanceof Error ? data.error.stack : undefined;
// Remove the first frame from the stack as it's the HttpClient call
const stack = virtualStackTrace && client ? client.getOptions().stackParser(virtualStackTrace, 0, 1) : undefined;
const message = `HTTP Client Error with status code: ${data.status}`;

const event: SentryEvent = {
Expand All @@ -371,6 +382,7 @@ function _createEvent(data: {
{
type: 'Error',
value: message,
stacktrace: stack ? { frames: stack } : undefined,
},
],
},
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/types-hoist/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ export interface HandlerDataXhr {
xhr: SentryWrappedXMLHttpRequest;
startTimestamp?: number;
endTimestamp?: number;
error?: unknown;
// This is to be consumed by the HttpClient integration
virtualError?: unknown;
}

interface SentryFetchData {
Expand All @@ -56,6 +59,8 @@ export interface HandlerDataFetch {
headers: WebFetchHeaders;
};
error?: unknown;
// This is to be consumed by the HttpClient integration
virtualError?: unknown;
}

export interface HandlerDataDom {
Expand Down
22 changes: 12 additions & 10 deletions packages/core/src/utils-hoist/instrument/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNat

fill(GLOBAL_OBJ, 'fetch', function (originalFetch: () => void): () => void {
return function (...args: any[]): void {
// We capture the error right here and not in the Promise error callback because Safari (and probably other
// browsers too) will wipe the stack trace up to this point, only leaving us with this file which is useless.

// NOTE: If you are a Sentry user, and you are seeing this stack frame,
// it means the error, that was caused by your fetch call did not
// have a stack trace, so the SDK backfilled the stack trace so
// you can see which fetch call failed.
const virtualError = new Error();

const { method, url } = parseFetchArgs(args);
const handlerData: HandlerDataFetch = {
args,
Expand All @@ -56,6 +65,8 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNat
url,
},
startTimestamp: timestampInSeconds() * 1000,
// // Adding the error to be able to fingerprint the failed fetch event in HttpClient instrumentation
virtualError,
};

// if there is no callback, fetch is instrumented directly
Expand All @@ -65,15 +76,6 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNat
});
}

// We capture the stack right here and not in the Promise error callback because Safari (and probably other
// browsers too) will wipe the stack trace up to this point, only leaving us with this file which is useless.

// NOTE: If you are a Sentry user, and you are seeing this stack frame,
// it means the error, that was caused by your fetch call did not
// have a stack trace, so the SDK backfilled the stack trace so
// you can see which fetch call failed.
const virtualStackTrace = new Error().stack;

// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
return originalFetch.apply(GLOBAL_OBJ, args).then(
async (response: Response) => {
Expand Down Expand Up @@ -101,7 +103,7 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNat
// it means the error, that was caused by your fetch call did not
// have a stack trace, so the SDK backfilled the stack trace so
// you can see which fetch call failed.
error.stack = virtualStackTrace;
error.stack = virtualError.stack;
addNonEnumerableProperty(error, 'framesToPop', 1);
}

Expand Down

0 comments on commit 63476fa

Please sign in to comment.