diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md index ffe0c545600bd..51cbbebfbfdc8 100644 --- a/docs/src/api/class-browsercontext.md +++ b/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 diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index e875547905777..527cf0d2d3bf9 100644 --- a/docs/src/api/class-page.md +++ b/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]> diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 8c6023a70358e..7b6752497d953 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/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 readonly _serviceWorkers = new Set(); readonly _isChromium: boolean; private _harRecorders = new Map(); - private _closeWasCalled = false; + _closeWasCalled = false; private _closeReason: string | undefined; + private _activeRouteHandlers: MultiMap> = new MultiMap(); static from(context: channels.BrowserContextChannel): BrowserContext { return (context as any)._object; @@ -191,10 +192,14 @@ export class BrowserContext extends ChannelOwner 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); @@ -202,11 +207,26 @@ export class BrowserContext extends ChannelOwner 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 this._bindings.set(name, binding); } - async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number } = {}): Promise { - 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 { + 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 harRouter.addContextRoute(this); } - async unroute(url: URLMatch, handler?: network.RouteHandlerCallback): Promise { - 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 { + 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 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 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, diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts index d0c00b51d61de..b41685c036cb5 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -209,6 +209,14 @@ export class Request extends ChannelOwner 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[]) { diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index ac15fb4ffe7d6..bcf2a98413812 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/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 implements api.Page private _video: Video | null = null; readonly _opener: Page | null; private _closeReason: string | undefined; + _closeWasCalled: boolean = false; + readonly _activeRouteHandlers: MultiMap> = new MultiMap(); static from(page: channels.PageChannel): Page { return (page as any)._object; @@ -173,8 +175,11 @@ export class Page extends ChannelOwner 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); @@ -182,12 +187,26 @@ export class Page extends ChannelOwner implements api.Page 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 implements api.Page await this._channel.addInitScript({ source }); } - async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise { - 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 { + 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 implements api.Page harRouter.addPageRoute(this); } - async unroute(url: URLMatch, handler?: RouteHandlerCallback): Promise { - this._routes = this._routes.filter(route => !urlMatchesEqual(route.url, url) || (handler && route.handler !== handler)); + async unroute(url: URLMatch, handler?: RouteHandlerCallback, options?: { noWaitForActive?: boolean }): Promise { + 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 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(); diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index 51979c06cdc1b..d5e4deb0725bc 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/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), 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. + */ + autoRemoveOnClose?: 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)): Promise; + unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise|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; 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), 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. + */ + autoRemoveOnClose?: 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)): Promise; + unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise|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; /** * **NOTE** Only works with Chromium browser's persistent context. diff --git a/tests/library/browsercontext-route.spec.ts b/tests/library/browsercontext-route.spec.ts index a597cf1157e2a..31af2a68b17f4 100644 --- a/tests/library/browsercontext-route.spec.ts +++ b/tests/library/browsercontext-route.spec.ts @@ -79,6 +79,66 @@ it('should unroute', async ({ browser, server }) => { await context.close(); }); +it('unroute should wait for pending handlers to complete', async ({ page, context, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let secondHandlerCalled = false; + await context.route(/.*/, async route => { + secondHandlerCalled = true; + await route.continue(); + }); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + let continueRouteCallback; + const routeBarrier = new Promise(f => continueRouteCallback = f); + const handler = async route => { + routeCallback(); + await routeBarrier; + await route.fallback(); + }; + await context.route(/.*/, handler); + const navigationPromise = page.goto(server.EMPTY_PAGE); + await routePromise; + let didUnroute = false; + const unroutePromise = context.unroute(/.*/, handler).then(() => didUnroute = true); + await new Promise(f => setTimeout(f, 500)); + expect(didUnroute).toBe(false); + continueRouteCallback(); + await unroutePromise; + expect(didUnroute).toBe(true); + await navigationPromise; + expect(secondHandlerCalled).toBe(true); +}); + +it('unroute should not wait for pending handlers to complete if noWaitForActive is true', async ({ page, context, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let secondHandlerCalled = false; + await context.route(/.*/, async route => { + secondHandlerCalled = true; + await route.continue(); + }); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + let continueRouteCallback; + const routeBarrier = new Promise(f => continueRouteCallback = f); + const handler = async route => { + routeCallback(); + await routeBarrier; + throw new Error('Handler error'); + }; + await context.route(/.*/, handler); + const navigationPromise = page.goto(server.EMPTY_PAGE); + await routePromise; + let didUnroute = false; + const unroutePromise = context.unroute(/.*/, handler, { noWaitForActive: true }).then(() => didUnroute = true); + await new Promise(f => setTimeout(f, 500)); + await unroutePromise; + expect(didUnroute).toBe(true); + continueRouteCallback(); + await navigationPromise.catch(e => void e); + // The error in the unrouted handler should be silently caught and remaining handler called. + expect(secondHandlerCalled).toBe(true); +}); + it('should yield to page.route', async ({ browser, server }) => { const context = await browser.newContext(); await context.route('**/empty.html', route => { @@ -387,3 +447,87 @@ it('should fall back async', async ({ page, context, server }) => { await page.goto(server.EMPTY_PAGE); expect(intercepted).toEqual([3, 2, 1]); }); + +it('page.close waits for active route handlers on the owning context by default', async ({ page, context, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + let continueRouteCallback; + const routeBarrier = new Promise(f => continueRouteCallback = f); + await context.route(/.*/, async route => { + routeCallback(); + await routeBarrier; + await route.continue(); + }); + await page.route(/.*/, async route => { + await route.fallback(); + }); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await routePromise; + let didClose = false; + const closePromise = page.close().then(() => didClose = true); + await new Promise(f => setTimeout(f, 500)); + expect(didClose).toBe(false); + continueRouteCallback(); + await closePromise; + expect(didClose).toBe(true); +}); + +it('context.close waits for active route handlers on the owned pages by default', async ({ page, context, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + let continueRouteCallback; + const routeBarrier = new Promise(f => continueRouteCallback = f); + await context.route(/.*/, async route => { + routeCallback(); + await routeBarrier; + await route.continue(); + }); + await page.route(/.*/, async route => { + await route.fallback(); + }); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await routePromise; + let didClose = false; + const closePromise = context.close().then(() => didClose = true); + await new Promise(f => setTimeout(f, 500)); + expect(didClose).toBe(false); + continueRouteCallback(); + await closePromise; + expect(didClose).toBe(true); +}); + +it('page.close does not wait for active route handlers on the owning context with autoRemoveOnClose: true', async ({ page, context, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let secondHandlerCalled = false; + await context.route(/.*/, () => secondHandlerCalled = true); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + await context.route(/.*/, async route => { + routeCallback(); + await new Promise(() => {}); + }, { autoRemoveOnClose: true }); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await routePromise; + await page.close(); + await new Promise(f => setTimeout(f, 500)); + expect(secondHandlerCalled).toBe(false); +}); + +it('context.close does not wait for active route handlers on the owned pages with autoRemoveOnClose: true', async ({ page, context, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let secondHandlerCalled = false; + await context.route(/.*/, () => secondHandlerCalled = true); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + await context.route(/.*/, async route => { + routeCallback(); + await new Promise(() => {}); + }, { autoRemoveOnClose: true }); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await routePromise; + await context.close(); + await new Promise(f => setTimeout(f, 500)); + expect(secondHandlerCalled).toBe(false); +}); diff --git a/tests/page/page-request-continue.spec.ts b/tests/page/page-request-continue.spec.ts index d36bc8575b250..6e85ccc1ee156 100644 --- a/tests/page/page-request-continue.spec.ts +++ b/tests/page/page-request-continue.spec.ts @@ -134,9 +134,11 @@ it('should not throw when continuing after page is closed', async ({ page, serve let done; await page.route('**/*', async route => { + console.log('before close'); await page.close(); + console.log('after close'); done = route.continue(); - }); + }, { autoRemoveOnClose: true }); const error = await page.goto(server.EMPTY_PAGE).catch(e => e); await done; expect(error).toBeInstanceOf(Error); diff --git a/tests/page/page-route.spec.ts b/tests/page/page-route.spec.ts index b33054b8bbebd..2f8da9fbac007 100644 --- a/tests/page/page-route.spec.ts +++ b/tests/page/page-route.spec.ts @@ -72,6 +72,66 @@ it('should unroute', async ({ page, server }) => { expect(intercepted).toEqual([1]); }); +it('unroute should wait for pending handlers to complete', async ({ page, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let secondHandlerCalled = false; + await page.route(/.*/, async route => { + secondHandlerCalled = true; + await route.continue(); + }); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + let continueRouteCallback; + const routeBarrier = new Promise(f => continueRouteCallback = f); + const handler = async route => { + routeCallback(); + await routeBarrier; + await route.fallback(); + }; + await page.route(/.*/, handler); + const navigationPromise = page.goto(server.EMPTY_PAGE); + await routePromise; + let didUnroute = false; + const unroutePromise = page.unroute(/.*/, handler).then(() => didUnroute = true); + await new Promise(f => setTimeout(f, 500)); + expect(didUnroute).toBe(false); + continueRouteCallback(); + await unroutePromise; + expect(didUnroute).toBe(true); + await navigationPromise; + expect(secondHandlerCalled).toBe(true); +}); + +it('unroute should not wait for pending handlers to complete if noWaitForActive is true', async ({ page, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let secondHandlerCalled = false; + await page.route(/.*/, async route => { + secondHandlerCalled = true; + await route.continue(); + }); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + let continueRouteCallback; + const routeBarrier = new Promise(f => continueRouteCallback = f); + const handler = async route => { + routeCallback(); + await routeBarrier; + throw new Error('Handler error'); + }; + await page.route(/.*/, handler); + const navigationPromise = page.goto(server.EMPTY_PAGE); + await routePromise; + let didUnroute = false; + const unroutePromise = page.unroute(/.*/, handler, { noWaitForActive: true }).then(() => didUnroute = true); + await new Promise(f => setTimeout(f, 500)); + await unroutePromise; + expect(didUnroute).toBe(true); + continueRouteCallback(); + await navigationPromise.catch(e => void e); + // The error in the unrouted handler should be silently caught and remaining handler called. + expect(secondHandlerCalled).toBe(true); +}); + it('should support ? in glob pattern', async ({ page, server }) => { server.setRoute('/index', (req, res) => res.end('index-no-hello')); server.setRoute('/index123hello', (req, res) => res.end('index123hello')); @@ -1002,3 +1062,42 @@ it('should intercept when postData is more than 1MB', async ({ page, server }) = }).catch(e => {}), POST_BODY); expect(await interceptionPromise).toBe(POST_BODY); }); + +it('page.close waits for active route handlers by default', async ({ page, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + let continueRouteCallback; + const routeBarrier = new Promise(f => continueRouteCallback = f); + await page.route(/.*/, async route => { + routeCallback(); + await routeBarrier; + await route.continue(); + }); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await routePromise; + let didClose = false; + const closePromise = page.close().then(() => didClose = true); + await new Promise(f => setTimeout(f, 500)); + expect(didClose).toBe(false); + continueRouteCallback(); + await closePromise; + expect(didClose).toBe(true); +}); + +it('page.close does not wait for active route handlers with autoRemoveOnClose: true', async ({ page, server }) => { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/23781' }); + let secondHandlerCalled = false; + await page.route(/.*/, () => secondHandlerCalled = true); + let routeCallback; + const routePromise = new Promise(f => routeCallback = f); + await page.route(/.*/, async route => { + routeCallback(); + await new Promise(() => {}); + }, { autoRemoveOnClose: true }); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await routePromise; + await page.close(); + await new Promise(f => setTimeout(f, 500)); + expect(secondHandlerCalled).toBe(false); +});