response.text() should wait for request to finish

This patch makes sure that request.text() doesn't try
to fetch response body from the backend until the request is
actually finished (finished or failed).

lib/NetworkManager.js

@@ -86,12 +86,12 @@ class NetworkManager extends EventEmitter {
   _onRequestWillBeSent(event) {
     if (event.redirectResponse) {
       let request = this._idToRequest.get(event.requestId);
-      let response = new Response(request, event.redirectResponse, this._getResponseBody.bind(this, event.requestId));
+      let response = new Response(this._client, request, event.redirectResponse);
       request._response = response;
       this.emit(NetworkManager.Events.Response, response);
       this.emit(NetworkManager.Events.RequestFinished, request);
     }
-    let request = new Request(event.request);
+    let request = new Request(event.requestId, event.request);
     this._idToRequest.set(event.requestId, request);
     this.emit(NetworkManager.Events.Request, request);
   }
@@ -104,7 +104,7 @@ class NetworkManager extends EventEmitter {
     // FileUpload sends a response without a matching request.
     if (!request)
       return;
-    let response = new Response(request, event.response, this._getResponseBody.bind(this, event.requestId));
+    let response = new Response(this._client, request, event.response);
     request._response = response;
     this.emit(NetworkManager.Events.Response, response);
   }
@@ -118,6 +118,7 @@ class NetworkManager extends EventEmitter {
     // @see https://github.com/GoogleChrome/puppeteer/issues/168
     if (!request)
       return;
+    request._completePromiseFulfill.call(null);
     this._idToRequest.delete(event.requestId);
     this.emit(NetworkManager.Events.RequestFinished, request);
   }
@@ -131,26 +132,22 @@ class NetworkManager extends EventEmitter {
     // @see https://github.com/GoogleChrome/puppeteer/issues/168
     if (!request)
       return;
+    request._completePromiseFulfill.call(null);
     this._idToRequest.delete(event.requestId);
     this.emit(NetworkManager.Events.RequestFailed, request);
   }
-
-  /**
-   * @param {string} requestId
-   * @return {!Promise<!Buffer>}
-   */
-  async _getResponseBody(requestId) {
-    let response = await this._client.send('Network.getResponseBody', {requestId});
-    return Buffer.from(response.body, response.base64Encoded ? 'base64' : 'utf8');
-  }
 }
 
 class Request {
   /**
    * @param {!Object} payload
    */
-  constructor(payload) {
+  constructor(requestId, payload) {
+    this._requestId = requestId;
     this._response = null;
+    this._completePromise = new Promise(fulfill => {
+      this._completePromiseFulfill = fulfill;
+    });
     this.url = payload.url;
     this.method = payload.method;
     this.postData = payload.postData;
@@ -168,14 +165,15 @@ helper.tracePublicAPI(Request);
 
 class Response {
   /**
+   * @param {!Connection} client
    * @param {?Request} request
    * @param {!Object} payload
-   * @param {function():!Promise<!Buffer>} contentCallback
    */
-  constructor(request, payload, contentCallback) {
+  constructor(client, request, payload) {
+    this._client = client;
     this._request = request;
-    this._contentCallback = contentCallback;
     this._contentPromise = null;
+
     this.headers = new Map(Object.entries(payload.headers));
     this.ok = payload.status >= 200 && payload.status <= 299;
     this.status = payload.status;
@@ -187,8 +185,14 @@ class Response {
    * @return {!Promise<!Buffer>}
    */
   buffer() {
-    if (!this._contentPromise)
-      this._contentPromise = this._contentCallback();
+    if (!this._contentPromise) {
+      this._contentPromise = this._request._completePromise.then(async() => {
+        let response = await this._client.send('Network.getResponseBody', {
+          requestId: this._request._requestId
+        });
+        return Buffer.from(response.body, response.base64Encoded ? 'base64' : 'utf8');
+      });
+    }
     return this._contentPromise;
   }
 

test/test.js

@@ -1178,6 +1178,35 @@ describe('Puppeteer', function() {
       expect(await response.text()).toBe('{"foo": "bar"}\n');
       expect(await response.json()).toEqual({foo: 'bar'});
     }));
+    it('Page.Events.Response should not report body unless request is finished', SX(async() => {
+      await page.navigate(EMPTY_PAGE);
+      // Setup server to trap request.
+      let serverResponse = null;
+      server.setRoute('/get', (req, res) => {
+        serverResponse = res;
+        res.write('hello ');
+      });
+      // Setup page to trap response.
+      let pageResponse = null;
+      let requestFinished = false;
+      page.on('response', r => pageResponse = r);
+      page.on('requestfinished', () => requestFinished = true);
+      // send request and wait for server response
+      page.evaluate(() => fetch('./get', { method: 'GET'}));
+      await waitForEvents(page, 'response');
+
+      expect(serverResponse).toBeTruthy();
+      expect(pageResponse).toBeTruthy();
+      expect(pageResponse.status).toBe(200);
+      expect(requestFinished).toBe(false);
+
+      let responseText = pageResponse.text();
+      // Write part of the response and wait for it to be flushed.
+      await new Promise(x => serverResponse.write('wor', x));
+      // Finish response.
+      await new Promise(x => serverResponse.end('ld!', x));
+      expect(await responseText).toBe('hello world!');
+    }));
     it('Page.Events.RequestFailed', SX(async function() {
       page.setRequestInterceptor(request => {
         if (request.url.endsWith('css'))