fix(integration): Ensure HttpClient integration works with axios

This streamlines the integration to use the general fetch/xhr instrumentation, instead of adding it's own.

This also adds `request_headers` to the xhr info (which we'll also need for replay). 

It also ensures that we always correctly get the method/url from fetch options, even when they are other valid objects.

Author: mydea

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

@@ -1,5 +1,9 @@
 import axios from 'axios';
 
-axios.get('http://localhost:7654/foo').then(response => {
-  console.log(response.data);
-});
+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

@@ -49,7 +49,6 @@ sentryTest(
         headers: {
           Accept: 'application/json',
           Cache: 'no-cache',
-          'Content-Type': 'application/json',
         },
       },
       contexts: {

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/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,55 @@ 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]);
+
+  return {
+    url: getUrlFromResource(fetchArgs[0] as FetchResource),
+    method: 'GET',
+  };
 }
-/* eslint-enable @typescript-eslint/no-unsafe-member-access */
 
 /** JSDoc */
 function instrumentXHR(): void {
@@ -220,6 +248,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 +294,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);
     };
   });

packages/utils/test/instrument.test.ts

@@ -0,0 +1,30 @@
+import { parseFetchArgs } from '../src/instrument';
+
+describe('instrument', () => {
+  describe('parseFetchArgs', () => {
+    it.each([
+      ['string URL only', ['http://example.com'], { method: 'GET', url: 'http://example.com' }],
+      ['URL object only', [new URL('http://example.com')], { method: 'GET', url: 'http://example.com/' }],
+      ['Request URL only', [{ url: 'http://example.com' }], { method: 'GET', url: 'http://example.com' }],
+      [
+        'string URL & options',
+        ['http://example.com', { method: 'post' }],
+        { method: 'POST', url: 'http://example.com' },
+      ],
+      [
+        'URL object & options',
+        [new URL('http://example.com'), { method: 'post' }],
+        { method: 'POST', url: 'http://example.com/' },
+      ],
+      [
+        'Request URL & options',
+        [{ url: 'http://example.com' }, { method: 'post' }],
+        { method: 'POST', url: 'http://example.com' },
+      ],
+    ])('%s', (_name, args, expected) => {
+      const actual = parseFetchArgs(args as unknown[]);
+
+      expect(actual).toEqual(expected);
+    });
+  });
+});