fix: throw if fuliflled with unsupported status code (#28539)

If request gets cancelled by the page before we fulfill it, we receive
`loadingFailed` event. In that case we'll ignore interception error if
any, otherwise the error will be propagated to the caller.

Fixes https://github.com/microsoft/playwright/issues/28490

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
+    await this._session.send('Fetch.continueRequest', this._alreadyContinuedParams);
-    // or the page was closed. We should tolerate these errors.
-    await this._session._sendMayFail('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
+    await this._session.send('Fetch.fulfillRequest', {
-    // or the page was closed. We should tolerate these errors.
-    await this._session._sendMayFail('Fetch.fulfillRequest', {
       requestId: this._interceptionId!,
       responseCode: response.status,
       responsePhrase: network.STATUS_TEXTS[String(response.status)],

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({
+    await Promise.race([
-      status: overrides.status || 200,
+      this._delegate.fulfill({
-      headers,
+        status: overrides.status || 200,
-      body,
+        headers,
-      isBase64,
+        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();
   }
 

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<Route>(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 => {

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<Error|undefined>(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<Route>(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);

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<Route>(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/'