Fix response body capture for fetch telemetry

src/browser/telemetry.js

@@ -383,6 +383,9 @@ Instrumenter.prototype.instrumentNetwork = function() {
         if (self.trackHttpErrors()) {
           metadata.stack = (new Error()).stack;
         }
+
+        // Start our handler before returning the promise. This allows resp.clone()
+        // to execute before other handlers touch the response.
         return orig.apply(this, args).then(function (resp) {
           metadata.end_time_ms = _.now();
           metadata.status_code = resp.status;
@@ -395,6 +398,7 @@ Instrumenter.prototype.instrumentNetwork = function() {
           if (self.autoInstrument.networkResponseBody) {
             if (typeof resp.text === 'function') { // Response.text() is not implemented on some platforms
               // The response must be cloned to prevent reading (and locking) the original stream.
+              // This must be done before other handlers touch the response.
               body = resp.clone().text(); //returns a Promise
             }
           }

test/browser.rollbar.test.js

@@ -1529,15 +1529,9 @@ describe('options.autoInstrument', function() {
 
     window.fetchStub = sinon.stub(window, 'fetch');
 
-    var readableStream = new ReadableStream({
-      start(controller) {
-        controller.enqueue(JSON.stringify({name: 'foo', password: '123456'}));
-        controller.close();
-      }
-    });
-
+    var responseBody = JSON.stringify({name: 'foo', password: '123456'});
     window.fetch.returns(Promise.resolve(new Response(
-      readableStream,
+      responseBody,
       { status: 200, statusText: 'OK', headers: { 'content-type': 'application/json', 'password': '123456' }}
     )));
 
@@ -1561,11 +1555,18 @@ describe('options.autoInstrument', function() {
     const fetchInit = {
       method: 'POST',
       headers: fetchHeaders,
-      body: JSON.stringify({name: 'bar', secret: 'xhr post'})
+      body: JSON.stringify({name: 'bar', secret: 'fetch post'})
     };
-    var fetchRequest = new Request('https://example.com/xhr-test');
+    var fetchRequest = new Request('https://example.com/fetch-test');
     window.fetch(fetchRequest, fetchInit)
     .then(function(response) {
+      // Assert that the original stream reader hasn't been read.
+      expect(response.bodyUsed).to.eql(false);
+      return response.text()
+    })
+    .then(function(text) {
+      expect(text).to.eql(responseBody);
+
       try {
         rollbar.log('test'); // generate a payload to inspect
       } catch (e) {
@@ -1577,28 +1578,22 @@ describe('options.autoInstrument', function() {
         try {
           server.respond();
 
-          expect(server.requests.length).to.eql(2);
-          var body = JSON.parse(server.requests[1].requestBody);
+          expect(window.fetchStub.called).to.be.ok();
+          expect(server.requests.length).to.eql(1);
+          var body = JSON.parse(server.requests[0].requestBody);
 
           // Verify request capture and scrubbing
           expect(body.data.body.telemetry[0].body.request).to.eql('{"name":"bar","secret":"********"}');
 
           // Verify request headers capture and case-insensitive scrubbing
           expect(body.data.body.telemetry[0].body.request_headers).to.eql({'content-type': 'application/json', secret: '********'});
 
-          // When using the Sinon test stub, the response body is populated in Headless Chrome 73,
-          // but not in 77. When using the Fetch API normally, it is populated in all tested Chrome versions.
-          // Disable here due to the Sinon limitation.
-          //
           // Verify response capture and scrubbing
-          // expect(body.data.body.telemetry[0].body.response.body).to.eql('{"name":"foo","password":"********"}');
+          expect(body.data.body.telemetry[0].body.response.body).to.eql('{"name":"foo","password":"********"}');
 
           // Verify response headers capture and case-insensitive scrubbing
           expect(body.data.body.telemetry[0].body.response.headers).to.eql({'content-type': 'application/json', password: '********'});
 
-          // Assert that the original stream reader hasn't been read.
-          expect(response.bodyUsed).to.eql(false);
-
           rollbar.configure({ autoInstrument: false });
           window.fetch.restore();
           done();