diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md index ffe0c545600bd..94225adb71fec 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.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 diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index 807f749c95c81..7860c57e08bbc 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.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]> diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 8c6023a70358e..c5aa679433e06 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -47,7 +47,7 @@ import { TargetClosedError, parseError } from './errors'; export class BrowserContext extends ChannelOwner implements api.BrowserContext { _pages = new Set(); - private _routes: network.RouteHandler[] = []; + _routes: network.RouteHandler[] = []; readonly _browser: Browser | null = null; _browserType: BrowserType | undefined; readonly _bindings = new Map any>(); @@ -62,7 +62,7 @@ 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; static from(context: channels.BrowserContextChannel): BrowserContext { @@ -193,8 +193,12 @@ export class BrowserContext extends ChannelOwner 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 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, noWaitForFinish?: boolean } = {}): Promise { + 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 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 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 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 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, diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts index d0c00b51d61de..8aefd3dce734c 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -209,6 +209,10 @@ export class Request extends ChannelOwner 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 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 }> = 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 { + 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 { ++this.handledCount; const handledPromise = route._startHandling(); // Extract handler into a variable to avoid [RouteHandler.handler] in the stack. diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index ac15fb4ffe7d6..0ed6c300b1a59 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -80,7 +80,7 @@ export class Page extends ChannelOwner 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 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 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 implements api.Page if (handled) return; } + await this._browserContext._onRoute(route); } @@ -451,8 +456,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, noWaitForFinish?: boolean } = {}): Promise { + 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 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 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 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(); diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index 31dc4798ed832..ffb24f26aae7b 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. + */ + 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)): 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. + */ + 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)): 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..8cf13254bd215 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,137 @@ 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 page.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('context.close waits for active route handlers 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 noWaitForFinish: 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(() => {}); + }, { noWaitForFinish: true }); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await routePromise; + await page.close(); + await new Promise(f => setTimeout(f, 500)); + expect(secondHandlerCalled).toBe(false); +}); + +it('page.close swallows errors in active route handlers on the owning context with noWaitForFinish: 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); + let continueRouteCallback; + const routeBarrier = new Promise(f => continueRouteCallback = f); + await context.route(/.*/, async route => { + routeCallback(); + await routeBarrier; + throw new Error('Error in route handler'); + }, { noWaitForFinish: true }); + await page.route(/.*/, async route => { + await route.fallback(); + }); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await routePromise; + await page.close(); + continueRouteCallback(); + await new Promise(f => setTimeout(f, 500)); + // The exception should be silently caught and the remaining handler called. + expect(secondHandlerCalled).toBe(false); +}); + +it('context.close does not wait for active route handlers on the owned pages with noWaitForFinish: 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(() => {}); + }, { noWaitForFinish: 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-event-request.spec.ts b/tests/page/page-event-request.spec.ts index 07c1fcc3ce5e5..cad9407512e1b 100644 --- a/tests/page/page-event-request.spec.ts +++ b/tests/page/page-event-request.spec.ts @@ -203,7 +203,7 @@ it('should fire requestfailed when intercepting race', async ({ page, server, br }); // Stall requests to make sure we don't get requestfinished. - await page.route('**', route => {}); + await page.route('**', route => {}, { noWaitForFinish: true }); await page.setContent(` diff --git a/tests/page/page-request-continue.spec.ts b/tests/page/page-request-continue.spec.ts index d36bc8575b250..c641b7ca9ed48 100644 --- a/tests/page/page-request-continue.spec.ts +++ b/tests/page/page-request-continue.spec.ts @@ -108,7 +108,7 @@ it('should not allow changing protocol when overriding url', async ({ page, serv } catch (e) { resolve(e); } - }); + }, { noWaitForFinish: true }); page.goto(server.EMPTY_PAGE).catch(() => {}); const error = await errorPromise; expect(error).toBeTruthy(); @@ -136,7 +136,7 @@ it('should not throw when continuing after page is closed', async ({ page, serve await page.route('**/*', async route => { await page.close(); done = route.continue(); - }); + }, { noWaitForFinish: true }); const error = await page.goto(server.EMPTY_PAGE).catch(e => e); await done; expect(error).toBeInstanceOf(Error); diff --git a/tests/page/page-request-intercept.spec.ts b/tests/page/page-request-intercept.spec.ts index 024919c898771..35c5418d87307 100644 --- a/tests/page/page-request-intercept.spec.ts +++ b/tests/page/page-request-intercept.spec.ts @@ -184,7 +184,7 @@ it('should intercept multipart/form-data request body', async ({ page, server, a const requestPromise = new Promise(async fulfill => { await page.route('**/upload', route => { fulfill(route.request()); - }); + }, { noWaitForFinish: true }); }); const [request] = await Promise.all([ requestPromise, @@ -219,7 +219,7 @@ it('should support timeout option in route.fetch', async ({ page, server, isElec await page.route('**/*', async route => { const error = await route.fetch({ timeout: 1000 }).catch(e => e); expect(error.message).toContain(`Request timed out after 1000ms`); - }); + }, { noWaitForFinish: true }); const error = await page.goto(server.PREFIX + '/slow', { timeout: 2000 }).catch(e => e); expect(error.message).toContain(`Timeout 2000ms exceeded`); }); diff --git a/tests/page/page-route.spec.ts b/tests/page/page-route.spec.ts index b33054b8bbebd..49546dc55a65b 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')); @@ -575,7 +635,7 @@ it('should not fulfill with redirect status', async ({ page, server, browserName } catch (e) { fulfill(e); } - }); + }, { noWaitForFinish: true }); for (status = 300; status < 310; status++) { const [, exception] = await Promise.all([ @@ -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 noWaitForFinish: 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(() => {}); + }, { noWaitForFinish: true }); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await routePromise; + await page.close(); + await new Promise(f => setTimeout(f, 500)); + expect(secondHandlerCalled).toBe(false); +});