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.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
 
@@ -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.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]>
@@ -3889,6 +3896,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

@@ -47,7 +47,7 @@ import { TargetClosedError, parseError } from './errors';
 
 export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel> implements api.BrowserContext {
   _pages = new Set<Page>();
-  private _routes: network.RouteHandler[] = [];
+  _routes: network.RouteHandler[] = [];
   readonly _browser: Browser | null = null;
   _browserType: BrowserType | undefined;
   readonly _bindings = new Map<string, (source: structs.BindingSource, ...args: any[]) => any>();
@@ -62,7 +62,7 @@ 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;
 
   static from(context: channels.BrowserContextChannel): BrowserContext {
@@ -193,8 +193,12 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
 
   async _onRoute(route: network.Route) {
     route._context = this;
+    const page = route.request()._safePage();
     const routeHandlers = this._routes.slice();
     for (const routeHandler of routeHandlers) {
+      // If the page or the context was closed we stall all requests right away.
+      if (page?._closeWasCalled || this._closeWasCalled)
+        return;
       if (!routeHandler.matches(route.request().url()))
         continue;
       const index = this._routes.indexOf(routeHandler);
@@ -303,8 +307,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, noWaitForFinish?: boolean } = {}): Promise<void> {
+    this._routes.unshift(new network.RouteHandler(this._options.baseURL, url, handler, options.times, options.noWaitForFinish));
     await this._updateInterceptionPatterns();
   }
 
@@ -330,9 +334,20 @@ 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 removed = [];
+    const remaining = [];
+    for (const route of this._routes) {
+      if (urlMatchesEqual(route.url, url) && (!handler || route.handler === handler))
+        removed.push(route);
+      else
+        remaining.push(route);
+    }
+    this._routes = remaining;
     await this._updateInterceptionPatterns();
+    const promises = removed.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(null, options?.noWaitForActive));
+    await Promise.all(promises);
+
   }
 
   private async _updateInterceptionPatterns() {
@@ -399,6 +414,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 +436,12 @@ 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

@@ -209,6 +209,10 @@ export class Request extends ChannelOwner<channels.RequestChannel> implements ap
     return frame;
   }
 
+  _safePage(): Page | null {
+    return Frame.fromNullable(this._initializer.frame)?._page || null;
+  }
+
   serviceWorker(): Worker | null {
     return this._initializer.serviceWorker ? Worker.from(this._initializer.serviceWorker) : null;
   }
@@ -275,8 +279,7 @@ export class Request extends ChannelOwner<channels.RequestChannel> implements ap
   }
 
   _targetClosedScope(): LongStandingScope {
-    const frame = Frame.fromNullable(this._initializer.frame);
-    return this.serviceWorker()?._closedScope || frame?._page?._closedOrCrashedScope || new LongStandingScope();
+    return this.serviceWorker()?._closedScope || this._safePage()?._closedOrCrashedScope || new LongStandingScope();
   }
 }
 
@@ -632,12 +635,15 @@ 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> }> = new MultiMap();
 
-  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, noWaitOnUnrouteOrClose: boolean = false) {
     this._baseURL = baseURL;
     this._times = times;
     this.url = url;
     this.handler = handler;
+    this.noWaitOnUnrouteOrClose = noWaitOnUnrouteOrClose;
   }
 
   static prepareInterceptionPatterns(handlers: RouteHandler[]) {
@@ -661,6 +667,37 @@ export class RouteHandler {
   }
 
   public async handle(route: Route): Promise<boolean> {
+    const page = route.request()._safePage();
+    const handlerInvocation = { ignoreException: false, complete: new ManualPromise() } ;
+    this._activeInvocations.set(page, 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)
+        return false;
+      throw e;
+    } finally {
+      handlerInvocation.complete.resolve();
+      this._activeInvocations.delete(page, handlerInvocation);
+    }
+  }
+
+  async stopAndWaitForRunningHandlers(page: Page | null, noWait?: boolean) {
+    // 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);
+    else
+      await Promise.all(handlerActivations.map(h => h.complete));
+  }
+
+  private async _handleInternal(route: Route): Promise<boolean> {
     ++this.handledCount;
     const handledPromise = route._startHandling();
     // Extract handler into a variable to avoid [RouteHandler.handler] in the stack.

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

@@ -80,7 +80,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
   private _closed = false;
   readonly _closedOrCrashedScope = new LongStandingScope();
   private _viewportSize: Size | null;
-  private _routes: RouteHandler[] = [];
+  _routes: RouteHandler[] = [];
 
   readonly accessibility: Accessibility;
   readonly coverage: Coverage;
@@ -94,6 +94,7 @@ 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;
 
   static from(page: channels.PageChannel): Page {
     return (page as any)._object;
@@ -175,6 +176,9 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     route._context = this.context();
     const routeHandlers = this._routes.slice();
     for (const routeHandler of routeHandlers) {
+      // If the page was closed we stall all requests right away.
+      if (this._closeWasCalled || this._browserContext._closeWasCalled)
+        return;
       if (!routeHandler.matches(route.request().url()))
         continue;
       const index = this._routes.indexOf(routeHandler);
@@ -188,6 +192,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
       if (handled)
         return;
     }
+
     await this._browserContext._onRoute(route);
   }
 
@@ -451,8 +456,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, noWaitForFinish?: boolean } = {}): Promise<void> {
+    this._routes.unshift(new RouteHandler(this._browserContext._options.baseURL, url, handler, options.times, options.noWaitForFinish));
     await this._updateInterceptionPatterns();
   }
 
@@ -465,9 +470,19 @@ 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 removed = [];
+    const remaining = [];
+    for (const route of this._routes) {
+      if (urlMatchesEqual(route.url, url) && (!handler || route.handler === handler))
+        removed.push(route);
+      else
+        remaining.push(route);
+    }
+    this._routes = remaining;
     await this._updateInterceptionPatterns();
+    const promises = removed.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(this, options?.noWaitForActive));
+    await Promise.all(promises);
   }
 
   private async _updateInterceptionPatterns() {
@@ -525,8 +540,17 @@ 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

@@ -3596,7 +3596,7 @@ export interface Page {
    * when request matches both handlers.
    *
    * To remove a route with its handler you can use
-   * [page.unroute(url[, handler])](https://playwright.dev/docs/api/class-page#page-unroute).
+   * [page.unroute(url[, handler, options])](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
@@ -3606,6 +3606,14 @@ 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.
      */
@@ -4229,8 +4237,16 @@ 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)): Promise<void>;
+  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>;
 
   url(): string;
 
@@ -8376,7 +8392,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])](https://playwright.dev/docs/api/class-browsercontext#browser-context-unroute).
+   * [browserContext.unroute(url[, handler, options])](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
@@ -8386,6 +8402,15 @@ 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.
      */
@@ -8589,8 +8614,17 @@ 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)): Promise<void>;
+  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>;
 
   /**
    * **NOTE** Only works with Chromium browser's persistent context.