feat(navigation): waitForNavigation/goto should not wait until respon…

…se finished (#1225)
microsoft · Mar 4, 2020 · 5ff660d · 5ff660d
1 parent 3127840
commit 5ff660d
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 13 deletions.
diff --git a/docs/api.md b/docs/api.md
@@ -3185,6 +3185,7 @@ ResourceType will be one of the following: `document`, `stylesheet`, `image`, `m
 
 <!-- GEN:toc -->
 - [response.buffer()](#responsebuffer)
+- [response.finished()](#responsefinished)
 - [response.frame()](#responseframe)
 - [response.headers()](#responseheaders)
 - [response.json()](#responsejson)
@@ -3199,6 +3200,9 @@ ResourceType will be one of the following: `document`, `stylesheet`, `image`, `m
 #### response.buffer()
 - returns: <Promise<[Buffer]>> Promise which resolves to a buffer with response body.
 
+#### response.finished()
+- returns: <Promise[?string]> Waits for this response to finish, throws when corresponding request failed.
+
 #### response.frame()
 - returns: <?[Frame]> A [Frame] that initiated this response, or `null` if navigating to error pages.
 

diff --git a/src/network.ts b/src/network.ts
@@ -165,9 +165,7 @@ export class Request {
   }
 
   async _waitForResponse(): Promise<Response> {
-    const response = await this._waitForResponsePromise;
-    await response._finishedPromise;
-    return response;
+    return await this._waitForResponsePromise;
   }
 
   _setResponse(response: Response) {
@@ -271,6 +269,10 @@ export class Response {
     return this._headers;
   }
 
+  finished(): Promise<Error | null> {
+    return this._finishedPromise;
+  }
+
   buffer(): Promise<platform.BufferType> {
     if (!this._contentPromise) {
       this._contentPromise = this._finishedPromise.then(async error => {

diff --git a/test/network.spec.js b/test/network.spec.js
@@ -258,21 +258,23 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM
       expect(failedRequests[0].frame()).toBeTruthy();
     });
     it('Page.Events.RequestFinished', async({page, server}) => {
-      const requests = [];
-      page.on('requestfinished', request => requests.push(request));
-      await page.goto(server.EMPTY_PAGE);
-      expect(requests.length).toBe(1);
-      expect(requests[0].url()).toBe(server.EMPTY_PAGE);
-      expect(requests[0].response()).toBeTruthy();
-      expect(requests[0].frame() === page.mainFrame()).toBe(true);
-      expect(requests[0].frame().url()).toBe(server.EMPTY_PAGE);
+      const [response] = await Promise.all([
+        page.goto(server.EMPTY_PAGE),
+        page.waitForEvent('requestfinished')
+      ]);
+      const request = response.request();
+      expect(request.url()).toBe(server.EMPTY_PAGE);
+      expect(request.response()).toBeTruthy();
+      expect(request.frame() === page.mainFrame()).toBe(true);
+      expect(request.frame().url()).toBe(server.EMPTY_PAGE);
     });
     it('should fire events in proper order', async({page, server}) => {
       const events = [];
       page.on('request', request => events.push('request'));
       page.on('response', response => events.push('response'));
-      page.on('requestfinished', request => events.push('requestfinished'));
-      await page.goto(server.EMPTY_PAGE);
+      const response = await page.goto(server.EMPTY_PAGE);
+      await response.finished();
+      events.push('requestfinished')
       expect(events).toEqual(['request', 'response', 'requestfinished']);
     });
     it('should support redirects', async({page, server}) => {
@@ -284,6 +286,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM
       server.setRedirect('/foo.html', '/empty.html');
       const FOO_URL = server.PREFIX + '/foo.html';
       const response = await page.goto(FOO_URL);
+      await response.finished();
       expect(events).toEqual([
         `GET ${FOO_URL}`,
         `302 ${FOO_URL}`,