chore: hint at unroute for handle errors (#30949)

examples/todomvc/tests/integration.spec.ts

@@ -54,58 +54,6 @@ test.describe('New Todo', () => {
     await checkNumberOfTodosInLocalStorage(page, 1);
   });
 
-  test('should clear text input field when an item is added 3', async ({ page }) => {
-    // create a new todo locator
-    const newTodo = page.getByPlaceholder('What needs to be done?');
-
-    // Create one todo item.
-    await newTodo.fill(TODO_ITEMS[0]);
-    await newTodo.press('Enter');
-
-    // Check that input is empty.
-    await expect(newTodo).toBeEmpty();
-    await checkNumberOfTodosInLocalStorage(page, 1);
-  });
-
-  test('should clear text input field when an item is added 4', async ({ page }) => {
-    // create a new todo locator
-    const newTodo = page.getByPlaceholder('What needs to be done?');
-
-    // Create one todo item.
-    await newTodo.fill(TODO_ITEMS[0]);
-    await newTodo.press('Enter');
-
-    // Check that input is empty.
-    await expect(newTodo).toBeEmpty();
-    await checkNumberOfTodosInLocalStorage(page, 1);
-  });
-
-  test('should clear text input field when an item is added 5', async ({ page }) => {
-    // create a new todo locator
-    const newTodo = page.getByPlaceholder('What needs to be done?');
-
-    // Create one todo item.
-    await newTodo.fill(TODO_ITEMS[0]);
-    await newTodo.press('Enter');
-
-    // Check that input is empty.
-    await expect(newTodo).toBeEmpty();
-    await checkNumberOfTodosInLocalStorage(page, 1);
-  });
-
-  test('should clear text input field when an item is added 2', async ({ page }) => {
-    // create a new todo locator
-    const newTodo = page.getByPlaceholder('What needs to be done?');
-
-    // Create one todo item.
-    await newTodo.fill(TODO_ITEMS[0]);
-    await newTodo.press('Enter');
-
-    // Check that input is empty.
-    await expect(newTodo).toBeEmpty();
-    await checkNumberOfTodosInLocalStorage(page, 1);
-  });
-
   test('should append new items to the bottom of the list', async ({ page }) => {
     // Create 3 items.
     await createDefaultTodos(page);
@@ -403,22 +351,6 @@ test.describe('Routing', () => {
     await expect(page.getByTestId('todo-item')).toHaveText([TODO_ITEMS[0], TODO_ITEMS[2]]);
   });
 
-  test('should allow me to display active items 2', async ({ page }) => {
-    await page.locator('.todo-list li .toggle').nth(1).check();
-    await checkNumberOfCompletedTodosInLocalStorage(page, 1);
-    await page.getByRole('link', { name: 'Active' }).click();
-    await expect(page.getByTestId('todo-item')).toHaveCount(2);
-    await expect(page.getByTestId('todo-item')).toHaveText([TODO_ITEMS[0], TODO_ITEMS[2]]);
-  });
-
-  test('should allow me to display active items 3', async ({ page }) => {
-    await page.locator('.todo-list li .toggle').nth(1).check();
-    await checkNumberOfCompletedTodosInLocalStorage(page, 1);
-    await page.getByRole('link', { name: 'Active' }).click();
-    await expect(page.getByTestId('todo-item')).toHaveCount(2);
-    await expect(page.getByTestId('todo-item')).toHaveText([TODO_ITEMS[0], TODO_ITEMS[2]]);
-  });
-
   test('should respect the back button', async ({ page }) => {
     await page.locator('.todo-list li .toggle').nth(1).check();
     await checkNumberOfCompletedTodosInLocalStorage(page, 1);
@@ -497,4 +429,3 @@ async function checkTodosInLocalStorage(page: Page, title: string) {
     return JSON.parse(localStorage['react-todos']).map((todo: any) => todo.title).includes(t);
   }, title);
 }
-

packages/playwright-core/src/client/fetch.ts

@@ -28,7 +28,7 @@ import { RawHeaders } from './network';
 import type { FilePayload, Headers, StorageState } from './types';
 import type { Playwright } from './playwright';
 import { Tracing } from './tracing';
-import { isTargetClosedError } from './errors';
+import { TargetClosedError, isTargetClosedError } from './errors';
 
 export type FetchOptions = {
   params?: { [key: string]: string; },
@@ -165,7 +165,7 @@ export class APIRequestContext extends ChannelOwner<channels.APIRequestContextCh
   async _innerFetch(options: FetchOptions & { url?: string, request?: api.Request } = {}): Promise<APIResponse> {
     return await this._wrapApiCall(async () => {
       if (this._closeReason)
-        throw new Error(this._closeReason);
+        throw new TargetClosedError(this._closeReason);
       assert(options.request || typeof options.url === 'string', 'First argument must be either URL string or Request');
       assert((options.data === undefined ? 0 : 1) + (options.form === undefined ? 0 : 1) + (options.multipart === undefined ? 0 : 1) <= 1, `Only one of 'data', 'form' or 'multipart' can be specified`);
       assert(options.maxRedirects === undefined || options.maxRedirects >= 0, `'maxRedirects' should be greater than or equal to '0'`);

packages/playwright-core/src/client/network.ts

@@ -22,7 +22,7 @@ import { Worker } from './worker';
 import type { Headers, RemoteAddr, SecurityDetails, WaitForEventOptions } from './types';
 import fs from 'fs';
 import { mime } from '../utilsBundle';
-import { assert, isString, headersObjectToArray, isRegExp } from '../utils';
+import { assert, isString, headersObjectToArray, isRegExp, rewriteErrorMessage } from '../utils';
 import { ManualPromise, LongStandingScope } from '../utils/manualPromise';
 import { Events } from './events';
 import type { Page } from './page';
@@ -34,6 +34,7 @@ import { MultiMap } from '../utils/multimap';
 import { APIResponse } from './fetch';
 import type { Serializable } from '../../types/structs';
 import type { BrowserContext } from './browserContext';
+import { isTargetClosedError } from './errors';
 
 export type NetworkCookie = {
   name: string,
@@ -691,6 +692,11 @@ export class RouteHandler {
       // If the handler was stopped (without waiting for completion), we ignore all exceptions.
       if (this._ignoreException)
         return false;
+      if (isTargetClosedError(e)) {
+        // We are failing in the handler because the target close closed.
+        // Give user a hint!
+        rewriteErrorMessage(e, `"${e.message}" while running route callback.\nConsider awaiting \`await page.unrouteAll({ behavior: 'ignoreErrors' })\`\nbefore the end of the test to ignore remaining routes in flight.`);
+      }
       throw e;
     } finally {
       handlerInvocation.complete.resolve();

tests/playwright-test/playwright.fetch.spec.ts

@@ -80,3 +80,30 @@ test('should stop tracing on requestContext.dispose()', async ({ runInlineTest,
   expect(result.exitCode).toBe(1);
   expect(result.failed).toBe(1);
 });
+
+test('should hint unrouteAll if failed in the handler', async ({ runInlineTest, server }) => {
+  const result = await runInlineTest({
+    'a.test.ts': `
+      import { test, expect } from '@playwright/test';
+      test('late fetch', async ({ page }) => {
+        let closedCallback = () => {};
+        const closedPromise = new Promise<void>(f => closedCallback = f);
+        await page.route('**/empty.html', async route => {
+          await route.continue();
+          await closedPromise;
+          await route.fetch();
+        });
+        await page.goto('${server.EMPTY_PAGE}');
+        closedCallback();
+      });
+
+      test('second test', async ({ page }) => {
+        // Wait enough for the worker to be killed.
+        await new Promise(f => setTimeout(f, 1000));
+      });
+    `,
+  }, { workers: 1 });
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+  expect(result.output).toContain('Consider awaiting `await page.unrouteAll({ behavior: \'ignoreErrors\' })`');
+});