From b1a3b23c389fd87ef6713f6601cdab53f4b8853a Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 13 Mar 2020 08:54:19 -0700 Subject: [PATCH] api(request): make request.response a promise (#1377) --- docs/api.md | 2 +- src/chromium/crNetworkManager.ts | 4 ++-- src/firefox/ffNetworkManager.ts | 4 ++-- src/frames.ts | 4 ++-- src/network.ts | 22 +++++++--------------- src/webkit/wkPage.ts | 4 ++-- test/interception.spec.js | 2 +- test/network.spec.js | 8 ++++---- 8 files changed, 21 insertions(+), 29 deletions(-) diff --git a/docs/api.md b/docs/api.md index 626b40006cfc7..0ee15e445ad3a 100644 --- a/docs/api.md +++ b/docs/api.md @@ -3290,7 +3290,7 @@ Contains the request's resource type as it was perceived by the rendering engine ResourceType will be one of the following: `document`, `stylesheet`, `image`, `media`, `font`, `script`, `texttrack`, `xhr`, `fetch`, `eventsource`, `websocket`, `manifest`, `other`. #### request.response() -- returns: A matching [Response] object, or `null` if the response has not been received yet. +- returns: <[Promise] A matching [Response] object, or `null` if the response was not received due to error. #### request.url() - returns: <[string]> URL of the request. diff --git a/src/chromium/crNetworkManager.ts b/src/chromium/crNetworkManager.ts index 2bccb6a14d044..ea79d67a03d23 100644 --- a/src/chromium/crNetworkManager.ts +++ b/src/chromium/crNetworkManager.ts @@ -215,7 +215,7 @@ export class CRNetworkManager { // Under certain conditions we never get the Network.responseReceived // event from protocol. @see https://crbug.com/883475 - const response = request.request.response(); + const response = request.request._existingResponse(); if (response) response._requestFinished(); this._requestIdToRequest.delete(request._requestId); @@ -230,7 +230,7 @@ export class CRNetworkManager { // @see https://crbug.com/750469 if (!request) return; - const response = request.request.response(); + const response = request.request._existingResponse(); if (response) response._requestFinished(); this._requestIdToRequest.delete(request._requestId); diff --git a/src/firefox/ffNetworkManager.ts b/src/firefox/ffNetworkManager.ts index 654d97c47b405..6e5210ef958b7 100644 --- a/src/firefox/ffNetworkManager.ts +++ b/src/firefox/ffNetworkManager.ts @@ -90,7 +90,7 @@ export class FFNetworkManager { const request = this._requests.get(event.requestId); if (!request) return; - const response = request.request.response()!; + const response = request.request._existingResponse()!; // Keep redirected requests in the map for future reference in redirectChain. const isRedirected = response.status() >= 300 && response.status() <= 399; if (isRedirected) { @@ -107,7 +107,7 @@ export class FFNetworkManager { if (!request) return; this._requests.delete(request._id); - const response = request.request.response(); + const response = request.request._existingResponse(); if (response) response._requestFinished(); request.request._setFailureText(event.errorCode); diff --git a/src/frames.ts b/src/frames.ts index 9d5aae1a90320..c2e508ea6c192 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -387,7 +387,7 @@ export class Frame { disposer.dispose(); - return request ? request._finalRequest._waitForResponse() : null; + return request ? request._finalRequest.response() : null; function throwIfError(error: Error|void): asserts error is void { if (!error) @@ -430,7 +430,7 @@ export class Frame { if (error) throw error; - return request ? request._finalRequest._waitForResponse() : null; + return request ? request._finalRequest.response() : null; } async _waitForLoadState(options: types.NavigateOptions = {}): Promise { diff --git a/src/network.ts b/src/network.ts index 5aa30047fa3da..efbfbded8012b 100644 --- a/src/network.ts +++ b/src/network.ts @@ -108,10 +108,8 @@ export class Request { private _postData: string | null; private _headers: Headers; private _frame: frames.Frame; - private _waitForResponsePromise: Promise; - private _waitForResponsePromiseCallback: (value: Response) => void = () => {}; - private _waitForFinishedPromise: Promise; - private _waitForFinishedPromiseCallback: (value: Response | null) => void = () => {}; + private _waitForResponsePromise: Promise; + private _waitForResponsePromiseCallback: (value: Response | null) => void = () => {}; private _interceptionHandled = false; constructor(delegate: RequestDelegate | null, frame: frames.Frame, redirectChain: Request[], documentId: string | undefined, @@ -130,13 +128,12 @@ export class Request { this._postData = postData; this._headers = headers; this._waitForResponsePromise = new Promise(f => this._waitForResponsePromiseCallback = f); - this._waitForFinishedPromise = new Promise(f => this._waitForFinishedPromiseCallback = f); this._isFavicon = url.endsWith('/favicon.ico'); } _setFailureText(failureText: string) { this._failureText = failureText; - this._waitForFinishedPromiseCallback(null); + this._waitForResponsePromiseCallback(null); } url(): string { @@ -159,22 +156,17 @@ export class Request { return this._headers; } - response(): Response | null { - return this._response; - } - - async _waitForFinished(): Promise { - return this._waitForFinishedPromise; + response(): Promise { + return this._waitForResponsePromise; } - async _waitForResponse(): Promise { - return await this._waitForResponsePromise; + _existingResponse(): Response | null { + return this._response; } _setResponse(response: Response) { this._response = response; this._waitForResponsePromiseCallback(response); - response._finishedPromise.then(() => this._waitForFinishedPromiseCallback(response)); } frame(): frames.Frame { diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index 438a303fb1d0b..c73116ad8c38f 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -789,7 +789,7 @@ export class WKPage implements PageDelegate { // Under certain conditions we never get the Network.responseReceived // event from protocol. @see https://crbug.com/883475 - const response = request.request.response(); + const response = request.request._existingResponse(); if (response) response._requestFinished(); this._requestIdToRequest.delete(request._requestId); @@ -802,7 +802,7 @@ export class WKPage implements PageDelegate { // @see https://crbug.com/750469 if (!request) return; - const response = request.request.response(); + const response = request.request._existingResponse(); if (response) response._requestFinished(); this._requestIdToRequest.delete(request._requestId); diff --git a/test/interception.spec.js b/test/interception.spec.js index 906501a8b2a58..2378254c73a43 100644 --- a/test/interception.spec.js +++ b/test/interception.spec.js @@ -314,7 +314,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p const response = await page.goto(`data:text/html,`); expect(response).toBe(null); expect(requests.length).toBe(1); - expect(requests[0].response().status()).toBe(404); + expect((await requests[0].response()).status()).toBe(404); }); it('should not throw "Invalid Interception Id" if the request was cancelled', async({page, server}) => { await page.setContent(''); diff --git a/test/network.spec.js b/test/network.spec.js index 7fa4ece158586..59426d4b66ddf 100644 --- a/test/network.spec.js +++ b/test/network.spec.js @@ -132,7 +132,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM const response = await page.goto(server.PREFIX + '/foo.html'); const redirectChain = response.request().redirectChain(); expect(redirectChain.length).toBe(1); - const redirected = redirectChain[0].response(); + const redirected = await redirectChain[0].response(); expect(redirected.status()).toBe(302); let error = null; await redirected.text().catch(e => error = e); @@ -216,7 +216,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM expect(requests[0].url()).toBe(server.EMPTY_PAGE); expect(requests[0].resourceType()).toBe('document'); expect(requests[0].method()).toBe('GET'); - expect(requests[0].response()).toBeTruthy(); + expect(await requests[0].response()).toBeTruthy(); expect(requests[0].frame() === page.mainFrame()).toBe(true); expect(requests[0].frame().url()).toBe(server.EMPTY_PAGE); }); @@ -241,7 +241,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM await page.goto(server.PREFIX + '/one-style.html'); expect(failedRequests.length).toBe(1); expect(failedRequests[0].url()).toContain('one-style.css'); - expect(failedRequests[0].response()).toBe(null); + expect(await failedRequests[0].response()).toBe(null); expect(failedRequests[0].resourceType()).toBe('stylesheet'); if (CHROMIUM) { expect(failedRequests[0].failure().errorText).toBe('net::ERR_INVALID_HTTP_RESPONSE'); @@ -264,7 +264,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM ]); const request = response.request(); expect(request.url()).toBe(server.EMPTY_PAGE); - expect(request.response()).toBeTruthy(); + expect(await request.response()).toBeTruthy(); expect(request.frame() === page.mainFrame()).toBe(true); expect(request.frame().url()).toBe(server.EMPTY_PAGE); });