fix(click): don't fail on stale context while clicking (#11228)

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

@@ -29,6 +29,13 @@ import * as types from './types';
 
 type SetInputFilesFiles = channels.ElementHandleSetInputFilesParams['files'];
 
+export class NonRecoverableDOMError extends Error {
+}
+
+export function isNonRecoverableDOMError(error: Error) {
+  return error instanceof NonRecoverableDOMError;
+}
+
 export class FrameExecutionContext extends js.ExecutionContext {
   readonly frame: frames.Frame;
   private _injectedScriptPromise?: Promise<js.JSHandle>;
@@ -356,13 +363,13 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       ++retry;
       if (result === 'error:notvisible') {
         if (options.force)
-          throw new Error('Element is not visible');
+          throw new NonRecoverableDOMError('Element is not visible');
         progress.log('  element is not visible');
         continue;
       }
       if (result === 'error:notinviewport') {
         if (options.force)
-          throw new Error('Element is outside of the viewport');
+          throw new NonRecoverableDOMError('Element is outside of the viewport');
         progress.log('  element is outside of the viewport');
         continue;
       }
@@ -740,7 +747,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     if (options.trial)
       return 'done';
     if (await isChecked() !== state)
-      throw new Error('Clicking the checkbox did not change its state');
+      throw new NonRecoverableDOMError('Clicking the checkbox did not change its state');
     return 'done';
   }
 

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

@@ -998,10 +998,18 @@ export class Frame extends SdkObject {
         // Always fail on JavaScript errors or when the main connection is closed.
         if (js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e))
           throw e;
-        // If error has happened in the detached inner frame, ignore it, keep polling.
-        if (selectorInFrame?.frame !== this && selectorInFrame?.frame.isDetached())
-          continue;
+        // Certain error opt-out of the retries, throw.
+        if (dom.isNonRecoverableDOMError(e))
           throw e;
+        // If the call is made on the detached frame - throw.
+        if (this.isDetached())
+          throw e;
+        // If there is scope, and scope is within the frame we use to select, assume context is destroyed and
+        // operation is not recoverable.
+        if (scope && scope._context.frame === selectorInFrame?.frame)
+          throw e;
+        // Retry upon all other errors.
+        continue;
       }
     }
     progress.throwIfAborted();

tests/page/page-click-during-navigation.spec.ts

@@ -0,0 +1,30 @@
+/**
+ * Copyright 2018 Google Inc. All rights reserved.
+ * Modifications 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 } from './pageTest';
+
+it('should not fail with internal error upon navigation', async ({ page, server }) => {
+  it.slow();
+  (async () => {
+    while (true) {
+      await page.goto(server.PREFIX + '/input/button.html').catch(() => {});
+      await page.waitForTimeout(100).catch(() => {});
+    }
+  })();
+  for (let i = 0; i < 100; ++i)
+    await page.click('button');
+});