chore: revert noWaitForFinish option, page.close does not wait for ro… (

#28658)

…ute handlers

Reference #23781

docs/src/api/class-browsercontext.md

@@ -1202,13 +1202,6 @@ handler function to route the request.
 
 How often a route should be used. By default it will be used every time.
 
-### option: BrowserContext.route.noWaitForFinish
-* since: v1.41
-- `noWaitForFinish` <[boolean]>
-
-If set to true, [`method: BrowserContext.close`] and [`method: Page.close`] will not wait for the handler to finish and all
-errors thrown by then handler after the context has been closed are silently caught. Defaults to false.
-
 ## async method: BrowserContext.routeFromHAR
 * since: v1.23
 
@@ -1450,13 +1443,6 @@ Optional handler function used to register a routing with [`method: BrowserConte
 
 Optional handler function used to register a routing with [`method: BrowserContext.route`].
 
-### option: BrowserContext.unroute.noWaitForActive
-* since: v1.41
-- `noWaitForActive` <[boolean]>
-
-If set to true, [`method: BrowserContext.unroute`] will not wait for current handler call (if any) to finish and all
-errors thrown by the handler after unrouting are silently caught. Defaults to false.
-
 ## async method: BrowserContext.waitForCondition
 * since: v1.32
 * langs: java

docs/src/api/class-page.md

@@ -3324,13 +3324,6 @@ handler function to route the request.
 
 handler function to route the request.
 
-### option: Page.route.noWaitForFinish
-* since: v1.41
-- `noWaitForFinish` <[boolean]>
-
-If set to true, [`method: Page.close`] and [`method: BrowserContext.close`] will not wait for the handler to finish and all
-errors thrown by then handler after the page has been closed are silently caught. Defaults to false.
-
 ### option: Page.route.times
 * since: v1.15
 - `times` <[int]>
@@ -3904,13 +3897,6 @@ Optional handler function to route the request.
 
 Optional handler function to route the request.
 
-### option: Page.unroute.noWaitForActive
-* since: v1.41
-- `noWaitForActive` <[boolean]>
-
-If set to true, [`method: Page.unroute`] will not wait for current handler call (if any) to finish and all
-errors thrown by the handler after unrouting are silently caught. Defaults to false.
-
 ## method: Page.url
 * since: v1.8
 - returns: <[string]>

docs/src/api/params.md

@@ -736,7 +736,7 @@ Whether to allow sites to register Service workers. Defaults to `'allow'`.
 
 ## unroute-all-options-behavior
 * since: v1.41
-- `behavior` <[UnrouteAllBehavior]<"wait"|"ignoreErrors"|"default">>
+- `behavior` <[UnrouteBehavior]<"wait"|"ignoreErrors"|"default">>
 
 Specifies wether to wait for already running handlers and what to do if they throw errors:
 * `'default'` - do not wait for current handler calls (if any) to finish, if unrouted handler throws, it may result in unhandled error

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

@@ -310,8 +310,8 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
     this._bindings.set(name, binding);
   }
 
-  async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number, noWaitForFinish?: boolean } = {}): Promise<void> {
-    this._routes.unshift(new network.RouteHandler(this._options.baseURL, url, handler, options.times, options.noWaitForFinish));
+  async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number } = {}): Promise<void> {
+    this._routes.unshift(new network.RouteHandler(this._options.baseURL, url, handler, options.times));
     await this._updateInterceptionPatterns();
   }
 
@@ -344,11 +344,11 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
   }
 
   async unrouteAll(options?: { behavior?: 'wait'|'ignoreErrors'|'default' }): Promise<void> {
-    await this._unrouteInternal(this._routes, [], options);
+    await this._unrouteInternal(this._routes, [], options?.behavior);
     this._disposeHarRouters();
   }
 
-  async unroute(url: URLMatch, handler?: network.RouteHandlerCallback, options?: { noWaitForActive?: boolean }): Promise<void> {
+  async unroute(url: URLMatch, handler?: network.RouteHandlerCallback): Promise<void> {
     const removed = [];
     const remaining = [];
     for (const route of this._routes) {
@@ -357,16 +357,15 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
       else
         remaining.push(route);
     }
-    const behavior = options?.noWaitForActive ? 'ignoreErrors' : 'wait';
-    await this._unrouteInternal(removed, remaining, { behavior });
+    await this._unrouteInternal(removed, remaining, 'default');
   }
 
-  private async _unrouteInternal(removed: network.RouteHandler[], remaining: network.RouteHandler[], options?: { behavior?: 'wait'|'ignoreErrors'|'default' }): Promise<void> {
+  private async _unrouteInternal(removed: network.RouteHandler[], remaining: network.RouteHandler[], behavior?: 'wait'|'ignoreErrors'|'default'): Promise<void> {
     this._routes = remaining;
     await this._updateInterceptionPatterns();
-    if (!options?.behavior || options?.behavior === 'default')
+    if (!behavior || behavior === 'default')
       return;
-    const promises = removed.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(null, options?.behavior === 'ignoreErrors'));
+    const promises = removed.map(routeHandler => routeHandler.stop(behavior));
     await Promise.all(promises);
   }
 
@@ -435,7 +434,6 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
       return;
     this._closeReason = options.reason;
     this._closeWasCalled = true;
-    await this._waitForActiveRouteHandlersToFinish();
     await this._wrapApiCall(async () => {
       await this._browserType?._willCloseContext(this);
       for (const [harId, harParams] of this._harRecorders) {
@@ -457,12 +455,6 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
     await this._closedPromise;
   }
 
-  private async _waitForActiveRouteHandlersToFinish() {
-    const promises = this._routes.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(null));
-    promises.push(...[...this._pages].map(page => page._routes.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(page))).flat());
-    await Promise.all(promises);
-  }
-
   async _enableRecorder(params: {
       language: string,
       launchOptions?: LaunchOptions,

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

@@ -647,15 +647,14 @@ export class RouteHandler {
   private readonly _times: number;
   readonly url: URLMatch;
   readonly handler: RouteHandlerCallback;
-  readonly noWaitOnUnrouteOrClose: boolean;
-  private _activeInvocations: MultiMap<Page|null, { ignoreException: boolean, complete: Promise<void>, route: Route }> = new MultiMap();
+  private _ignoreException: boolean = false;
+  private _activeInvocations: Set<{ complete: Promise<void>, route: Route }> = new Set();
 
-  constructor(baseURL: string | undefined, url: URLMatch, handler: RouteHandlerCallback, times: number = Number.MAX_SAFE_INTEGER, noWaitOnUnrouteOrClose: boolean = false) {
+  constructor(baseURL: string | undefined, url: URLMatch, handler: RouteHandlerCallback, times: number = Number.MAX_SAFE_INTEGER) {
     this._baseURL = baseURL;
     this._times = times;
     this.url = url;
     this.handler = handler;
-    this.noWaitOnUnrouteOrClose = noWaitOnUnrouteOrClose;
   }
 
   static prepareInterceptionPatterns(handlers: RouteHandler[]) {
@@ -679,38 +678,32 @@ export class RouteHandler {
   }
 
   public async handle(route: Route): Promise<boolean> {
-    const page = route.request()._safePage();
-    const handlerInvocation = { ignoreException: false, complete: new ManualPromise(), route } ;
-    this._activeInvocations.set(page, handlerInvocation);
+    const handlerInvocation = { complete: new ManualPromise(), route } ;
+    this._activeInvocations.add(handlerInvocation);
     try {
       return await this._handleInternal(route);
     } catch (e) {
       // If the handler was stopped (without waiting for completion), we ignore all exceptions.
-      if (handlerInvocation.ignoreException)
+      if (this._ignoreException)
         return false;
       throw e;
     } finally {
       handlerInvocation.complete.resolve();
-      this._activeInvocations.delete(page, handlerInvocation);
+      this._activeInvocations.delete(handlerInvocation);
     }
   }
 
-  async stopAndWaitForRunningHandlers(page: Page | null, noWait?: boolean) {
+  async stop(behavior: 'wait' | 'ignoreErrors') {
     // When a handler is manually unrouted or its page/context is closed we either
     // - wait for the current handler invocations to finish
     // - or do not wait, if the user opted out of it, but swallow all exceptions
     //   that happen after the unroute/close.
-    // Note that context.route handler may be later invoked on a different page,
-    // so we only swallow errors for the current page's routes.
-    const handlerActivations = page ? this._activeInvocations.get(page) : [...this._activeInvocations.values()];
-    if (this.noWaitOnUnrouteOrClose || noWait) {
-      handlerActivations.forEach(h => h.ignoreException = true);
+    if (behavior === 'ignoreErrors') {
+      this._ignoreException = true;
     } else {
       const promises = [];
-      for (const activation of handlerActivations) {
-        if (activation.route._didThrow)
-          activation.ignoreException = true;
-        else
+      for (const activation of this._activeInvocations) {
+        if (!activation.route._didThrow)
           promises.push(activation.complete);
       }
       await Promise.all(promises);

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

@@ -458,8 +458,8 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     await this._channel.addInitScript({ source });
   }
 
-  async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number, noWaitForFinish?: boolean } = {}): Promise<void> {
-    this._routes.unshift(new RouteHandler(this._browserContext._options.baseURL, url, handler, options.times, options.noWaitForFinish));
+  async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise<void> {
+    this._routes.unshift(new RouteHandler(this._browserContext._options.baseURL, url, handler, options.times));
     await this._updateInterceptionPatterns();
   }
 
@@ -479,11 +479,11 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
   }
 
   async unrouteAll(options?: { behavior?: 'wait'|'ignoreErrors'|'default' }): Promise<void> {
-    await this._unrouteInternal(this._routes, [], options);
+    await this._unrouteInternal(this._routes, [], options?.behavior);
     this._disposeHarRouters();
   }
 
-  async unroute(url: URLMatch, handler?: RouteHandlerCallback, options?: { noWaitForActive?: boolean }): Promise<void> {
+  async unroute(url: URLMatch, handler?: RouteHandlerCallback): Promise<void> {
     const removed = [];
     const remaining = [];
     for (const route of this._routes) {
@@ -492,16 +492,15 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
       else
         remaining.push(route);
     }
-    const behavior = options?.noWaitForActive ? 'ignoreErrors' : 'wait';
-    await this._unrouteInternal(removed, remaining, { behavior });
+    await this._unrouteInternal(removed, remaining, 'default');
   }
 
-  private async _unrouteInternal(removed: RouteHandler[], remaining: RouteHandler[], options?: { behavior?: 'wait'|'ignoreErrors'|'default' }): Promise<void> {
+  private async _unrouteInternal(removed: RouteHandler[], remaining: RouteHandler[], behavior?: 'wait'|'ignoreErrors'|'default'): Promise<void> {
     this._routes = remaining;
     await this._updateInterceptionPatterns();
-    if (!options?.behavior || options?.behavior === 'default')
+    if (!behavior || behavior === 'default')
       return;
-    const promises = removed.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(this, options?.behavior === 'ignoreErrors'));
+    const promises = removed.map(routeHandler => routeHandler.stop(behavior));
     await Promise.all(promises);
   }
 
@@ -560,17 +559,9 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     await this.close();
   }
 
-  private async _waitForActiveRouteHandlersToFinish() {
-    const promises = this._routes.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(this));
-    promises.push(...this._browserContext._routes.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(this)));
-    await Promise.all(promises);
-  }
-
   async close(options: { runBeforeUnload?: boolean, reason?: string } = {}) {
     this._closeReason = options.reason;
     this._closeWasCalled = true;
-    if (!options.runBeforeUnload)
-      await this._waitForActiveRouteHandlersToFinish();
     try {
       if (this._ownedContext)
         await this._ownedContext.close();

packages/playwright-core/types/types.d.ts

@@ -3601,7 +3601,7 @@ export interface Page {
    * when request matches both handlers.
    *
    * To remove a route with its handler you can use
-   * [page.unroute(url[, handler, options])](https://playwright.dev/docs/api/class-page#page-unroute).
+   * [page.unroute(url[, handler])](https://playwright.dev/docs/api/class-page#page-unroute).
    *
    * **NOTE** Enabling routing disables http cache.
    * @param url A glob pattern, regex pattern or predicate receiving [URL] to match while routing. When a `baseURL` via the context
@@ -3611,14 +3611,6 @@ export interface Page {
    * @param options
    */
   route(url: string|RegExp|((url: URL) => boolean), handler: ((route: Route, request: Request) => Promise<any>|any), options?: {
-    /**
-     * If set to true, [page.close([options])](https://playwright.dev/docs/api/class-page#page-close) and
-     * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) will
-     * not wait for the handler to finish and all errors thrown by then handler after the page has been closed are
-     * silently caught. Defaults to false.
-     */
-    noWaitForFinish?: boolean;
-
     /**
      * How often a route should be used. By default it will be used every time.
      */
@@ -4242,16 +4234,8 @@ export interface Page {
    * specified, removes all routes for the `url`.
    * @param url A glob pattern, regex pattern or predicate receiving [URL] to match while routing.
    * @param handler Optional handler function to route the request.
-   * @param options
    */
-  unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise<any>|any), options?: {
-    /**
-     * If set to true, [page.unroute(url[, handler, options])](https://playwright.dev/docs/api/class-page#page-unroute)
-     * will not wait for current handler call (if any) to finish and all errors thrown by the handler after unrouting are
-     * silently caught. Defaults to false.
-     */
-    noWaitForActive?: boolean;
-  }): Promise<void>;
+  unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise<any>|any)): Promise<void>;
 
   /**
    * Removes all routes created with
@@ -8420,7 +8404,7 @@ export interface BrowserContext {
    * browser context routes when request matches both handlers.
    *
    * To remove a route with its handler you can use
-   * [browserContext.unroute(url[, handler, options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-unroute).
+   * [browserContext.unroute(url[, handler])](https://playwright.dev/docs/api/class-browsercontext#browser-context-unroute).
    *
    * **NOTE** Enabling routing disables http cache.
    * @param url A glob pattern, regex pattern or predicate receiving [URL] to match while routing. When a `baseURL` via the context
@@ -8430,15 +8414,6 @@ export interface BrowserContext {
    * @param options
    */
   route(url: string|RegExp|((url: URL) => boolean), handler: ((route: Route, request: Request) => Promise<any>|any), options?: {
-    /**
-     * If set to true,
-     * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) and
-     * [page.close([options])](https://playwright.dev/docs/api/class-page#page-close) will not wait for the handler to
-     * finish and all errors thrown by then handler after the context has been closed are silently caught. Defaults to
-     * false.
-     */
-    noWaitForFinish?: boolean;
-
     /**
      * How often a route should be used. By default it will be used every time.
      */
@@ -8642,17 +8617,8 @@ export interface BrowserContext {
    * [browserContext.route(url, handler[, options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-route).
    * @param handler Optional handler function used to register a routing with
    * [browserContext.route(url, handler[, options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-route).
-   * @param options
    */
-  unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise<any>|any), options?: {
-    /**
-     * If set to true,
-     * [browserContext.unroute(url[, handler, options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-unroute)
-     * will not wait for current handler call (if any) to finish and all errors thrown by the handler after unrouting are
-     * silently caught. Defaults to false.
-     */
-    noWaitForActive?: boolean;
-  }): Promise<void>;
+  unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise<any>|any)): Promise<void>;
 
   /**
    * Removes all routes created with