Skip to content

Commit 985faeb

Browse files
authored
fix: avoid unhandled promise rejection in WKSession.send (#770)
1 parent fd3a930 commit 985faeb

File tree

5 files changed

+39
-32
lines changed

5 files changed

+39
-32
lines changed

src/webkit/wkConnection.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,20 +119,19 @@ export class WKSession extends platform.EventEmitter {
119119
this.once = super.once;
120120
}
121121

122-
send<T extends keyof Protocol.CommandParameters>(
122+
async send<T extends keyof Protocol.CommandParameters>(
123123
method: T,
124124
params?: Protocol.CommandParameters[T]
125125
): Promise<Protocol.CommandReturnValues[T]> {
126126
if (this._disposed)
127-
return Promise.reject(new Error(`Protocol error (${method}): ${this.errorText}`));
127+
throw new Error(`Protocol error (${method}): ${this.errorText}`);
128128
const id = this.connection.nextMessageId();
129129
const messageObj = { id, method, params };
130130
platform.debug('pw:wrapped:' + this.sessionId)('SEND ► ' + JSON.stringify(messageObj, null, 2));
131-
const result = new Promise<Protocol.CommandReturnValues[T]>((resolve, reject) => {
131+
this._rawSend(messageObj);
132+
return new Promise<Protocol.CommandReturnValues[T]>((resolve, reject) => {
132133
this._callbacks.set(id, {resolve, reject, error: new Error(), method});
133134
});
134-
this._rawSend(messageObj);
135-
return result;
136135
}
137136

138137
isDisposed(): boolean {

src/webkit/wkExecutionContext.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,21 +120,17 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
120120
throw error;
121121
}
122122

123-
let callFunctionOnPromise;
124-
try {
125-
callFunctionOnPromise = this._session.send('Runtime.callFunctionOn', {
126-
functionDeclaration: functionText + '\n' + suffix + '\n',
127-
objectId: thisObjectId,
128-
arguments: serializableArgs.map((arg: any) => this._convertArgument(arg)),
129-
returnByValue: false,
130-
emulateUserGesture: true
131-
});
132-
} catch (err) {
123+
return this._session.send('Runtime.callFunctionOn', {
124+
functionDeclaration: functionText + '\n' + suffix + '\n',
125+
objectId: thisObjectId,
126+
arguments: serializableArgs.map((arg: any) => this._convertArgument(arg)),
127+
returnByValue: false,
128+
emulateUserGesture: true
129+
}).catch(err => {
133130
if (err instanceof TypeError && err.message.startsWith('Converting circular structure to JSON'))
134131
err.message += ' Are you passing a nested JSHandle?';
135132
throw err;
136-
}
137-
return callFunctionOnPromise.then(response => {
133+
}).then(response => {
138134
if (response.result.type === 'object' && response.result.className === 'Promise') {
139135
return Promise.race([
140136
this._executionContextDestroyedPromise.then(() => contextDestroyedResult),

src/webkit/wkPage.ts

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,24 @@ export class WKPage implements PageDelegate {
9595
// This method is called for provisional targets as well. The session passed as the parameter
9696
// may be different from the current session and may be destroyed without becoming current.
9797
async _initializeSession(session: WKSession, resourceTreeHandler: (r: Protocol.Page.getResourceTreeReturnValue) => void) {
98-
const promises : Promise<any>[] = [
98+
await this._initializeSessionMayThrow(session, resourceTreeHandler).catch(e => {
99+
if (session.isDisposed())
100+
return;
101+
// Swallow initialization errors due to newer target swap in,
102+
// since we will reinitialize again.
103+
if (this._session === session)
104+
throw e;
105+
});
106+
}
107+
108+
private async _initializeSessionMayThrow(session: WKSession, resourceTreeHandler: (r: Protocol.Page.getResourceTreeReturnValue) => void) {
109+
const [, frameTree] = await Promise.all([
99110
// Page agent must be enabled before Runtime.
100111
session.send('Page.enable'),
101-
session.send('Page.getResourceTree').then(resourceTreeHandler),
112+
session.send('Page.getResourceTree'),
113+
]);
114+
resourceTreeHandler(frameTree);
115+
const promises : Promise<any>[] = [
102116
// Resource tree should be received before first execution context.
103117
session.send('Runtime.enable'),
104118
session.send('Page.createUserWorld', { name: UTILITY_WORLD_NAME }).catch(_ => {}), // Worlds are per-process
@@ -129,19 +143,13 @@ export class WKPage implements PageDelegate {
129143
promises.push(session.send('Network.setExtraHTTPHeaders', { headers: this._page._state.extraHTTPHeaders }));
130144
if (this._page._state.hasTouch)
131145
promises.push(session.send('Page.setTouchEmulationEnabled', { enabled: true }));
132-
await Promise.all(promises).catch(e => {
133-
if (session.isDisposed())
134-
return;
135-
// Swallow initialization errors due to newer target swap in,
136-
// since we will reinitialize again.
137-
if (this._session === session)
138-
throw e;
139-
});
146+
await Promise.all(promises);
140147
}
141148

142-
onProvisionalLoadStarted(provisionalSession: WKSession) {
149+
initializeProvisionalPage(provisionalSession: WKSession) : Promise<void> {
143150
assert(!this._provisionalPage);
144151
this._provisionalPage = new WKProvisionalPage(provisionalSession, this);
152+
return this._provisionalPage.initializationPromise;
145153
}
146154

147155
onProvisionalLoadCommitted(session: WKSession) {

src/webkit/wkPageProxy.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,13 @@ export class WKPageProxy {
139139
}
140140
if (targetInfo.isProvisional) {
141141
(session as any)[isPovisionalSymbol] = true;
142-
if (this._wkPage)
143-
this._wkPage.onProvisionalLoadStarted(session);
144-
if (targetInfo.isPaused)
142+
if (this._wkPage) {
143+
const provisionalPageInitialized = this._wkPage.initializeProvisionalPage(session);
144+
if (targetInfo.isPaused)
145+
provisionalPageInitialized.then(() => this._resumeTarget(targetInfo.targetId));
146+
} else if (targetInfo.isPaused) {
145147
this._resumeTarget(targetInfo.targetId);
148+
}
146149
} else if (this._pagePromise) {
147150
assert(!this._pagePausedOnStart);
148151
// This is the first time page target is created, will resume

src/webkit/wkProvisionalPage.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export class WKProvisionalPage {
2424
private readonly _wkPage: WKPage;
2525
private _sessionListeners: RegisteredListener[] = [];
2626
private _mainFrameId: string | null = null;
27+
readonly initializationPromise: Promise<void>;
2728

2829
constructor(session: WKSession, page: WKPage) {
2930
this._session = session;
@@ -47,7 +48,7 @@ export class WKProvisionalPage {
4748
helper.addEventListener(session, 'Network.loadingFailed', overrideFrameId(e => wkPage._onLoadingFailed(e))),
4849
];
4950

50-
this._wkPage._initializeSession(session, ({frameTree}) => this._handleFrameTree(frameTree));
51+
this.initializationPromise = this._wkPage._initializeSession(session, ({frameTree}) => this._handleFrameTree(frameTree));
5152
}
5253

5354
dispose() {

0 commit comments

Comments
 (0)