From 41474a191f839acdd7ae57e9e2684eae4f5560aa Mon Sep 17 00:00:00 2001
From: Alex Rudenko <alexrudenko@chromium.org>
Date: Tue, 13 Sep 2022 16:30:29 +0200
Subject: [PATCH] fix: supress init errors if the target is closed

With #8520 Puppeteer is now aware of all targets it connects
to. In order to have a not flaky init, Puppeteer waits for
all existing targets to be configured during the connection process.
This does not work well in case of concurrent connections because
while one connection might initializing a target the other one
might be closed it. In general, that is expected because we
can only be eventually consistent about the target state but we
also should not crash the init if some targets have been closed.
This PR implements checks to see if the errors are caused by the
target or session closures and supresses them if it's the case.
---
 src/common/BrowserConnector.ts |  1 -
 src/common/Connection.ts       | 10 ++++++++++
 src/common/FrameManager.ts     |  8 ++------
 src/common/Page.ts             | 24 +++++++++++++++++++++---
 4 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/src/common/BrowserConnector.ts b/src/common/BrowserConnector.ts
index 8d65873c31842..de6ceda23be14 100644
--- a/src/common/BrowserConnector.ts
+++ b/src/common/BrowserConnector.ts
@@ -131,7 +131,6 @@ export async function _connectToBrowser(
     targetFilter,
     isPageTarget
   );
-  await browser.pages();
   return browser;
 }
 
diff --git a/src/common/Connection.ts b/src/common/Connection.ts
index 541e225e4e072..1e4b810a022b0 100644
--- a/src/common/Connection.ts
+++ b/src/common/Connection.ts
@@ -445,3 +445,13 @@ function rewriteError(
   error.originalMessage = originalMessage ?? error.originalMessage;
   return error;
 }
+
+/**
+ * @internal
+ */
+export function isTargetClosedError(err: Error): boolean {
+  return (
+    err.message.includes('Target closed') ||
+    err.message.includes('Session closed')
+  );
+}
diff --git a/src/common/FrameManager.ts b/src/common/FrameManager.ts
index 8ab336bc26673..4e4bb12ccb044 100644
--- a/src/common/FrameManager.ts
+++ b/src/common/FrameManager.ts
@@ -19,7 +19,7 @@ import {assert} from '../util/assert.js';
 import {createDebuggableDeferredPromise} from '../util/DebuggableDeferredPromise.js';
 import {DeferredPromise} from '../util/DeferredPromise.js';
 import {isErrorLike} from '../util/ErrorLike.js';
-import {CDPSession} from './Connection.js';
+import {CDPSession, isTargetClosedError} from './Connection.js';
 import {EventEmitter} from './EventEmitter.js';
 import {EVALUATION_SCRIPT_URL, ExecutionContext} from './ExecutionContext.js';
 import {Frame} from './Frame.js';
@@ -172,11 +172,7 @@ export class FrameManager extends EventEmitter {
       ]);
     } catch (error) {
       // The target might have been closed before the initialization finished.
-      if (
-        isErrorLike(error) &&
-        (error.message.includes('Target closed') ||
-          error.message.includes('Session closed'))
-      ) {
+      if (isErrorLike(error) && isTargetClosedError(error)) {
         return;
       }
 
diff --git a/src/common/Page.ts b/src/common/Page.ts
index ab3a2a2c824a2..09481461bc105 100644
--- a/src/common/Page.ts
+++ b/src/common/Page.ts
@@ -24,7 +24,11 @@ import {
 import {isErrorLike} from '../util/ErrorLike.js';
 import {Accessibility} from './Accessibility.js';
 import {Browser, BrowserContext} from './Browser.js';
-import {CDPSession, CDPSessionEmittedEvents} from './Connection.js';
+import {
+  CDPSession,
+  CDPSessionEmittedEvents,
+  isTargetClosedError,
+} from './Connection.js';
 import {ConsoleMessage, ConsoleMessageType} from './ConsoleMessage.js';
 import {Coverage} from './Coverage.js';
 import {Dialog} from './Dialog.js';
@@ -402,6 +406,20 @@ export interface PageEventObject {
   workerdestroyed: WebWorker;
 }
 
+/**
+ * Supresses the error if it's caused by the target being closed. It usually happens
+ * if there are concurrent Puppeteer connections to the target and the target is closed by
+ * the concurrent connection.
+ * @internal
+ */
+function debugErrorIfTargetClosed(err: Error): void {
+  if (isTargetClosedError(err)) {
+    debugError(err);
+    return;
+  }
+  throw err;
+}
+
 /**
  * Page provides methods to interact with a single tab or
  * {@link https://developer.chrome.com/extensions/background_pages | extension background page}
@@ -470,7 +488,7 @@ export class Page extends EventEmitter {
     );
     await page.#initialize();
     if (defaultViewport) {
-      await page.setViewport(defaultViewport);
+      await page.setViewport(defaultViewport).catch(debugErrorIfTargetClosed);
     }
     return page;
   }
@@ -649,7 +667,7 @@ export class Page extends EventEmitter {
       this.#frameManager.initialize(this.#target._targetId),
       this.#client.send('Performance.enable'),
       this.#client.send('Log.enable'),
-    ]);
+    ]).catch(debugErrorIfTargetClosed);
   }
 
   async #onFileChooser(