From cfaaa5e2c07e5f98baeb7de99e303aa840a351e8 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Wed, 14 Sep 2022 14:18:12 +0200 Subject: [PATCH] fix: suppress init errors if the target is closed (#8947) 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 suppresses 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 | 34 +++++++++++++++++++++++++++------- 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/common/BrowserConnector.ts b/src/common/BrowserConnector.ts index 8d65873c318..de6ceda23be 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 541e225e4e0..1e4b810a022 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 8ab336bc266..4e4bb12ccb0 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 ab3a2a2c824..3e6c4dee666 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'; @@ -470,7 +474,15 @@ export class Page extends EventEmitter { ); await page.#initialize(); if (defaultViewport) { - await page.setViewport(defaultViewport); + try { + await page.setViewport(defaultViewport); + } catch (err) { + if (isErrorLike(err) && isTargetClosedError(err)) { + debugError(err); + } else { + throw err; + } + } } return page; } @@ -645,11 +657,19 @@ export class Page extends EventEmitter { }; async #initialize(): Promise { - await Promise.all([ - this.#frameManager.initialize(this.#target._targetId), - this.#client.send('Performance.enable'), - this.#client.send('Log.enable'), - ]); + try { + await Promise.all([ + this.#frameManager.initialize(this.#target._targetId), + this.#client.send('Performance.enable'), + this.#client.send('Log.enable'), + ]); + } catch (err) { + if (isErrorLike(err) && isTargetClosedError(err)) { + debugError(err); + } else { + throw err; + } + } } async #onFileChooser(