Skip to content

Commit

Permalink
fix(webkit): do not swallow init errors (#2242)
Browse files Browse the repository at this point in the history
This is a speculative fix to the following issue from the bots:

NON-TEST ERROR #0: UNHANDLED ERROR
  TypeError: Cannot read property 'url' of undefined
      at WKPage._onTargetCreated (/Users/runner/runners/2.169.1/work/playwright/playwright/src/webkit/wkPage.ts:274:12)
      at process._tickCallback (internal/process/next_tick.js:68:7)

I assume that _initializeSession did swallow an error, so we erroneously
consider Page to be fully initialized (and having main frame).
  • Loading branch information
dgozman authored May 14, 2020
1 parent e8e761f commit 63cc126
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
8 changes: 5 additions & 3 deletions src/webkit/wkPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ export class WKPage implements PageDelegate {

// This method is called for provisional targets as well. The session passed as the parameter
// may be different from the current session and may be destroyed without becoming current.
async _initializeSession(session: WKSession, resourceTreeHandler: (r: Protocol.Page.getResourceTreeReturnValue) => void) {
async _initializeSession(session: WKSession, provisional: boolean, resourceTreeHandler: (r: Protocol.Page.getResourceTreeReturnValue) => void) {
await this._initializeSessionMayThrow(session, resourceTreeHandler).catch(e => {
if (session.isDisposed())
// Provisional session can be disposed at any time, for example due to new navigation initiating
// a new provisional page.
if (provisional && session.isDisposed())
return;
// Swallow initialization errors due to newer target swap in,
// since we will reinitialize again.
Expand Down Expand Up @@ -263,7 +265,7 @@ export class WKPage implements PageDelegate {
this._setSession(session);
await Promise.all([
this._initializePageProxySession(),
this._initializeSession(session, ({frameTree}) => this._handleFrameTree(frameTree)),
this._initializeSession(session, false, ({frameTree}) => this._handleFrameTree(frameTree)),
]);
pageOrError = this._page;
} catch (e) {
Expand Down
2 changes: 1 addition & 1 deletion src/webkit/wkProvisionalPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class WKProvisionalPage {
helper.addEventListener(session, 'Network.loadingFailed', overrideFrameId(e => wkPage._onLoadingFailed(e))),
];

this.initializationPromise = this._wkPage._initializeSession(session, ({frameTree}) => this._handleFrameTree(frameTree));
this.initializationPromise = this._wkPage._initializeSession(session, true, ({frameTree}) => this._handleFrameTree(frameTree));
}

dispose() {
Expand Down

0 comments on commit 63cc126

Please sign in to comment.