chore: use isNonRetriableError in more places (#36373)

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

@@ -17,10 +17,9 @@
 import fs from 'fs';
 
 import * as js from './javascript';
-import { isAbortError, ProgressController } from './progress';
+import { ProgressController } from './progress';
 import { asLocator, isUnderTest } from '../utils';
 import { prepareFilesForUpload } from './fileUploadUtils';
-import { isSessionClosedError } from './protocolError';
 import * as rawInjectedScriptSource from '../generated/injectedScriptSource';
 
 import type * as frames from './frames';
@@ -141,7 +140,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       const utility = await this._frame._utilityContext();
       return await utility.evaluate(pageFunction, [await utility.injectedScript(), this, arg]);
     } catch (e) {
-      if (isAbortError(e) || js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e))
+      if (this._frame.isNonRetriableError(e))
         throw e;
       return 'error:notconnected';
     }
@@ -152,7 +151,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       const utility = await this._frame._utilityContext();
       return await utility.evaluateHandle(pageFunction, [await utility.injectedScript(), this, arg]);
     } catch (e) {
-      if (isAbortError(e) || js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e))
+      if (this._frame.isNonRetriableError(e))
         throw e;
       return 'error:notconnected';
     }

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

@@ -864,7 +864,7 @@ export class Frame extends SdkObject {
         return retVal;
       });
     } catch (e) {
-      if (isAbortError(e) || js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e))
+      if (this.isNonRetriableError(e))
         throw e;
       throw new Error(`Unable to retrieve content because the page is navigating and changing the content.`);
     }
@@ -1060,14 +1060,14 @@ export class Frame extends SdkObject {
           continue;
         return result as R;
       } catch (e) {
-        if (this._isErrorThatCannotBeRetried(e))
+        if (this.isNonRetriableError(e))
           throw e;
         continue;
       }
     }
   }
 
-  private _isErrorThatCannotBeRetried(e: Error) {
+  isNonRetriableError(e: Error) {
     if (isAbortError(e))
       return true;
     // Always fail on JavaScript errors or when the main connection is closed.
@@ -1288,7 +1288,7 @@ export class Frame extends SdkObject {
         return state.matches;
       }, { info: resolved.info, root: resolved.frame === this ? scope : undefined }));
     } catch (e) {
-      if (isAbortError(e) || js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e) || isSessionClosedError(e))
+      if (this.isNonRetriableError(e))
         throw e;
       return false;
     }
@@ -1408,7 +1408,7 @@ export class Frame extends SdkObject {
         if (resultOneShot.matches !== options.isNot)
           return resultOneShot;
       } catch (e) {
-        if (isAbortError(e) || js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e))
+        if (this.isNonRetriableError(e))
           throw e;
         // Ignore any other errors from one-shot, we'll handle them during retries.
       }
@@ -1434,7 +1434,7 @@ export class Frame extends SdkObject {
         });
       }, timeout);
     } catch (e) {
-      // Q: Why not throw upon isSessionClosedError(e) as in other places?
+      // Q: Why not throw upon isNonRetriableError(e) as in other places?
       // A: We want user to receive a friendly message containing the last intermediate result.
       if (js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e))
         throw e;

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

@@ -25,7 +25,7 @@ import { helper } from './helper';
 import * as input from './input';
 import { SdkObject } from './instrumentation';
 import * as js from './javascript';
-import { isAbortError, ProgressController } from './progress';
+import { ProgressController } from './progress';
 import { Screenshotter, validateScreenshotOptions } from './screenshotter';
 import { LongStandingScope, assert, renderTitleForCall, trimStringWithEllipsis } from '../utils';
 import { asLocator } from '../utils';
@@ -36,7 +36,6 @@ import { ManualPromise } from '../utils/isomorphic/manualPromise';
 import { parseEvaluationResultValue } from '../utils/isomorphic/utilityScriptSerializers';
 import { compressCallLog } from './callLog';
 import * as rawBindingsControllerSource from '../generated/bindingsControllerSource';
-import { isSessionClosedError } from './protocolError';
 
 import type { Artifact } from './artifact';
 import type * as dom from './dom';
@@ -643,7 +642,7 @@ export class Page extends SdkObject {
           progress.log(`waiting ${screenshotTimeout}ms before taking screenshot`);
         previous = actual;
         actual = await rafrafScreenshot(progress, screenshotTimeout).catch(e => {
-          if (isAbortError(e))
+          if (this.mainFrame().isNonRetriableError(e))
             throw e;
           progress.log(`failed to take screenshot - ` + e.message);
           return undefined;
@@ -676,7 +675,7 @@ export class Page extends SdkObject {
       }
       throw new Error(intermediateResult!.errorMessage);
     }, callTimeout).catch(e => {
-      // Q: Why not throw upon isSessionClosedError(e) as in other places?
+      // Q: Why not throw upon isNonRetriableError(e) as in other places?
       // A: We want user to receive a friendly diff between actual and expected/previous.
       if (js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e))
         throw e;
@@ -1010,7 +1009,7 @@ async function snapshotFrameForAI(progress: Progress, frame: frames.Frame, frame
         return continuePolling;
       return snapshotOrRetry;
     } catch (e) {
-      if (isAbortError(e) || isSessionClosedError(e) || js.isJavaScriptErrorInEvaluate(e))
+      if (frame.isNonRetriableError(e))
         throw e;
       return continuePolling;
     }