feat: wait for active route handlers on page/context close

Reference microsoft#23781

docs/src/api/class-browsercontext.md

@@ -1202,6 +1202,13 @@ handler function to route the request.
 
 How often a route should be used. By default it will be used every time.
 
+### option: BrowserContext.route.autoRemoveOnClose
+* since: v1.41
+- `autoRemoveOnClose` <[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
 
@@ -1435,6 +1442,13 @@ 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,6 +3324,13 @@ handler function to route the request.
 
 handler function to route the request.
 
+### option: Page.route.autoRemoveOnClose
+* since: v1.41
+- `autoRemoveOnClose` <[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]>
@@ -3886,6 +3893,13 @@ 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]>

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

@@ -29,7 +29,7 @@ import { Events } from './events';
 import { TimeoutSettings } from '../common/timeoutSettings';
 import { Waiter } from './waiter';
 import type { URLMatch, Headers, WaitForEventOptions, BrowserContextOptions, StorageState, LaunchOptions } from './types';
-import { headersObjectToArray, isRegExp, isString, urlMatchesEqual } from '../utils';
+import { ManualPromise, MultiMap, headersObjectToArray, isRegExp, isString, urlMatchesEqual } from '../utils';
 import { mkdirIfNeeded } from '../utils/fileUtils';
 import type * as api from '../../types/types';
 import type * as structs from '../../types/structs';
@@ -62,8 +62,9 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
   readonly _serviceWorkers = new Set<Worker>();
   readonly _isChromium: boolean;
   private _harRecorders = new Map<string, { path: string, content: 'embed' | 'attach' | 'omit' | undefined }>();
-  private _closeWasCalled = false;
+  _closeWasCalled = false;
   private _closeReason: string | undefined;
+  private _activeRouteHandlers: MultiMap<network.RouteHandler, Promise<void>> = new MultiMap();
 
   static from(context: channels.BrowserContextChannel): BrowserContext {
     return (context as any)._object;
@@ -191,22 +192,41 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
       response._finishedPromise.resolve(null);
   }
 
-  async _onRoute(route: network.Route) {
+  async _onRoute(route: network.Route, page?: Page) {
     route._context = this;
+    page ??= route.request()._safePage();
+    const closeWasCalled = () => page?._closeWasCalled || this._closeWasCalled;
     const routeHandlers = this._routes.slice();
     for (const routeHandler of routeHandlers) {
+      if (closeWasCalled())
+        return;
       if (!routeHandler.matches(route.request().url()))
         continue;
       const index = this._routes.indexOf(routeHandler);
       if (index === -1)
         continue;
       if (routeHandler.willExpire())
         this._routes.splice(index, 1);
-      const handled = await routeHandler.handle(route);
-      if (!this._routes.length)
-        this._wrapApiCall(() => this._updateInterceptionPatterns(), true).catch(() => {});
-      if (handled)
-        return;
+      const routeHandlerPromise = new ManualPromise();
+      this._activeRouteHandlers.set(routeHandler, routeHandlerPromise);
+      page?._activeRouteHandlers.set(routeHandler, routeHandlerPromise);
+      try {
+        const handled = await routeHandler.handle(route);
+        if (!this._routes.length)
+          this._wrapApiCall(() => this._updateInterceptionPatterns(), true).catch(() => {});
+        if (handled)
+          return;
+      } catch (e) {
+        if (closeWasCalled() && routeHandler.autoRemoveOnClose)
+          return;
+        // If the handler was removed without waiting for completion we ignore the excetions.
+        if (!routeHandler.swallowExceptions)
+          throw e;
+      } finally {
+        routeHandlerPromise.resolve();
+        page?._activeRouteHandlers.delete(routeHandler, routeHandlerPromise);
+        this._activeRouteHandlers.delete(routeHandler, routeHandlerPromise);
+      }
     }
     await route._innerContinue(true);
   }
@@ -303,8 +323,8 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
     this._bindings.set(name, binding);
   }
 
-  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));
+  async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number, autoRemoveOnClose?: boolean } = {}): Promise<void> {
+    this._routes.unshift(new network.RouteHandler(this._options.baseURL, url, handler, options.times, options.autoRemoveOnClose));
     await this._updateInterceptionPatterns();
   }
 
@@ -330,9 +350,21 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
     harRouter.addContextRoute(this);
   }
 
-  async unroute(url: URLMatch, handler?: network.RouteHandlerCallback): Promise<void> {
-    this._routes = this._routes.filter(route => !urlMatchesEqual(route.url, url) || (handler && route.handler !== handler));
+  async unroute(url: URLMatch, handler?: network.RouteHandlerCallback, options?: { noWaitForActive?: boolean }): Promise<void> {
+    const prediacate = (route: network.RouteHandler) => urlMatchesEqual(route.url, url) && (!handler || route.handler === handler);
+    const toBeRemoved = this._routes.filter(prediacate);
+    this._routes = this._routes.filter(route => !prediacate(route));
     await this._updateInterceptionPatterns();
+    if (options?.noWaitForActive) {
+      toBeRemoved.forEach(routeHandler => routeHandler.swallowExceptions = true);
+    } else {
+      const promises = [];
+      for (const routeHandler of toBeRemoved) {
+        if (!routeHandler.autoRemoveOnClose)
+          promises.push(...this._activeRouteHandlers.get(routeHandler));
+      }
+      await Promise.all(promises);
+    }
   }
 
   private async _updateInterceptionPatterns() {
@@ -399,6 +431,7 @@ 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) {
@@ -420,6 +453,17 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
     await this._closedPromise;
   }
 
+  private async _waitForActiveRouteHandlersToFinish() {
+    this._routes = [];
+    await this._updateInterceptionPatterns();
+    const activeHandlers = [];
+    for (const [routeHandler, promises] of this._activeRouteHandlers) {
+      if (!routeHandler.autoRemoveOnClose)
+        activeHandlers.push(...promises);
+    }
+    await Promise.all(activeHandlers);
+  }
+
   async _enableRecorder(params: {
       language: string,
       launchOptions?: LaunchOptions,

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

@@ -209,6 +209,14 @@ export class Request extends ChannelOwner<channels.RequestChannel> implements ap
     return frame;
   }
 
+  _safePage(): Page | undefined {
+    try {
+      return this.frame().page();
+    } catch (e) {
+      return undefined;
+    }
+  }
+
   serviceWorker(): Worker | null {
     return this._initializer.serviceWorker ? Worker.from(this._initializer.serviceWorker) : null;
   }
@@ -632,12 +640,15 @@ export class RouteHandler {
   private readonly _times: number;
   readonly url: URLMatch;
   readonly handler: RouteHandlerCallback;
+  readonly autoRemoveOnClose: boolean;
+  swallowExceptions: boolean = false;
 
-  constructor(baseURL: string | undefined, url: URLMatch, handler: RouteHandlerCallback, times: number = Number.MAX_SAFE_INTEGER) {
+  constructor(baseURL: string | undefined, url: URLMatch, handler: RouteHandlerCallback, times: number = Number.MAX_SAFE_INTEGER, autoRemoveOnClose: boolean = false) {
     this._baseURL = baseURL;
     this._times = times;
     this.url = url;
     this.handler = handler;
+    this.autoRemoveOnClose = autoRemoveOnClose;
   }
 
   static prepareInterceptionPatterns(handlers: RouteHandler[]) {

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

@@ -23,7 +23,7 @@ import { serializeError, isTargetClosedError, TargetClosedError } from './errors
 import { urlMatches } from '../utils/network';
 import { TimeoutSettings } from '../common/timeoutSettings';
 import type * as channels from '@protocol/channels';
-import { assert, headersObjectToArray, isObject, isRegExp, isString, LongStandingScope, urlMatchesEqual } from '../utils';
+import { assert, headersObjectToArray, isObject, isRegExp, isString, LongStandingScope, ManualPromise, MultiMap, urlMatchesEqual } from '../utils';
 import { mkdirIfNeeded } from '../utils/fileUtils';
 import { Accessibility } from './accessibility';
 import { Artifact } from './artifact';
@@ -94,6 +94,8 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
   private _video: Video | null = null;
   readonly _opener: Page | null;
   private _closeReason: string | undefined;
+  _closeWasCalled: boolean = false;
+  readonly _activeRouteHandlers: MultiMap<RouteHandler, Promise<void>> = new MultiMap();
 
   static from(page: channels.PageChannel): Page {
     return (page as any)._object;
@@ -173,21 +175,38 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
 
   private async _onRoute(route: Route) {
     route._context = this.context();
+    const closeWasCalled = () => this._closeWasCalled || this._browserContext._closeWasCalled;
     const routeHandlers = this._routes.slice();
     for (const routeHandler of routeHandlers) {
+      if (closeWasCalled())
+        return;
       if (!routeHandler.matches(route.request().url()))
         continue;
       const index = this._routes.indexOf(routeHandler);
       if (index === -1)
         continue;
       if (routeHandler.willExpire())
         this._routes.splice(index, 1);
-      const handled = await routeHandler.handle(route);
-      if (!this._routes.length)
-        this._wrapApiCall(() => this._updateInterceptionPatterns(), true).catch(() => {});
-      if (handled)
-        return;
+      const routeHandlerPromise = new ManualPromise();
+      this._activeRouteHandlers.set(routeHandler, routeHandlerPromise);
+      try {
+        const handled = await routeHandler.handle(route);
+        if (!this._routes.length)
+          this._wrapApiCall(() => this._updateInterceptionPatterns(), true).catch(() => {});
+        if (handled)
+          return;
+      } catch (e) {
+        if (closeWasCalled() && routeHandler.autoRemoveOnClose)
+          return;
+        // If the handler was removed without waiting for completion we ignore the excetions.
+        if (!routeHandler.swallowExceptions)
+          throw e;
+      } finally {
+        routeHandlerPromise.resolve();
+        this._activeRouteHandlers.delete(routeHandler, routeHandlerPromise);
+      }
     }
+
     await this._browserContext._onRoute(route);
   }
 
@@ -451,8 +470,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 } = {}): Promise<void> {
-    this._routes.unshift(new RouteHandler(this._browserContext._options.baseURL, url, handler, options.times));
+  async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number, autoRemoveOnClose?: boolean } = {}): Promise<void> {
+    this._routes.unshift(new RouteHandler(this._browserContext._options.baseURL, url, handler, options.times, options.autoRemoveOnClose));
     await this._updateInterceptionPatterns();
   }
 
@@ -465,9 +484,21 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     harRouter.addPageRoute(this);
   }
 
-  async unroute(url: URLMatch, handler?: RouteHandlerCallback): Promise<void> {
-    this._routes = this._routes.filter(route => !urlMatchesEqual(route.url, url) || (handler && route.handler !== handler));
+  async unroute(url: URLMatch, handler?: RouteHandlerCallback, options?: { noWaitForActive?: boolean }): Promise<void> {
+    const prediacate = (route: RouteHandler) => urlMatchesEqual(route.url, url) && (!handler || route.handler === handler);
+    const toBeRemoved = this._routes.filter(prediacate);
+    this._routes = this._routes.filter(route => !prediacate(route));
     await this._updateInterceptionPatterns();
+    if (options?.noWaitForActive) {
+      toBeRemoved.forEach(routeHandler => routeHandler.swallowExceptions = true);
+    } else {
+      const promises = [];
+      for (const routeHandler of toBeRemoved) {
+        if (!routeHandler.autoRemoveOnClose)
+          promises.push(...this._activeRouteHandlers.get(routeHandler));
+      }
+      await Promise.all(promises);
+    }
   }
 
   private async _updateInterceptionPatterns() {
@@ -525,8 +556,19 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     await this.close();
   }
 
+  private async _waitForActiveRouteHandlersToFinish() {
+    const activeHandlers = [];
+    for (const [routeHandler, promises] of this._activeRouteHandlers) {
+      if (!routeHandler.autoRemoveOnClose)
+        activeHandlers.push(...promises);
+    }
+    await Promise.all(activeHandlers);
+  }
+
   async close(options: { runBeforeUnload?: boolean, reason?: string } = {}) {
     this._closeReason = options.reason;
+    this._closeWasCalled = true;
+    await this._waitForActiveRouteHandlersToFinish();
     try {
       if (this._ownedContext)
         await this._ownedContext.close();