Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add missing await to floating promises #5813

Merged
merged 5 commits into from
Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions src/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ program

commandWithOpenOptions('open [url]', 'open page in browser specified via -b, --browser', [])
.action(function(url, command) {
open(command, url, language());
open(command, url, language()).catch(logErrorAndExit);
})
.on('--help', function() {
console.log('');
Expand All @@ -54,7 +54,7 @@ commandWithOpenOptions('codegen [url]', 'open page and generate code for user ac
['-o, --output <file name>', 'saves the generated script to a file'],
['--target <language>', `language to use, one of javascript, python, python-async, csharp`, language()],
]).action(function(url, command) {
codegen(command, url, command.target, command.output);
codegen(command, url, command.target, command.output).catch(logErrorAndExit);
}).on('--help', function() {
console.log('');
console.log('Examples:');
Expand Down Expand Up @@ -122,7 +122,7 @@ const browsers = [
for (const {alias, name, type} of browsers) {
commandWithOpenOptions(`${alias} [url]`, `open page in ${name}`, [])
.action(function(url, command) {
open({ ...command, browser: type }, url, command.target);
open({ ...command, browser: type }, url, command.target).catch(logErrorAndExit);
}).on('--help', function() {
console.log('');
console.log('Examples:');
Expand All @@ -137,7 +137,7 @@ commandWithOpenOptions('screenshot <url> <filename>', 'capture a page screenshot
['--wait-for-timeout <timeout>', 'wait for timeout in milliseconds before taking a screenshot'],
['--full-page', 'whether to take a full page screenshot (entire scrollable area)'],
]).action(function(url, filename, command) {
screenshot(command, command, url, filename);
screenshot(command, command, url, filename).catch(logErrorAndExit);
}).on('--help', function() {
console.log('');
console.log('Examples:');
Expand All @@ -150,7 +150,7 @@ commandWithOpenOptions('pdf <url> <filename>', 'save page as pdf',
['--wait-for-selector <selector>', 'wait for given selector before saving as pdf'],
['--wait-for-timeout <timeout>', 'wait for given timeout in milliseconds before saving as pdf'],
]).action(function(url, filename, command) {
pdf(command, command, url, filename);
pdf(command, command, url, filename).catch(logErrorAndExit);
}).on('--help', function() {
console.log('');
console.log('Examples:');
Expand All @@ -164,7 +164,7 @@ if (process.env.PWTRACE) {
.option('--resources <dir>', 'load resources from shared folder')
.description('Show trace viewer')
.action(function(trace, command) {
showTraceViewer(trace, command.resources);
showTraceViewer(trace, command.resources).catch(logErrorAndExit);
}).on('--help', function() {
console.log('');
console.log('Examples:');
Expand All @@ -179,7 +179,7 @@ if (process.argv[2] === 'run-driver')
else if (process.argv[2] === 'print-api-json')
printApiJson();
else if (process.argv[2] === 'launch-server')
launchBrowserServer(process.argv[3], process.argv[4]);
launchBrowserServer(process.argv[3], process.argv[4]).catch(logErrorAndExit);
else
program.parse(process.argv);

Expand Down Expand Up @@ -446,6 +446,11 @@ function validateOptions(options: Options) {
}
}

function logErrorAndExit(e: Error) {
console.error(e);
process.exit(1);
}

function language(): string {
return process.env.PW_CLI_TARGET_LANG || 'javascript';
}
Expand Down
4 changes: 2 additions & 2 deletions src/client/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
const func = this._bindings.get(bindingCall._initializer.name);
if (!func)
return;
bindingCall.call(func);
await bindingCall.call(func);
}

setDefaultNavigationTimeout(timeout: number) {
Expand Down Expand Up @@ -238,7 +238,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
});
}

async _onClose() {
_onClose() {
if (this._browser)
this._browser._contexts.delete(this);
this.emit(Events.BrowserContext.Close, this);
Expand Down
4 changes: 2 additions & 2 deletions src/client/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,10 @@ export class Page extends ChannelOwner<channels.PageChannel, channels.PageInitia
async _onBinding(bindingCall: BindingCall) {
const func = this._bindings.get(bindingCall._initializer.name);
if (func) {
bindingCall.call(func);
await bindingCall.call(func);
return;
}
this._browserContext._onBinding(bindingCall);
await this._browserContext._onBinding(bindingCall);
}

_onWorker(worker: Worker): void {
Expand Down
2 changes: 1 addition & 1 deletion src/dispatchers/androidDispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export class AndroidSocketDispatcher extends Dispatcher<SocketBackend, channels.
}

async close(params: channels.AndroidSocketCloseParams, metadata: CallMetadata): Promise<void> {
await this._object.close();
this._object.close();
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/remote/playwrightServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ export class PlaywrightServer {
ws.on('message', message => dispatcherConnection.dispatch(JSON.parse(message.toString())));
ws.on('close', () => {
debugLog('Client closed');
this._onDisconnect();
this._onDisconnect().catch(debugLog);
});
ws.on('error', error => {
debugLog('Client error ' + error);
this._onDisconnect();
this._onDisconnect().catch(debugLog);
});
dispatcherConnection.onmessage = message => ws.send(JSON.stringify(message));
new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), createPlaywright());
Expand Down
12 changes: 6 additions & 6 deletions src/server/android/android.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ export interface Backend {
export interface DeviceBackend {
serial: string;
status: string;
close(): Promise<void>;
close(): void;
init(): Promise<void>;
runCommand(command: string): Promise<Buffer>;
open(command: string): Promise<SocketBackend>;
}

export interface SocketBackend extends EventEmitter {
write(data: Buffer): Promise<void>;
close(): Promise<void>;
close(): void;
}

export class Android extends SdkObject {
Expand Down Expand Up @@ -176,7 +176,7 @@ export class AndroidDevice extends SdkObject {
await this.installApk(await readFileAsync(require.resolve(`../../../bin/${file}`)));

debug('pw:android')('Starting the new driver');
this.shell('am instrument -w com.microsoft.playwright.androiddriver.test/androidx.test.runner.AndroidJUnitRunner');
this.shell('am instrument -w com.microsoft.playwright.androiddriver.test/androidx.test.runner.AndroidJUnitRunner').catch(e => debug('pw:android')(e));
const socket = await this._waitForLocalAbstract('playwright_android_driver_socket');
const transport = new Transport(socket, socket, socket, 'be');
transport.onmessage = message => {
Expand Down Expand Up @@ -301,7 +301,7 @@ export class AndroidDevice extends SdkObject {
await installSocket.write(content);
const success = await new Promise(f => installSocket.on('data', f));
debug('pw:android')('Written driver bytes: ' + success);
await installSocket.close();
installSocket.close();
}

async push(content: Buffer, path: string, mode = 0o644): Promise<void> {
Expand All @@ -325,7 +325,7 @@ export class AndroidDevice extends SdkObject {
const code = result.slice(0, 4).toString();
if (code !== 'OKAY')
throw new Error('Could not push: ' + code);
await socket.close();
socket.close();
}

private async _refreshWebViews() {
Expand Down Expand Up @@ -420,7 +420,7 @@ Sec-WebSocket-Version: 13\r
}

async close() {
await this._socket!.close();
this._socket!.close();
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/server/android/backendAdb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ async function runCommand(command: string, serial?: string): Promise<Buffer> {
} else {
commandOutput = await socket.readAll();
}
await socket.close();
socket.close();
return commandOutput;
}

Expand Down Expand Up @@ -138,7 +138,7 @@ class BufferedSocketWrapper extends EventEmitter implements SocketBackend {
await new Promise(f => this._socket.write(data, f));
}

async close() {
close() {
if (this._isClosed)
return;
debug('pw:adb')('Close ' + this._command);
Expand Down
4 changes: 2 additions & 2 deletions src/server/chromium/crDevTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class CRDevTools {
this._savePromise = Promise.resolve();
}

async install(session: CRSession) {
install(session: CRSession) {
session.on('Runtime.bindingCalled', async event => {
if (event.name !== kBindingName)
return;
Expand Down Expand Up @@ -66,7 +66,7 @@ export class CRDevTools {
contextId: event.executionContextId
}).catch(e => null);
});
await Promise.all([
Promise.all([
session.send('Runtime.enable'),
session.send('Runtime.addBinding', { name: kBindingName }),
session.send('Page.enable'),
Expand Down
9 changes: 4 additions & 5 deletions src/server/chromium/crPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -646,11 +646,10 @@ class FrameSession {
session.once('Runtime.executionContextCreated', async event => {
worker._createExecutionContext(new CRExecutionContext(session, event.context));
});
Promise.all([
session._sendMayFail('Runtime.enable'),
session._sendMayFail('Network.enable'),
session._sendMayFail('Runtime.runIfWaitingForDebugger'),
]); // This might fail if the target is closed before we initialize.
// This might fail if the target is closed before we initialize.
session._sendMayFail('Runtime.enable');
session._sendMayFail('Network.enable');
session._sendMayFail('Runtime.runIfWaitingForDebugger');
session.on('Runtime.consoleAPICalled', event => {
const args = event.args.map(o => worker._existingExecutionContext!.createHandle(o));
this._page._addConsoleMessage(event.type, args, toConsoleMessageLocation(event.stackTrace));
Expand Down
6 changes: 3 additions & 3 deletions src/server/download.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class Download {
throw new Error('Download not found on disk. Check download.failure() for details.');

if (this._finished) {
saveCallback(path.join(this._downloadsPath, this._uuid));
saveCallback(path.join(this._downloadsPath, this._uuid)).catch(e => {});
return;
}
this._saveCallbacks.push(saveCallback);
Expand Down Expand Up @@ -117,7 +117,7 @@ export class Download {
const fileName = path.join(this._downloadsPath, this._uuid);
await util.promisify(fs.unlink)(fileName).catch(e => {});
}
this._reportFinished('Download deleted upon browser context closure.');
await this._reportFinished('Download deleted upon browser context closure.');
}

async _reportFinished(error?: string) {
Expand All @@ -128,7 +128,7 @@ export class Download {

if (error) {
for (const callback of this._saveCallbacks)
callback('', error);
await callback('', error);
} else {
const fullPath = path.join(this._downloadsPath, this._uuid);
for (const callback of this._saveCallbacks)
Expand Down
2 changes: 1 addition & 1 deletion src/server/firefox/ffExecutionContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export class FFExecutionContext implements js.ExecutionContextDelegate {
await this._session.send('Runtime.disposeObject', {
executionContextId: this._executionContextId,
objectId: handle._objectId,
}).catch(error => {});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's caller's responsibility to catch exceptions if any and it does so now in .dispose().

}
}

Expand Down
6 changes: 3 additions & 3 deletions src/server/firefox/ffPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,14 +232,14 @@ export class FFPage implements PageDelegate {
const context = this._contextIdToContext.get(event.executionContextId)!;
const pageOrError = await this.pageOrError();
if (!(pageOrError instanceof Error))
this._page._onBindingCalled(event.payload, context);
await this._page._onBindingCalled(event.payload, context);
}

async _onFileChooserOpened(payload: Protocol.Page.fileChooserOpenedPayload) {
const {executionContextId, element} = payload;
const context = this._contextIdToContext.get(executionContextId)!;
const handle = context.createHandle(element).asElement()!;
this._page._onFileChooserOpened(handle);
await this._page._onFileChooserOpened(handle);
}

async _onWorkerCreated(event: Protocol.Page.workerCreatedPayload) {
Expand Down Expand Up @@ -267,7 +267,7 @@ export class FFPage implements PageDelegate {
// Note: we receive worker exceptions directly from the page.
}

async _onWorkerDestroyed(event: Protocol.Page.workerDestroyedPayload) {
_onWorkerDestroyed(event: Protocol.Page.workerDestroyedPayload) {
const workerId = event.workerId;
const worker = this._workers.get(workerId);
if (!worker)
Expand Down
4 changes: 2 additions & 2 deletions src/server/javascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,11 @@ export class JSHandle<T = any> extends SdkObject {
return null;
}

async dispose() {
dispose() {
if (this._disposed)
return;
this._disposed = true;
await this._context._delegate.releaseHandle(this);
this._context._delegate.releaseHandle(this).catch(e => {});
}

toString(): string {
Expand Down
2 changes: 1 addition & 1 deletion src/server/progress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class ProgressController {
return result;
} catch (e) {
this._state = 'aborted';
await Promise.all(this._cleanups.splice(0).map(cleanup => runCleanup(cleanup)));
await Promise.all(this._cleanups.splice(0).map(runCleanup));
throw e;
} finally {
clearTimeout(timer);
Expand Down
3 changes: 1 addition & 2 deletions src/server/trace/recorder/tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ class ContextTracer {
async dispose() {
this._disposed = true;
helper.removeEventListeners(this._eventListeners);
this._snapshotter.dispose();
await this._snapshotter.dispose();
const event: trace.ContextDestroyedTraceEvent = {
timestamp: monotonicTime(),
type: 'context-destroyed',
Expand All @@ -219,7 +219,6 @@ class ContextTracer {

// Ensure all writes are finished.
await this._appendEventChain;
await this._snapshotter.dispose();
}

private _appendTraceEvent(event: any) {
Expand Down
2 changes: 1 addition & 1 deletion src/server/webkit/wkExecutionContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
async releaseHandle(handle: js.JSHandle): Promise<void> {
if (!handle._objectId)
return;
await this._session.send('Runtime.releaseObject', {objectId: handle._objectId}).catch(error => {});
await this._session.send('Runtime.releaseObject', {objectId: handle._objectId});
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/server/webkit/wkPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export class WKPage implements PageDelegate {
promises.push(this.updateHttpCredentials());
if (this._browserContext._permissions.size) {
for (const [key, value] of this._browserContext._permissions)
this._grantPermissions(key, value);
promises.push(this._grantPermissions(key, value));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

}
if (this._browserContext._options.recordVideo) {
const size = this._browserContext._options.recordVideo.size || this._browserContext._options.viewport || { width: 1280, height: 720 };
Expand Down