fix(instrumentation-fetch): fetch(string, Request) silently drops r…

…equest body (#2411) * fix(instrumentation-fetch): pass request object as the only parameter * Pass arguments as-is when no span is created Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * fix: typescript not liking arguments Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
open-telemetry · Aug 14, 2021 · 5b4ca1c · 5b4ca1c
1 parent 90ea0fe
commit 5b4ca1c
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 15 deletions.
diff --git a/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/packages/opentelemetry-instrumentation-fetch/src/fetch.ts
@@ -284,23 +284,18 @@ export class FetchInstrumentation extends InstrumentationBase<
   /**
    * Patches the constructor of fetch
    */
-  private _patchConstructor(): (
-    original: (input: RequestInfo, init?: RequestInit) => Promise<Response>
-  ) => (input: RequestInfo, init?: RequestInit) => Promise<Response> {
-    return (
-      original: (input: RequestInfo, init?: RequestInit) => Promise<Response>
-    ): ((input: RequestInfo, init?: RequestInit) => Promise<Response>) => {
+  private _patchConstructor(): (original: Window['fetch']) => Window['fetch'] {
+    return original => {
       const plugin = this;
       return function patchConstructor(
-        this: (input: RequestInfo, init?: RequestInit) => Promise<Response>,
-        input: RequestInfo,
-        init?: RequestInit
+        this: Window,
+        ...args: Parameters<Window['fetch']>
       ): Promise<Response> {
-        const url = input instanceof Request ? input.url : input;
-        const options = input instanceof Request ? input : init || {};
+        const url = args[0] instanceof Request ? args[0].url : args[0];
+        const options = args[0] instanceof Request ? args[0] : args[1] || {};
         const createdSpan = plugin._createSpan(url, options);
         if (!createdSpan) {
-          return original.apply(this, [url, options]);
+          return original.apply(this, args);
         }
         const spanData = plugin._prepareSpanData(url);
 
@@ -380,7 +375,7 @@ export class FetchInstrumentation extends InstrumentationBase<
               plugin._addHeaders(options, url);
               plugin._tasksCount++;
               return original
-                .apply(this, [url, options])
+                .apply(this, options instanceof Request ? [options] : [url, options])
                 .then(
                   (onSuccess as any).bind(this, createdSpan, resolve),
                   onError.bind(this, createdSpan, reject)

diff --git a/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts b/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts
@@ -176,11 +176,16 @@ describe('fetch', () => {
         };
         response.headers = Object.assign({}, init.headers);
 
-        if (init.method === 'DELETE') {
+        if (init instanceof Request) {
+          // Passing request as 2nd argument causes missing body bug (#2411)
+          response.status = 400;
+          response.statusText = 'Bad Request (Request object as 2nd argument)';
+          reject(new window.Response(JSON.stringify(response), response));
+        } else if (init.method === 'DELETE') {
           response.status = 405;
           response.statusText = 'OK';
           resolve(new window.Response('foo', response));
-        } else if (input === url) {
+        } else if ((input instanceof Request && input.url === url) || input === url) {
           response.status = 200;
           response.statusText = 'OK';
           resolve(new window.Response(JSON.stringify(response), response));
@@ -530,6 +535,15 @@ describe('fetch', () => {
       assert.ok(typeof r.headers.get(X_B3_TRACE_ID) === 'string');
     });
 
+    it('should pass request object as first parameter to the original function (#2411)', () => {
+      const r = new Request(url);
+      return window.fetch(r).then(() => {
+        assert.ok(true);
+      }, (response: Response) => {
+        assert.fail(response.statusText);
+      });
+    });
+
     it('should NOT clear the resources', () => {
       assert.strictEqual(
         clearResourceTimingsSpy.args.length,
@@ -659,6 +673,14 @@ describe('fetch', () => {
     it('should NOT create any span', () => {
       assert.strictEqual(exportSpy.args.length, 0, "span shouldn't b exported");
     });
+    it('should pass request object as the first parameter to the original function (#2411)', () => {
+      const r = new Request(url);
+      return window.fetch(r).then(() => {
+        assert.ok(true);
+      }, (response: Response) => {
+        assert.fail(response.statusText);
+      });
+    });
   });
 
   describe('when clearTimingResources is TRUE', () => {