diff --git a/packages/playwright-core/src/server/chromium/crNetworkManager.ts b/packages/playwright-core/src/server/chromium/crNetworkManager.ts index a079bd409e..ac22de3634 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 fdf0573ac3..dc2fa2ea3c 100644 --- a/packages/playwright-core/src/server/network.ts +++ b/packages/playwright-core/src/server/network.ts @@ -134,6 +134,18 @@ export class Request extends SdkObject { this._waitForResponsePromise.resolve(null); } + async _waitForRequestFailure() { + const response = await this._waitForResponsePromise; + // If response is null it was a failure an we are done. + if (!response) + return; + await response._finishedPromise; + if (this.failure()) + return; + // If request finished without errors, we stall. + await new Promise(() => {}); + } + _setOverrides(overrides: types.NormalizedContinueOverrides) { this._overrides = overrides; this._updateHeadersMap(); @@ -258,7 +270,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 event and will ignore route handling error. + this._request._waitForRequestFailure() + ]); + this._endHandling(); } @@ -286,12 +304,17 @@ 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 event and will ignore route handling error. + this._request._waitForRequestFailure() + ]); this._endHandling(); } @@ -324,7 +347,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 event and will ignore route handling error. + this._request._waitForRequestFailure() + ]); + this._endHandling(); } diff --git a/tests/page/page-request-continue.spec.ts b/tests/page/page-request-continue.spec.ts index 56a58ec630..57e7ce3926 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,24 @@ it('should not throw when continuing after page is closed', async ({ page, serve expect(error).toBeInstanceOf(Error); }); +it('should not throw if request was cancelled by the page', async ({ page, server, browserName }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/28490' }); + let interceptCallback; + const interceptPromise = new Promise(f => interceptCallback = f); + await page.route('**/data.json', route => interceptCallback(route)); + 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; + expect(cancelledRequest.failure().errorText).toMatch(/cancelled|aborted/i); + await route.continue(); // Should not throw. +}); + 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 376b03d69c..f4401dab10 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,46 @@ 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 }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/28490' }); + 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 not throw if request was cancelled by the page', async ({ page, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/28490' }); + 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; + expect(cancelledRequest.failure()).toBeTruthy(); + expect(cancelledRequest.failure().errorText).toMatch(/cancelled|aborted/i); + await route.fulfill({ status: 200 }); // Should not throw. +}); + 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 d1757ce1be..578d763914 100644 --- a/tests/page/page-route.spec.ts +++ b/tests/page/page-route.spec.ts @@ -375,6 +375,24 @@ it('should be abortable with custom error codes', async ({ page, server, browser expect(failedRequest.failure().errorText).toBe('net::ERR_INTERNET_DISCONNECTED'); }); +it('should not throw if request was cancelled by the page', async ({ page, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/28490' }); + 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; + expect(cancelledRequest.failure().errorText).toMatch(/cancelled|aborted/i); + await route.abort(); // Should not throw. +}); + it('should send referer', async ({ page, server }) => { await page.setExtraHTTPHeaders({ referer: 'http://google.com/'