fix(expect): do not fail on navigated frames while polling (#9659)

packages/playwright-core/src/server/dom.ts

@@ -505,7 +505,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       if (poll === 'error:notconnected')
         return poll;
       const pollHandler = new InjectedScriptPollHandler(progress, poll);
-      const result = await pollHandler.finish();
+      const result = await pollHandler.finishMaybeNotConnected();
       await this._page._doSlowMo();
       return result;
     });
@@ -530,7 +530,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       if (poll === 'error:notconnected')
         return poll;
       const pollHandler = new InjectedScriptPollHandler(progress, poll);
-      const filled = await pollHandler.finish();
+      const filled = await pollHandler.finishMaybeNotConnected();
       progress.throwIfAborted();  // Avoid action that has side-effects.
       if (filled === 'error:notconnected')
         return filled;
@@ -556,7 +556,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
         return injected.waitForElementStatesAndPerformAction(node, ['visible'], force, injected.selectText.bind(injected));
       }, options.force);
       const pollHandler = new InjectedScriptPollHandler(progress, throwRetargetableDOMError(poll));
-      const result = await pollHandler.finish();
+      const result = await pollHandler.finishMaybeNotConnected();
       assertDone(throwRetargetableDOMError(result));
     }, this._page._timeoutSettings.timeout(options));
   }
@@ -761,7 +761,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
         return injected.waitForElementStatesAndPerformAction(node, [state], false, () => 'done' as const);
       }, state);
       const pollHandler = new InjectedScriptPollHandler(progress, throwRetargetableDOMError(poll));
-      assertDone(throwRetargetableDOMError(await pollHandler.finish()));
+      assertDone(throwRetargetableDOMError(await pollHandler.finishMaybeNotConnected()));
     }, this._page._timeoutSettings.timeout(options));
   }
 
@@ -808,7 +808,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     if (poll === 'error:notconnected')
       return poll;
     const pollHandler = new InjectedScriptPollHandler(progress, poll);
-    const result = await pollHandler.finish();
+    const result = await pollHandler.finishMaybeNotConnected();
     if (waitForEnabled)
       progress.log('  element is visible, enabled and stable');
     else
@@ -867,7 +867,17 @@ export class InjectedScriptPollHandler<T> {
     }
   }
 
-  async finish(): Promise<T | 'error:notconnected'> {
+  async finish(): Promise<T> {
+    try {
+      const result = await this._poll!.evaluate(poll => poll.run());
+      await this._finishInternal();
+      return result;
+    } finally {
+      await this.cancel();
+    }
+  }
+
+  async finishMaybeNotConnected(): Promise<T | 'error:notconnected'> {
     try {
       const result = await this._poll!.evaluate(poll => poll.run());
       await this._finishInternal();

packages/playwright-core/src/server/frames.ts

@@ -38,7 +38,7 @@ type ContextData = {
   contextPromise: Promise<dom.FrameExecutionContext>;
   contextResolveCallback: (c: dom.FrameExecutionContext) => void;
   context: dom.FrameExecutionContext | null;
-  rerunnableTasks: Set<RerunnableTask>;
+  rerunnableTasks: Set<RerunnableTask<any>>;
 };
 
 type DocumentInfo = {
@@ -1201,6 +1201,8 @@ export class Frame extends SdkObject {
     }, options, { strict: true, querySelectorAll, mainWorld, omitAttached: true, logScale: true, ...options }).catch(e => {
       if (js.isJavaScriptErrorInEvaluate(e))
         throw e;
+      // Q: Why not throw upon isSessionClosedError(e) as in other places?
+      // A: We want user to receive a friendly message containing the last intermediate result.
       return { received: controller.lastIntermediateResult(), matches: options.isNot, log: metadata.log };
     });
   }
@@ -1280,7 +1282,7 @@ export class Frame extends SdkObject {
 
     return controller.run(async progress => {
       progress.log(`waiting for selector "${selector}"`);
-      const rerunnableTask = new RerunnableTask(data, progress, injectedScript => {
+      const rerunnableTask = new RerunnableTask<R>(data, progress, injectedScript => {
         return injectedScript.evaluateHandle((injected, { info, taskData, callbackText, querySelectorAll, logScale, omitAttached, snapshotName }) => {
           const callback = injected.eval(callbackText) as DomTaskBody<T, R, Element | undefined>;
           const poller = logScale ? injected.pollLogScale.bind(injected) : injected.pollRaf.bind(injected);
@@ -1324,18 +1326,18 @@ export class Frame extends SdkObject {
         rerunnableTask.terminate(new Error('Frame got detached.'));
       if (data.context)
         rerunnableTask.rerun(data.context);
-      return await rerunnableTask.promise;
+      return await rerunnableTask.promise!;
     }, this._page._timeoutSettings.timeout(options));
   }
 
   private _scheduleRerunnableHandleTask<T>(progress: Progress, world: types.World, task: dom.SchedulableTask<T>): Promise<js.SmartHandle<T>> {
     const data = this._contextData.get(world)!;
-    const rerunnableTask = new RerunnableTask(data, progress, task, false /* returnByValue */);
+    const rerunnableTask = new RerunnableTask<T>(data, progress, task, false /* returnByValue */);
     if (this._detached)
       rerunnableTask.terminate(new Error('waitForFunction failed: frame got detached.'));
     if (data.context)
       rerunnableTask.rerun(data.context);
-    return rerunnableTask.promise;
+    return rerunnableTask.handlePromise!;
   }
 
   private _setContext(world: types.World, context: dom.FrameExecutionContext | null) {
@@ -1394,31 +1396,42 @@ export class Frame extends SdkObject {
   }
 }
 
-class RerunnableTask {
+class RerunnableTask<T> {
-  readonly promise: Promise<any>;
+  readonly promise: ManualPromise<T> | undefined;
-  private _task: dom.SchedulableTask<any>;
+  readonly handlePromise: ManualPromise<js.SmartHandle<T>> | undefined;
-  private _resolve: (result: any) => void = () => {};
+  private _task: dom.SchedulableTask<T>;
-  private _reject: (reason: Error) => void = () => {};
   private _progress: Progress;
   private _returnByValue: boolean;
   private _contextData: ContextData;
 
-  constructor(data: ContextData, progress: Progress, task: dom.SchedulableTask<any>, returnByValue: boolean) {
+  constructor(data: ContextData, progress: Progress, task: dom.SchedulableTask<T>, returnByValue: boolean) {
     this._task = task;
     this._progress = progress;
     this._returnByValue = returnByValue;
+    if (returnByValue)
+      this.promise = new ManualPromise<T>();
+    else
+      this.handlePromise = new ManualPromise<js.SmartHandle<T>>();
     this._contextData = data;
     this._contextData.rerunnableTasks.add(this);
-    this.promise = new Promise<any>((resolve, reject) => {
-      // The task is either resolved with a value, or rejected with a meaningful evaluation error.
-      this._resolve = resolve;
-      this._reject = reject;
-    });
   }
 
   terminate(error: Error) {
     this._reject(error);
   }
+  private _resolve(value: T | js.SmartHandle<T>) {
+    if (this.promise)
+      this.promise.resolve(value as T);
+    if (this.handlePromise)
+      this.handlePromise.resolve(value as js.SmartHandle<T>);
+  }
+
+  private _reject(error: Error) {
+    if (this.promise)
+      this.promise.reject(error);
+    if (this.handlePromise)
+      this.handlePromise.reject(error);
+  }
 
   async rerun(context: dom.FrameExecutionContext) {
     try {

packages/playwright-test/src/reporters/base.ts

@@ -98,7 +98,7 @@ export class BaseReporter implements Reporter  {
   }
 
   onError(error: TestError) {
-    console.log(formatError(error, colors.enabled));
+    console.log(formatError(error, colors.enabled).message);
   }
 
   async onEnd(result: FullResult) {

tests/page/matchers.misc.spec.ts

@@ -0,0 +1,25 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { test as it, expect } from './pageTest';
+
+it('should outlive frame navigation', async ({ page, server }) => {
+  await page.goto(server.EMPTY_PAGE);
+  setTimeout(async () => {
+    await page.goto(server.PREFIX + '/grid.html').catch(() => {});
+  }, 1000);
+  await expect(page.locator('.box').first()).toBeEmpty();
+});