diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md index 2d2ae9c623e98..98a1c7d392c30 100644 --- a/docs/src/api/class-browsercontext.md +++ b/docs/src/api/class-browsercontext.md @@ -1202,13 +1202,6 @@ handler function to route the request. How often a route should be used. By default it will be used every time. -### option: BrowserContext.route.noWaitForFinish -* since: v1.41 -- `noWaitForFinish` <[boolean]> - -If set to true, [`method: BrowserContext.close`] and [`method: Page.close`] will not wait for the handler to finish and all -errors thrown by then handler after the context has been closed are silently caught. Defaults to false. - ## async method: BrowserContext.routeFromHAR * since: v1.23 @@ -1450,13 +1443,6 @@ Optional handler function used to register a routing with [`method: BrowserConte Optional handler function used to register a routing with [`method: BrowserContext.route`]. -### option: BrowserContext.unroute.noWaitForActive -* since: v1.41 -- `noWaitForActive` <[boolean]> - -If set to true, [`method: BrowserContext.unroute`] will not wait for current handler call (if any) to finish and all -errors thrown by the handler after unrouting are silently caught. Defaults to false. - ## async method: BrowserContext.waitForCondition * since: v1.32 * langs: java diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index f942966f3be9c..635f49ebba1ee 100644 --- a/docs/src/api/class-page.md +++ b/docs/src/api/class-page.md @@ -3324,13 +3324,6 @@ handler function to route the request. handler function to route the request. -### option: Page.route.noWaitForFinish -* since: v1.41 -- `noWaitForFinish` <[boolean]> - -If set to true, [`method: Page.close`] and [`method: BrowserContext.close`] will not wait for the handler to finish and all -errors thrown by then handler after the page has been closed are silently caught. Defaults to false. - ### option: Page.route.times * since: v1.15 - `times` <[int]> @@ -3904,13 +3897,6 @@ Optional handler function to route the request. Optional handler function to route the request. -### option: Page.unroute.noWaitForActive -* since: v1.41 -- `noWaitForActive` <[boolean]> - -If set to true, [`method: Page.unroute`] will not wait for current handler call (if any) to finish and all -errors thrown by the handler after unrouting are silently caught. Defaults to false. - ## method: Page.url * since: v1.8 - returns: <[string]> diff --git a/docs/src/api/params.md b/docs/src/api/params.md index da1de83d2afd1..c616a5721ee91 100644 --- a/docs/src/api/params.md +++ b/docs/src/api/params.md @@ -736,7 +736,7 @@ Whether to allow sites to register Service workers. Defaults to `'allow'`. ## unroute-all-options-behavior * since: v1.41 -- `behavior` <[UnrouteAllBehavior]<"wait"|"ignoreErrors"|"default">> +- `behavior` <[UnrouteBehavior]<"wait"|"ignoreErrors"|"default">> Specifies wether to wait for already running handlers and what to do if they throw errors: * `'default'` - do not wait for current handler calls (if any) to finish, if unrouted handler throws, it may result in unhandled error diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 976a0a27ebf8d..6f32385ee8b97 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -310,8 +310,8 @@ export class BrowserContext extends ChannelOwner this._bindings.set(name, binding); } - 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)); + async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number } = {}): Promise { + this._routes.unshift(new network.RouteHandler(this._options.baseURL, url, handler, options.times)); await this._updateInterceptionPatterns(); } @@ -344,11 +344,11 @@ export class BrowserContext extends ChannelOwner } async unrouteAll(options?: { behavior?: 'wait'|'ignoreErrors'|'default' }): Promise { - await this._unrouteInternal(this._routes, [], options); + await this._unrouteInternal(this._routes, [], options?.behavior); this._disposeHarRouters(); } - async unroute(url: URLMatch, handler?: network.RouteHandlerCallback, options?: { noWaitForActive?: boolean }): Promise { + async unroute(url: URLMatch, handler?: network.RouteHandlerCallback): Promise { const removed = []; const remaining = []; for (const route of this._routes) { @@ -357,16 +357,15 @@ export class BrowserContext extends ChannelOwner else remaining.push(route); } - const behavior = options?.noWaitForActive ? 'ignoreErrors' : 'wait'; - await this._unrouteInternal(removed, remaining, { behavior }); + await this._unrouteInternal(removed, remaining, 'default'); } - private async _unrouteInternal(removed: network.RouteHandler[], remaining: network.RouteHandler[], options?: { behavior?: 'wait'|'ignoreErrors'|'default' }): Promise { + private async _unrouteInternal(removed: network.RouteHandler[], remaining: network.RouteHandler[], behavior?: 'wait'|'ignoreErrors'|'default'): Promise { this._routes = remaining; await this._updateInterceptionPatterns(); - if (!options?.behavior || options?.behavior === 'default') + if (!behavior || behavior === 'default') return; - const promises = removed.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(null, options?.behavior === 'ignoreErrors')); + const promises = removed.map(routeHandler => routeHandler.stop(behavior)); await Promise.all(promises); } @@ -435,7 +434,6 @@ export class BrowserContext extends ChannelOwner return; this._closeReason = options.reason; this._closeWasCalled = true; - await this._waitForActiveRouteHandlersToFinish(); await this._wrapApiCall(async () => { await this._browserType?._willCloseContext(this); for (const [harId, harParams] of this._harRecorders) { @@ -457,12 +455,6 @@ export class BrowserContext extends ChannelOwner 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 73a24034a32c6..6f35fdbc7067e 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -647,15 +647,14 @@ export class RouteHandler { private readonly _times: number; readonly url: URLMatch; readonly handler: RouteHandlerCallback; - readonly noWaitOnUnrouteOrClose: boolean; - private _activeInvocations: MultiMap, route: Route }> = new MultiMap(); + private _ignoreException: boolean = false; + private _activeInvocations: Set<{ complete: Promise, route: Route }> = new Set(); - constructor(baseURL: string | undefined, url: URLMatch, handler: RouteHandlerCallback, times: number = Number.MAX_SAFE_INTEGER, noWaitOnUnrouteOrClose: boolean = false) { + constructor(baseURL: string | undefined, url: URLMatch, handler: RouteHandlerCallback, times: number = Number.MAX_SAFE_INTEGER) { this._baseURL = baseURL; this._times = times; this.url = url; this.handler = handler; - this.noWaitOnUnrouteOrClose = noWaitOnUnrouteOrClose; } static prepareInterceptionPatterns(handlers: RouteHandler[]) { @@ -679,38 +678,32 @@ export class RouteHandler { } public async handle(route: Route): Promise { - const page = route.request()._safePage(); - const handlerInvocation = { ignoreException: false, complete: new ManualPromise(), route } ; - this._activeInvocations.set(page, handlerInvocation); + const handlerInvocation = { complete: new ManualPromise(), route } ; + this._activeInvocations.add(handlerInvocation); try { return await this._handleInternal(route); } catch (e) { // If the handler was stopped (without waiting for completion), we ignore all exceptions. - if (handlerInvocation.ignoreException) + if (this._ignoreException) return false; throw e; } finally { handlerInvocation.complete.resolve(); - this._activeInvocations.delete(page, handlerInvocation); + this._activeInvocations.delete(handlerInvocation); } } - async stopAndWaitForRunningHandlers(page: Page | null, noWait?: boolean) { + async stop(behavior: 'wait' | 'ignoreErrors') { // When a handler is manually unrouted or its page/context is closed we either // - wait for the current handler invocations to finish // - or do not wait, if the user opted out of it, but swallow all exceptions // that happen after the unroute/close. - // Note that context.route handler may be later invoked on a different page, - // so we only swallow errors for the current page's routes. - const handlerActivations = page ? this._activeInvocations.get(page) : [...this._activeInvocations.values()]; - if (this.noWaitOnUnrouteOrClose || noWait) { - handlerActivations.forEach(h => h.ignoreException = true); + if (behavior === 'ignoreErrors') { + this._ignoreException = true; } else { const promises = []; - for (const activation of handlerActivations) { - if (activation.route._didThrow) - activation.ignoreException = true; - else + for (const activation of this._activeInvocations) { + if (!activation.route._didThrow) promises.push(activation.complete); } await Promise.all(promises); diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index b03b7567c549b..570e391cb76f1 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -458,8 +458,8 @@ export class Page extends ChannelOwner implements api.Page await this._channel.addInitScript({ source }); } - 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)); + async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise { + this._routes.unshift(new RouteHandler(this._browserContext._options.baseURL, url, handler, options.times)); await this._updateInterceptionPatterns(); } @@ -479,11 +479,11 @@ export class Page extends ChannelOwner implements api.Page } async unrouteAll(options?: { behavior?: 'wait'|'ignoreErrors'|'default' }): Promise { - await this._unrouteInternal(this._routes, [], options); + await this._unrouteInternal(this._routes, [], options?.behavior); this._disposeHarRouters(); } - async unroute(url: URLMatch, handler?: RouteHandlerCallback, options?: { noWaitForActive?: boolean }): Promise { + async unroute(url: URLMatch, handler?: RouteHandlerCallback): Promise { const removed = []; const remaining = []; for (const route of this._routes) { @@ -492,16 +492,15 @@ export class Page extends ChannelOwner implements api.Page else remaining.push(route); } - const behavior = options?.noWaitForActive ? 'ignoreErrors' : 'wait'; - await this._unrouteInternal(removed, remaining, { behavior }); + await this._unrouteInternal(removed, remaining, 'default'); } - private async _unrouteInternal(removed: RouteHandler[], remaining: RouteHandler[], options?: { behavior?: 'wait'|'ignoreErrors'|'default' }): Promise { + private async _unrouteInternal(removed: RouteHandler[], remaining: RouteHandler[], behavior?: 'wait'|'ignoreErrors'|'default'): Promise { this._routes = remaining; await this._updateInterceptionPatterns(); - if (!options?.behavior || options?.behavior === 'default') + if (!behavior || behavior === 'default') return; - const promises = removed.map(routeHandler => routeHandler.stopAndWaitForRunningHandlers(this, options?.behavior === 'ignoreErrors')); + const promises = removed.map(routeHandler => routeHandler.stop(behavior)); await Promise.all(promises); } @@ -560,17 +559,9 @@ export class Page extends ChannelOwner 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 35bbe75c614c3..b0e44e476e4b1 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -3601,7 +3601,7 @@ export interface Page { * when request matches both handlers. * * To remove a route with its handler you can use - * [page.unroute(url[, handler, options])](https://playwright.dev/docs/api/class-page#page-unroute). + * [page.unroute(url[, handler])](https://playwright.dev/docs/api/class-page#page-unroute). * * **NOTE** Enabling routing disables http cache. * @param url A glob pattern, regex pattern or predicate receiving [URL] to match while routing. When a `baseURL` via the context @@ -3611,14 +3611,6 @@ export interface Page { * @param options */ route(url: string|RegExp|((url: URL) => boolean), handler: ((route: Route, request: Request) => Promise|any), options?: { - /** - * If set to true, [page.close([options])](https://playwright.dev/docs/api/class-page#page-close) and - * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) will - * not wait for the handler to finish and all errors thrown by then handler after the page has been closed are - * silently caught. Defaults to false. - */ - noWaitForFinish?: boolean; - /** * How often a route should be used. By default it will be used every time. */ @@ -4242,16 +4234,8 @@ export interface Page { * specified, removes all routes for the `url`. * @param url A glob pattern, regex pattern or predicate receiving [URL] to match while routing. * @param handler Optional handler function to route the request. - * @param options */ - unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise|any), 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; + unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise|any)): Promise; /** * Removes all routes created with @@ -8420,7 +8404,7 @@ export interface BrowserContext { * browser context routes when request matches both handlers. * * To remove a route with its handler you can use - * [browserContext.unroute(url[, handler, options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-unroute). + * [browserContext.unroute(url[, handler])](https://playwright.dev/docs/api/class-browsercontext#browser-context-unroute). * * **NOTE** Enabling routing disables http cache. * @param url A glob pattern, regex pattern or predicate receiving [URL] to match while routing. When a `baseURL` via the context @@ -8430,15 +8414,6 @@ export interface BrowserContext { * @param options */ route(url: string|RegExp|((url: URL) => boolean), handler: ((route: Route, request: Request) => Promise|any), options?: { - /** - * If set to true, - * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) and - * [page.close([options])](https://playwright.dev/docs/api/class-page#page-close) will not wait for the handler to - * finish and all errors thrown by then handler after the context has been closed are silently caught. Defaults to - * false. - */ - noWaitForFinish?: boolean; - /** * How often a route should be used. By default it will be used every time. */ @@ -8642,17 +8617,8 @@ export interface BrowserContext { * [browserContext.route(url, handler[, options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-route). * @param handler Optional handler function used to register a routing with * [browserContext.route(url, handler[, options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-route). - * @param options */ - unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise|any), 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; + unroute(url: string|RegExp|((url: URL) => boolean), handler?: ((route: Route, request: Request) => Promise|any)): Promise; /** * Removes all routes created with diff --git a/tests/library/browsercontext-route.spec.ts b/tests/library/browsercontext-route.spec.ts index 8f2cd92e91756..74c40f76778a5 100644 --- a/tests/library/browsercontext-route.spec.ts +++ b/tests/library/browsercontext-route.spec.ts @@ -79,7 +79,7 @@ it('should unroute', async ({ browser, server }) => { await context.close(); }); -it('unroute should wait for pending handlers to complete', async ({ page, context, server }) => { +it('unroute should not 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 => { @@ -98,47 +98,12 @@ it('unroute should wait for pending handlers to complete', async ({ page, contex 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); + await context.unroute(/.*/, handler); 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('unrouteAll removes all handlers', async ({ page, context, server }) => { await context.route('**/*', route => { void route.abort(); @@ -519,136 +484,32 @@ it('should fall back async', async ({ page, context, server }) => { 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('page.close should not wait for active route handlers on the owning context', 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); + await page.close(); }); -it('context.close waits for active route handlers on the owned pages by default', async ({ page, context, server }) => { +it('context.close should not wait for active route handlers on the owned pages', 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 cad9407512e1b..07c1fcc3ce5e5 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 => {}, { noWaitForFinish: true }); + await page.route('**', route => {}); await page.setContent(` diff --git a/tests/page/page-request-continue.spec.ts b/tests/page/page-request-continue.spec.ts index 57e7ce3926cc9..da9380781e69f 100644 --- a/tests/page/page-request-continue.spec.ts +++ b/tests/page/page-request-continue.spec.ts @@ -137,7 +137,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-fulfill.spec.ts b/tests/page/page-request-fulfill.spec.ts index f4401dab109e2..c76fbd2ab7e6e 100644 --- a/tests/page/page-request-fulfill.spec.ts +++ b/tests/page/page-request-fulfill.spec.ts @@ -103,7 +103,7 @@ it('should not throw if request was cancelled by the page', async ({ page, serve it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/28490' }); let interceptCallback; const interceptPromise = new Promise(f => interceptCallback = f); - await page.route('**/data.json', route => interceptCallback(route), { noWaitForFinish: true }); + await page.route('**/data.json', route => interceptCallback(route)); await page.goto(server.EMPTY_PAGE); page.evaluate(url => { globalThis.controller = new AbortController(); diff --git a/tests/page/page-request-intercept.spec.ts b/tests/page/page-request-intercept.spec.ts index 35c5418d87307..024919c898771 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 df7ab719069e8..934744148addd 100644 --- a/tests/page/page-route.spec.ts +++ b/tests/page/page-route.spec.ts @@ -72,7 +72,7 @@ it('should unroute', async ({ page, server }) => { expect(intercepted).toEqual([1]); }); -it('unroute should wait for pending handlers to complete', async ({ page, server }) => { +it('unroute should not 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 => { @@ -91,47 +91,12 @@ it('unroute should wait for pending handlers to complete', async ({ page, server 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); + await page.unroute(/.*/, handler); 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('unrouteAll removes all routes', async ({ page, server }) => { await page.route('**/*', route => { void route.abort(); @@ -451,7 +416,7 @@ it('should not throw if request was cancelled by the page', async ({ page, serve it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/28490' }); let interceptCallback; const interceptPromise = new Promise(f => interceptCallback = f); - await page.route('**/data.json', route => interceptCallback(route), { noWaitForFinish: true }); + await page.route('**/data.json', route => interceptCallback(route)); await page.goto(server.EMPTY_PAGE); page.evaluate(url => { globalThis.controller = new AbortController(); @@ -732,7 +697,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([ @@ -1160,29 +1125,7 @@ it('should intercept when postData is more than 1MB', async ({ page, server }) = 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('page.close does not wait for active route handlers', 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); @@ -1191,7 +1134,7 @@ it('page.close does not wait for active route handlers with noWaitForFinish: tru await page.route(/.*/, async route => { routeCallback(); await new Promise(() => {}); - }, { noWaitForFinish: true }); + }); page.goto(server.EMPTY_PAGE).catch(() => {}); await routePromise; await page.close();