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

Author: mydea

packages/browser-integration-tests/package.json

@@ -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",

packages/browser-integration-tests/suites/integrations/httpclient/axios/subject.js

@@ -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);
+  });

packages/browser-integration-tests/suites/integrations/httpclient/axios/test.ts

@@ -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',
+          },
+        },
+      },
+    });
+  },
+);

packages/integrations/src/httpclient.ts

@@ -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;
@@ -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);
     });
   }
 
@@ -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);
+        }
+      },
+    );
   }
 
   /**

packages/replay/test/unit/coreHandlers/handleXhr.test.ts

@@ -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';
 
@@ -9,6 +9,7 @@ const DEFAULT_DATA: HandlerDataXhr = {
       method: 'GET',
       url: '/api/0/organizations/sentry/',
       status_code: 200,
+      request_headers: {},
     },
   } as SentryWrappedXMLHttpRequest,
   startTimestamp: 10000,
@@ -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,
         },

packages/types/src/instrument.ts

@@ -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 {

packages/utils/src/instrument.ts

@@ -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';
@@ -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(),
       };
@@ -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',
+    };
   }
-  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 {
@@ -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
@@ -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);
     };
   });