Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(integrations): Ensure HttpClient integration works with Axios #7714

Merged
merged 4 commits into from
Apr 4, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/browser-integration-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"dependencies": {
"@babel/preset-typescript": "^7.16.7",
"@playwright/test": "^1.31.1",
"axios": "1.3.4",
"babel-loader": "^8.2.2",
"html-webpack-plugin": "^5.5.0",
"pako": "^2.1.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import axios from 'axios';

axios
.get('http://localhost:7654/foo', {
headers: { Accept: 'application/json', 'Content-Type': 'application/json', Cache: 'no-cache' },
})
.then(response => {
console.log(response.data);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { expect } from '@playwright/test';
import type { Event } from '@sentry/types';

import { sentryTest } from '../../../../utils/fixtures';
import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';

sentryTest(
'should assign request and response context from a failed 500 XHR request',
async ({ getLocalTestPath, page }) => {
const url = await getLocalTestPath({ testDir: __dirname });

await page.route('**/foo', route => {
return route.fulfill({
status: 500,
body: JSON.stringify({
error: {
message: 'Internal Server Error',
},
}),
headers: {
'Content-Type': 'text/html',
},
});
});

const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);

expect(eventData.exception?.values).toHaveLength(1);

// Not able to get the cookies from the request/response because of Playwright bug
// https://github.com/microsoft/playwright/issues/11035
expect(eventData).toMatchObject({
message: 'HTTP Client Error with status code: 500',
exception: {
values: [
{
type: 'Error',
value: 'HTTP Client Error with status code: 500',
mechanism: {
type: 'http.client',
handled: true,
},
},
],
},
request: {
url: 'http://localhost:7654/foo',
method: 'GET',
headers: {
Accept: 'application/json',
Cache: 'no-cache',
},
},
contexts: {
response: {
status_code: 500,
body_size: 45,
headers: {
'content-type': 'text/html',
'content-length': '45',
},
},
},
});
},
);
106 changes: 43 additions & 63 deletions packages/integrations/src/httpclient.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
import type { Event as SentryEvent, EventProcessor, Hub, Integration } from '@sentry/types';
import { addExceptionMechanism, fill, GLOBAL_OBJ, logger, supportsNativeFetch } from '@sentry/utils';
import type {
Event as SentryEvent,
EventProcessor,
HandlerDataFetch,
HandlerDataXhr,
Hub,
Integration,
SentryWrappedXMLHttpRequest,
} from '@sentry/types';
import {
addExceptionMechanism,
addInstrumentationHandler,
GLOBAL_OBJ,
logger,
supportsNativeFetch,
} from '@sentry/utils';

export type HttpStatusCodeRange = [number, number] | number;
export type HttpRequestTarget = string | RegExp;
Expand Down Expand Up @@ -283,25 +297,15 @@ export class HttpClient implements Integration {
return;
}

// eslint-disable-next-line @typescript-eslint/no-this-alias
const self = this;

fill(GLOBAL_OBJ, 'fetch', function (originalFetch: (...args: unknown[]) => Promise<Response>) {
return function (this: Window, ...args: unknown[]): Promise<Response> {
const [requestInfo, requestInit] = args as [RequestInfo, RequestInit | undefined];
const responsePromise: Promise<Response> = originalFetch.apply(this, args);

responsePromise
.then((response: Response) => {
self._fetchResponseHandler(requestInfo, response, requestInit);
return response;
})
.catch((error: Error) => {
throw error;
});
addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch & { response?: Response }) => {
const { response, args } = handlerData;
const [requestInfo, requestInit] = args as [RequestInfo, RequestInit | undefined];

if (!response) {
return;
}

return responsePromise;
};
this._fetchResponseHandler(requestInfo, response, requestInit);
});
}

Expand All @@ -313,52 +317,28 @@ export class HttpClient implements Integration {
return;
}

// eslint-disable-next-line @typescript-eslint/no-this-alias
const self = this;

fill(XMLHttpRequest.prototype, 'open', function (originalOpen: (...openArgs: unknown[]) => void): () => void {
return function (this: XMLHttpRequest, ...openArgs: unknown[]): void {
// eslint-disable-next-line @typescript-eslint/no-this-alias
const xhr = this;
const method = openArgs[0] as string;
const headers: Record<string, string> = {};

// Intercepting `setRequestHeader` to access the request headers of XHR instance.
// This will only work for user/library defined headers, not for the default/browser-assigned headers.
// Request cookies are also unavailable for XHR, as `Cookie` header can't be defined by `setRequestHeader`.
fill(
xhr,
'setRequestHeader',
// eslint-disable-next-line @typescript-eslint/ban-types
function (originalSetRequestHeader: (...setRequestHeaderArgs: unknown[]) => void): Function {
return function (...setRequestHeaderArgs: unknown[]): void {
const [header, value] = setRequestHeaderArgs as [string, string];

headers[header] = value;

return originalSetRequestHeader.apply(xhr, setRequestHeaderArgs);
};
},
);
addInstrumentationHandler(
'xhr',
(handlerData: HandlerDataXhr & { xhr: SentryWrappedXMLHttpRequest & XMLHttpRequest }) => {
const { xhr } = handlerData;

// eslint-disable-next-line @typescript-eslint/ban-types
fill(xhr, 'onloadend', function (original?: (...onloadendArgs: unknown[]) => void): Function {
return function (...onloadendArgs: unknown[]): void {
try {
self._xhrResponseHandler(xhr, method, headers);
} catch (e) {
__DEBUG_BUILD__ && logger.warn('Error while extracting response event form XHR response', e);
}
if (!xhr.__sentry_xhr__) {
return;
}

if (original) {
return original.apply(xhr, onloadendArgs);
}
};
});
const { method, request_headers: headers } = xhr.__sentry_xhr__;

return originalOpen.apply(this, openArgs);
};
});
if (!method) {
return;
}

try {
this._xhrResponseHandler(xhr, method, headers);
} catch (e) {
__DEBUG_BUILD__ && logger.warn('Error while extracting response event form XHR response', e);
}
},
);
}

/**
Expand Down
5 changes: 3 additions & 2 deletions packages/replay/test/unit/coreHandlers/handleXhr.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { HandlerDataXhr, SentryWrappedXMLHttpRequest } from '@sentry/types';
import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, SentryXhrData } from '@sentry/types';

import { handleXhr } from '../../../src/coreHandlers/handleXhr';

Expand All @@ -9,6 +9,7 @@ const DEFAULT_DATA: HandlerDataXhr = {
method: 'GET',
url: '/api/0/organizations/sentry/',
status_code: 200,
request_headers: {},
},
} as SentryWrappedXMLHttpRequest,
startTimestamp: 10000,
Expand Down Expand Up @@ -45,7 +46,7 @@ describe('Unit | coreHandlers | handleXhr', () => {
xhr: {
...DEFAULT_DATA.xhr,
__sentry_xhr__: {
...DEFAULT_DATA.xhr.__sentry_xhr__,
...(DEFAULT_DATA.xhr.__sentry_xhr__ as SentryXhrData),
request_body_size: 123,
response_body_size: 456,
},
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export interface SentryXhrData {
body?: XHRSendInput;
request_body_size?: number;
response_body_size?: number;
request_headers: Record<string, string>;
}

export interface HandlerDataXhr {
Expand Down
85 changes: 66 additions & 19 deletions packages/utils/src/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
WrappedFunction,
} from '@sentry/types';

import { isInstanceOf, isString } from './is';
import { isString } from './is';
import { CONSOLE_LEVELS, logger } from './logger';
import { fill } from './object';
import { getFunctionName } from './stacktrace';
Expand Down Expand Up @@ -142,11 +142,13 @@ function instrumentFetch(): void {

fill(WINDOW, 'fetch', function (originalFetch: () => void): () => void {
return function (...args: any[]): void {
const { method, url } = parseFetchArgs(args);

const handlerData: HandlerDataFetch = {
args,
fetchData: {
method: getFetchMethod(args),
url: getFetchUrl(args),
method,
url,
},
startTimestamp: Date.now(),
};
Expand Down Expand Up @@ -181,29 +183,56 @@ function instrumentFetch(): void {
});
}

/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/** Extract `method` from fetch call arguments */
function getFetchMethod(fetchArgs: any[] = []): string {
if ('Request' in WINDOW && isInstanceOf(fetchArgs[0], Request) && fetchArgs[0].method) {
return String(fetchArgs[0].method).toUpperCase();
function hasProp<T extends string>(obj: unknown, prop: T): obj is Record<string, string> {
return !!obj && typeof obj === 'object' && !!(obj as Record<string, string>)[prop];
}

type FetchResource = string | { toString(): string } | { url: string };

function getUrlFromResource(resource: FetchResource): string {
if (typeof resource === 'string') {
return resource;
}

if (!resource) {
return '';
}
if (fetchArgs[1] && fetchArgs[1].method) {
return String(fetchArgs[1].method).toUpperCase();

if (hasProp(resource, 'url')) {
return resource.url;
}

if (resource.toString) {
return resource.toString();
}
return 'GET';

return '';
}

/** Extract `url` from fetch call arguments */
function getFetchUrl(fetchArgs: any[] = []): string {
if (typeof fetchArgs[0] === 'string') {
return fetchArgs[0];
/**
* Exported only for tests.
* @hidden
* */
export function parseFetchArgs(fetchArgs: unknown[]): { method: string; url: string } {
if (fetchArgs.length === 0) {
return { method: 'GET', url: '' };
}
if ('Request' in WINDOW && isInstanceOf(fetchArgs[0], Request)) {
return fetchArgs[0].url;

if (fetchArgs.length === 2) {
const [url, options] = fetchArgs as [FetchResource, object];

return {
url: getUrlFromResource(url),
method: hasProp(options, 'method') ? String(options.method).toUpperCase() : 'GET',
Lms24 marked this conversation as resolved.
Show resolved Hide resolved
};
}
return String(fetchArgs[0]);

const arg = fetchArgs[0];
return {
url: getUrlFromResource(arg as FetchResource),
method: hasProp(arg, 'method') ? String(arg.method).toUpperCase() : 'GET',
};
}
/* eslint-enable @typescript-eslint/no-unsafe-member-access */

/** JSDoc */
function instrumentXHR(): void {
Expand All @@ -220,6 +249,7 @@ function instrumentXHR(): void {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
method: isString(args[0]) ? args[0].toUpperCase() : args[0],
url: args[1],
request_headers: {},
});

// if Sentry key appears in URL, don't capture it as a request
Expand Down Expand Up @@ -265,6 +295,23 @@ function instrumentXHR(): void {
this.addEventListener('readystatechange', onreadystatechangeHandler);
}

// Intercepting `setRequestHeader` to access the request headers of XHR instance.
// This will only work for user/library defined headers, not for the default/browser-assigned headers.
// Request cookies are also unavailable for XHR, as `Cookie` header can't be defined by `setRequestHeader`.
fill(this, 'setRequestHeader', function (original: WrappedFunction): Function {
return function (this: SentryWrappedXMLHttpRequest, ...setRequestHeaderArgs: unknown[]): void {
const [header, value] = setRequestHeaderArgs as [string, string];

const xhrInfo = this.__sentry_xhr__;

if (xhrInfo) {
xhrInfo.request_headers[header] = value;
}

return original.apply(this, setRequestHeaderArgs);
};
});

return originalOpen.apply(this, args);
};
});
Expand Down