Skip to content

Commit

Permalink
fix: suppress init errors if the target is closed (#8947)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
OrKoN authored and jrandolf committed Sep 15, 2022
1 parent c23c00a commit cfaaa5e
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 14 deletions.
1 change: 0 additions & 1 deletion src/common/BrowserConnector.ts
Expand Up @@ -131,7 +131,6 @@ export async function _connectToBrowser(
targetFilter,
isPageTarget
);
await browser.pages();
return browser;
}

Expand Down
10 changes: 10 additions & 0 deletions src/common/Connection.ts
Expand Up @@ -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')
);
}
8 changes: 2 additions & 6 deletions src/common/FrameManager.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

Expand Down
34 changes: 27 additions & 7 deletions src/common/Page.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -645,11 +657,19 @@ export class Page extends EventEmitter {
};

async #initialize(): Promise<void> {
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(
Expand Down

0 comments on commit cfaaa5e

Please sign in to comment.