From a19bd95fff8d2e855b5ce542ea1df55e292b688f Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 7 Dec 2023 13:19:15 -0800 Subject: [PATCH] fix: throw if fuliflled with unsupported status code --- .../src/server/chromium/crNetworkManager.ts | 8 +--- .../playwright-core/src/server/network.ts | 39 ++++++++++++---- tests/page/page-request-continue.spec.ts | 26 +++++++++++ tests/page/page-request-fulfill.spec.ts | 46 +++++++++++++++++++ tests/page/page-route.spec.ts | 25 ++++++++++ 5 files changed, 130 insertions(+), 14 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crNetworkManager.ts b/packages/playwright-core/src/server/chromium/crNetworkManager.ts index a079bd409e1b3..ac22de36344f0 100644 --- a/packages/playwright-core/src/server/chromium/crNetworkManager.ts +++ b/packages/playwright-core/src/server/chromium/crNetworkManager.ts @@ -563,18 +563,14 @@ class RouteImpl implements network.RouteDelegate { method: overrides.method, postData: overrides.postData ? overrides.postData.toString('base64') : undefined }; - // In certain cases, protocol will return error if the request was already canceled - // or the page was closed. We should tolerate these errors. - await this._session._sendMayFail('Fetch.continueRequest', this._alreadyContinuedParams); + await this._session.send('Fetch.continueRequest', this._alreadyContinuedParams); } async fulfill(response: types.NormalizedFulfillResponse) { const body = response.isBase64 ? response.body : Buffer.from(response.body).toString('base64'); const responseHeaders = splitSetCookieHeader(response.headers); - // In certain cases, protocol will return error if the request was already canceled - // or the page was closed. We should tolerate these errors. - await this._session._sendMayFail('Fetch.fulfillRequest', { + await this._session.send('Fetch.fulfillRequest', { requestId: this._interceptionId!, responseCode: response.status, responsePhrase: network.STATUS_TEXTS[String(response.status)], diff --git a/packages/playwright-core/src/server/network.ts b/packages/playwright-core/src/server/network.ts index fdf0573ac3683..e4ffb5e7134e0 100644 --- a/packages/playwright-core/src/server/network.ts +++ b/packages/playwright-core/src/server/network.ts @@ -258,7 +258,13 @@ export class Route extends SdkObject { async abort(errorCode: string = 'failed') { this._startHandling(); this._request._context.emit(BrowserContext.Events.RequestAborted, this._request); - await this._delegate.abort(errorCode); + await Promise.race([ + this._delegate.abort(errorCode), + // If the request is already cancelled by the page before we handle the route, + // we'll receive loading failed error and throw it here. + this._waitForFailure() + ]); + this._endHandling(); } @@ -286,15 +292,26 @@ export class Route extends SdkObject { const headers = [...(overrides.headers || [])]; this._maybeAddCorsHeaders(headers); this._request._context.emit(BrowserContext.Events.RequestFulfilled, this._request); - await this._delegate.fulfill({ - status: overrides.status || 200, - headers, - body, - isBase64, - }); + await Promise.race([ + this._delegate.fulfill({ + status: overrides.status || 200, + headers, + body, + isBase64, + }), + // If the request is already cancelled by the page before we handle the route, + // we'll receive loading failed error and throw it here. + this._waitForFailure() + ]); this._endHandling(); } + private async _waitForFailure() { + const response = await this._request.response(); + if (!response) + throw new Error(this._request._failureText || 'Request was cancelled'); + } + // See https://github.com/microsoft/playwright/issues/12929 private _maybeAddCorsHeaders(headers: NameValue[]) { const origin = this._request.headerValue('origin'); @@ -324,7 +341,13 @@ export class Route extends SdkObject { this._request._setOverrides(overrides); if (!overrides.isFallback) this._request._context.emit(BrowserContext.Events.RequestContinued, this._request); - await this._delegate.continue(this._request, overrides); + await Promise.race([ + this._delegate.continue(this._request, overrides), + // If the request is already cancelled by the page before we handle the route, + // we'll receive loading failed error and throw it here. + this._waitForFailure() + ]); + this._endHandling(); } diff --git a/tests/page/page-request-continue.spec.ts b/tests/page/page-request-continue.spec.ts index 56a58ec63066d..0eef6de86ccf6 100644 --- a/tests/page/page-request-continue.spec.ts +++ b/tests/page/page-request-continue.spec.ts @@ -16,6 +16,7 @@ */ import { test as it, expect } from './pageTest'; +import type { Route } from 'playwright-core'; it('should work', async ({ page, server }) => { await page.route('**/*', route => route.continue()); @@ -142,6 +143,31 @@ it('should not throw when continuing after page is closed', async ({ page, serve expect(error).toBeInstanceOf(Error); }); +it('should throw if request was cancelled by the page', async ({ page, server, browserName }) => { + let interceptCallback; + const interceptPromise = new Promise(f => interceptCallback = f); + await page.route('**/data.json', route => interceptCallback(route), { noWaitForFinish: true }); + await page.goto(server.EMPTY_PAGE); + page.evaluate(url => { + globalThis.controller = new AbortController(); + return fetch(url, { signal: globalThis.controller.signal }); + }, server.PREFIX + '/data.json').catch(() => {}); + const route = await interceptPromise; + const failurePromise = page.waitForEvent('requestfailed'); + await page.evaluate(() => globalThis.controller.abort()); + const cancelledRequest = await failurePromise; + console.log('cancelledRequest', cancelledRequest.failure()); + let error; + if (browserName === 'chromium') + error = 'net::ERR_ABORTED'; + else if (browserName === 'webkit') + error = 'cancelled'; + else if (browserName === 'firefox') + error = 'NS_BINDING_ABORTED'; + expect(cancelledRequest.failure().errorText).toBe(error); + await expect(route.continue()).rejects.toThrow(new RegExp(error)); +}); + it('should override method along with url', async ({ page, server }) => { const request = server.waitForRequest('/empty.html'); await page.route('**/foo', route => { diff --git a/tests/page/page-request-fulfill.spec.ts b/tests/page/page-request-fulfill.spec.ts index 376b03d69cbdb..f5e8342c7da63 100644 --- a/tests/page/page-request-fulfill.spec.ts +++ b/tests/page/page-request-fulfill.spec.ts @@ -18,6 +18,7 @@ import { test as base, expect } from './pageTest'; import fs from 'fs'; import type * as har from '../../packages/trace/src/har'; +import type { Route } from 'playwright-core'; const it = base.extend<{ // We access test servers at 10.0.2.2 from inside the browser on Android, @@ -77,6 +78,51 @@ it('should work with status code 422', async ({ page, server }) => { expect(await page.evaluate(() => document.body.textContent)).toBe('Yo, page!'); }); +it('should throw exception if status code is not supported', async ({ page, server, browserName }) => { + let fulfillPromiseCallback; + const fulfillPromise = new Promise(f => fulfillPromiseCallback = f); + await page.route('**/data.json', route => { + fulfillPromiseCallback(route.fulfill({ + status: 430, + body: 'Yo, page!' + }).catch(e => e)); + }); + await page.goto(server.EMPTY_PAGE); + page.evaluate(url => fetch(url), server.PREFIX + '/data.json').catch(() => {}); + const error = await fulfillPromise; + if (browserName === 'chromium') { + expect(error).toBeTruthy(); + expect(error.message).toContain(' Invalid http status code or phrase'); + } else { + expect(error).toBe(undefined); + } +}); + +it('should throw if request was cancelled by the page', async ({ page, server, browserName }) => { + let interceptCallback; + const interceptPromise = new Promise(f => interceptCallback = f); + await page.route('**/data.json', route => interceptCallback(route), { noWaitForFinish: true }); + await page.goto(server.EMPTY_PAGE); + page.evaluate(url => { + globalThis.controller = new AbortController(); + return fetch(url, { signal: globalThis.controller.signal }); + }, server.PREFIX + '/data.json').catch(() => {}); + const route = await interceptPromise; + const failurePromise = page.waitForEvent('requestfailed'); + await page.evaluate(() => globalThis.controller.abort()); + const cancelledRequest = await failurePromise; + console.log('cancelledRequest', cancelledRequest.failure()); + let error; + if (browserName === 'chromium') + error = 'net::ERR_ABORTED'; + else if (browserName === 'webkit') + error = 'cancelled'; + else if (browserName === 'firefox') + error = 'NS_BINDING_ABORTED'; + expect(cancelledRequest.failure().errorText).toBe(error); + await expect(route.fulfill({ status: 200 })).rejects.toThrow(new RegExp(error)); +}); + it('should allow mocking binary responses', async ({ page, server, browserName, headless, asset, isAndroid, mode }) => { it.skip(browserName === 'firefox' && !headless, 'Firefox headed produces a different image.'); it.skip(isAndroid); diff --git a/tests/page/page-route.spec.ts b/tests/page/page-route.spec.ts index d1757ce1bebc8..a6b97ef1bc8a6 100644 --- a/tests/page/page-route.spec.ts +++ b/tests/page/page-route.spec.ts @@ -375,6 +375,31 @@ it('should be abortable with custom error codes', async ({ page, server, browser expect(failedRequest.failure().errorText).toBe('net::ERR_INTERNET_DISCONNECTED'); }); +it('should throw if request was cancelled by the page', async ({ page, server, browserName }) => { + let interceptCallback; + const interceptPromise = new Promise(f => interceptCallback = f); + await page.route('**/data.json', route => interceptCallback(route), { noWaitForFinish: true }); + await page.goto(server.EMPTY_PAGE); + page.evaluate(url => { + globalThis.controller = new AbortController(); + return fetch(url, { signal: globalThis.controller.signal }); + }, server.PREFIX + '/data.json').catch(() => {}); + const route = await interceptPromise; + // const failurePromise = page.waitForEvent('requestfailed'); + // await page.evaluate(() => globalThis.controller.abort()); + // const cancelledRequest = await failurePromise; + // console.log('cancelledRequest', cancelledRequest.failure()); + let error; + if (browserName === 'chromium') + error = 'net::ERR_ABORTED'; + else if (browserName === 'webkit') + error = 'cancelled'; + else if (browserName === 'firefox') + error = 'NS_BINDING_ABORTED'; + // expect(cancelledRequest.failure().errorText).toBe(error); + await expect(route.abort()).rejects.toThrow(new RegExp(error)); +}); + it('should send referer', async ({ page, server }) => { await page.setExtraHTTPHeaders({ referer: 'http://google.com/'