chore: simplify timeout handling in progress (#2487)

This commit is contained in:
Dmitry Gozman 2020-06-05 15:53:30 -07:00 committed by GitHub
parent 300099734c
commit 3ec79e17fc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 56 additions and 70 deletions

View File

@ -314,7 +314,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
}
hover(options: PointerActionOptions & types.PointerActionWaitOptions = {}): Promise<void> {
return runAbortableTask(progress => this._hover(progress, options), options, this._page, this._page._timeoutSettings);
return runAbortableTask(progress => this._hover(progress, options), this._page, this._page._timeoutSettings.timeout(options));
}
_hover(progress: Progress, options: PointerActionOptions & types.PointerActionWaitOptions): Promise<void> {
@ -322,7 +322,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
}
click(options: ClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise<void> {
return runAbortableTask(progress => this._click(progress, options), options, this._page, this._page._timeoutSettings);
return runAbortableTask(progress => this._click(progress, options), this._page, this._page._timeoutSettings.timeout(options));
}
_click(progress: Progress, options: ClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<void> {
@ -330,7 +330,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
}
dblclick(options: MultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise<void> {
return runAbortableTask(progress => this._dblclick(progress, options), options, this._page, this._page._timeoutSettings);
return runAbortableTask(progress => this._dblclick(progress, options), this._page, this._page._timeoutSettings.timeout(options));
}
_dblclick(progress: Progress, options: MultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<void> {
@ -338,7 +338,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
}
async selectOption(values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[], options: types.NavigatingActionWaitOptions = {}): Promise<string[]> {
return runAbortableTask(progress => this._selectOption(progress, values, options), options, this._page, this._page._timeoutSettings);
return runAbortableTask(progress => this._selectOption(progress, values, options), this._page, this._page._timeoutSettings.timeout(options));
}
async _selectOption(progress: Progress, values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[], options: types.NavigatingActionWaitOptions): Promise<string[]> {
@ -366,7 +366,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
}
async fill(value: string, options: types.NavigatingActionWaitOptions = {}): Promise<void> {
return runAbortableTask(progress => this._fill(progress, value, options), options, this._page, this._page._timeoutSettings);
return runAbortableTask(progress => this._fill(progress, value, options), this._page, this._page._timeoutSettings.timeout(options));
}
async _fill(progress: Progress, value: string, options: types.NavigatingActionWaitOptions): Promise<void> {
@ -395,7 +395,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
}
async setInputFiles(files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions = {}) {
return runAbortableTask(async progress => this._setInputFiles(progress, files, options), options, this._page, this._page._timeoutSettings);
return runAbortableTask(async progress => this._setInputFiles(progress, files, options), this._page, this._page._timeoutSettings.timeout(options));
}
async _setInputFiles(progress: Progress, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions) {
@ -440,7 +440,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
}
async type(text: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) {
return runAbortableTask(progress => this._type(progress, text, options), options, this._page, this._page._timeoutSettings);
return runAbortableTask(progress => this._type(progress, text, options), this._page, this._page._timeoutSettings.timeout(options));
}
async _type(progress: Progress, text: string, options: { delay?: number } & types.NavigatingActionWaitOptions) {
@ -452,7 +452,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
}
async press(key: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) {
return runAbortableTask(progress => this._press(progress, key, options), options, this._page, this._page._timeoutSettings);
return runAbortableTask(progress => this._press(progress, key, options), this._page, this._page._timeoutSettings.timeout(options));
}
async _press(progress: Progress, key: string, options: { delay?: number } & types.NavigatingActionWaitOptions) {
@ -467,14 +467,14 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
return runAbortableTask(async progress => {
progress.log(apiLog, `elementHandle.check()`);
await this._setChecked(progress, true, options);
}, options, this._page, this._page._timeoutSettings);
}, this._page, this._page._timeoutSettings.timeout(options));
}
async uncheck(options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
return runAbortableTask(async progress => {
progress.log(apiLog, `elementHandle.uncheck()`);
await this._setChecked(progress, false, options);
}, options, this._page, this._page._timeoutSettings);
}, this._page, this._page._timeoutSettings.timeout(options));
}
async _setChecked(progress: Progress, state: boolean, options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions) {

View File

@ -31,7 +31,7 @@ export abstract class ExtendedEventEmitter extends EventEmitter {
const options = typeof optionsOrPredicate === 'function' ? { predicate: optionsOrPredicate } : optionsOrPredicate;
const { predicate = () => true } = options;
const progressController = new ProgressController(options, this._getLogger(), this._getTimeoutSettings());
const progressController = new ProgressController(this._getLogger(), this._getTimeoutSettings().timeout(options));
this._abortPromiseForEvent(event).then(error => progressController.abort(error));
return progressController.run(async progress => {

View File

@ -342,7 +342,7 @@ export class Frame {
}
async goto(url: string, options: GotoOptions = {}): Promise<network.Response | null> {
const progressController = new ProgressController(options, this._page, this._page._timeoutSettings.navigationTimeout());
const progressController = new ProgressController(this._page, this._page._timeoutSettings.navigationTimeout(options));
abortProgressOnFrameDetach(progressController, this);
return progressController.run(async progress => {
progress.log(apiLog, `${progress.apiName}("${url}"), waiting until "${options.waitUntil || 'load'}"`);
@ -377,7 +377,7 @@ export class Frame {
}
async waitForNavigation(options: types.WaitForNavigationOptions = {}): Promise<network.Response | null> {
const progressController = new ProgressController(options, this._page, this._page._timeoutSettings.navigationTimeout());
const progressController = new ProgressController(this._page, this._page._timeoutSettings.navigationTimeout(options));
abortProgressOnFrameDetach(progressController, this);
return progressController.run(async progress => {
const toUrl = typeof options.url === 'string' ? ` to "${options.url}"` : '';
@ -396,7 +396,7 @@ export class Frame {
}
async waitForLoadState(state: types.LifecycleEvent = 'load', options: types.TimeoutOptions = {}): Promise<void> {
const progressController = new ProgressController(options, this._page, this._page._timeoutSettings.navigationTimeout());
const progressController = new ProgressController(this._page, this._page._timeoutSettings.navigationTimeout(options));
abortProgressOnFrameDetach(progressController, this);
return progressController.run(progress => this._waitForLoadState(progress, state));
}
@ -469,16 +469,16 @@ export class Frame {
return adopted;
}
return handle;
}, options, this._page, this._page._timeoutSettings);
}, this._page, this._page._timeoutSettings.timeout(options));
}
async dispatchEvent(selector: string, type: string, eventInit?: Object, options?: types.TimeoutOptions): Promise<void> {
async dispatchEvent(selector: string, type: string, eventInit?: Object, options: types.TimeoutOptions = {}): Promise<void> {
const task = selectors._dispatchEventTask(selector, type, eventInit || {});
return runAbortableTask(async progress => {
progress.log(apiLog, `Dispatching "${type}" event on selector "${selector}"...`);
const result = await this._scheduleRerunnableTask(progress, 'main', task);
result.dispose();
}, options || {}, this._page, this._page._timeoutSettings);
}, this._page, this._page._timeoutSettings.timeout(options));
}
async $eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element, Arg, R>, arg: Arg): Promise<R>;
@ -520,7 +520,7 @@ export class Frame {
}
async setContent(html: string, options: types.NavigateOptions = {}): Promise<void> {
const progressController = new ProgressController(options, this._page, this._page._timeoutSettings.navigationTimeout());
const progressController = new ProgressController(this._page, this._page._timeoutSettings.navigationTimeout(options));
abortProgressOnFrameDetach(progressController, this);
return progressController.run(async progress => {
const waitUntil = options.waitUntil === undefined ? 'load' : options.waitUntil;
@ -729,7 +729,7 @@ export class Frame {
}
}
return undefined as any;
}, options, this._page, this._page._timeoutSettings);
}, this._page, this._page._timeoutSettings.timeout(options));
}
async click(selector: string, options: dom.ClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
@ -817,7 +817,7 @@ export class Frame {
};
return runAbortableTask(
progress => this._scheduleRerunnableTask(progress, 'main', task),
options, this._page, this._page._timeoutSettings);
this._page, this._page._timeoutSettings.timeout(options));
}
async title(): Promise<string> {

View File

@ -261,15 +261,6 @@ class Helper {
return crypto.randomBytes(16).toString('hex');
}
static monotonicTime(): number {
const [seconds, nanoseconds] = process.hrtime();
return seconds * 1000 + (nanoseconds / 1000000 | 0);
}
static timeUntilDeadline(deadline: number): number {
return Math.min(deadline - this.monotonicTime(), 2147483647); // 2^31-1 safe setTimeout in Node.
}
static getViewportSizeFromWindowFeatures(features: string[]): types.Size | null {
const widthString = features.find(f => f.startsWith('width='));
const heightString = features.find(f => f.startsWith('height='));

View File

@ -16,22 +16,20 @@
import { InnerLogger, Log } from './logger';
import { TimeoutError } from './errors';
import { helper, assert } from './helper';
import * as types from './types';
import { DEFAULT_TIMEOUT, TimeoutSettings } from './timeoutSettings';
import { assert } from './helper';
import { getCurrentApiCall, rewriteErrorMessage } from './debug/stackTrace';
export interface Progress {
readonly apiName: string;
readonly deadline: number; // To be removed?
readonly aborted: Promise<void>;
timeUntilDeadline(): number;
isRunning(): boolean;
cleanupWhenAborted(cleanup: () => any): void;
log(log: Log, message: string | Error): void;
}
export async function runAbortableTask<T>(task: (progress: Progress) => Promise<T>, timeoutOptions: types.TimeoutOptions, logger: InnerLogger, timeoutSettingsOrDefaultTimeout?: TimeoutSettings | number, apiName?: string): Promise<T> {
const controller = new ProgressController(timeoutOptions, logger, timeoutSettingsOrDefaultTimeout, apiName);
export async function runAbortableTask<T>(task: (progress: Progress) => Promise<T>, logger: InnerLogger, timeout: number, apiName?: string): Promise<T> {
const controller = new ProgressController(logger, timeout, apiName);
return controller.run(task);
}
@ -56,19 +54,12 @@ export class ProgressController {
private _deadline: number;
private _timeout: number;
constructor(timeoutOptions: types.TimeoutOptions, logger: InnerLogger, timeoutSettingsOrDefaultTimeout?: TimeoutSettings | number, apiName?: string) {
constructor(logger: InnerLogger, timeout: number, apiName?: string) {
this._apiName = apiName || getCurrentApiCall();
this._logger = logger;
// TODO: figure out nice timeout parameters.
let defaultTimeout = DEFAULT_TIMEOUT;
if (typeof timeoutSettingsOrDefaultTimeout === 'number')
defaultTimeout = timeoutSettingsOrDefaultTimeout;
if (timeoutSettingsOrDefaultTimeout instanceof TimeoutSettings)
defaultTimeout = timeoutSettingsOrDefaultTimeout.timeout();
const { timeout = defaultTimeout } = timeoutOptions;
this._timeout = timeout;
this._deadline = TimeoutSettings.computeDeadline(timeout);
this._deadline = timeout ? monotonicTime() + timeout : 0;
this._forceAbortPromise = new Promise((resolve, reject) => this._forceAbort = reject);
this._forceAbortPromise.catch(e => null); // Prevent unhandle promsie rejection.
@ -81,8 +72,8 @@ export class ProgressController {
const progress: Progress = {
apiName: this._apiName,
deadline: this._deadline,
aborted: this._abortedPromise,
timeUntilDeadline: () => this._deadline ? this._deadline - monotonicTime() : 2147483647, // 2^31-1 safe setTimeout in Node.
isRunning: () => this._state === 'running',
cleanupWhenAborted: (cleanup: () => any) => {
if (this._state === 'running')
@ -98,7 +89,7 @@ export class ProgressController {
};
const timeoutError = new TimeoutError(`Timeout ${this._timeout}ms exceeded during ${this._apiName}.`);
const timer = setTimeout(() => this._forceAbort(timeoutError), helper.timeUntilDeadline(this._deadline));
const timer = setTimeout(() => this._forceAbort(timeoutError), progress.timeUntilDeadline());
try {
const promise = task(progress);
const result = await Promise.race([promise, this._forceAbortPromise]);
@ -138,3 +129,8 @@ function formatLogRecording(log: string[], name: string): string {
const rightLength = headerLength - name.length - leftLength;
return `\n${'='.repeat(leftLength)}${name}${'='.repeat(rightLength)}\n${log.join('\n')}\n${'='.repeat(headerLength)}`;
}
function monotonicTime(): number {
const [seconds, nanoseconds] = process.hrtime();
return seconds * 1000 + (nanoseconds / 1000000 | 0);
}

View File

@ -15,7 +15,6 @@
*/
import { ChildProcess } from 'child_process';
import { helper } from '../helper';
import { EventEmitter } from 'events';
export class WebSocketWrapper {
@ -82,12 +81,12 @@ export class BrowserServer extends EventEmitter {
await this._webSocketWrapper.checkLeaks();
}
async _closeOrKill(deadline: number): Promise<void> {
async _closeOrKill(timeout: number): Promise<void> {
let timer: NodeJS.Timer;
try {
await Promise.race([
this.close(),
new Promise((resolve, reject) => timer = setTimeout(reject, helper.timeUntilDeadline(deadline))),
new Promise((resolve, reject) => timer = setTimeout(reject, timeout)),
]);
} catch (ignored) {
await this.kill().catch(ignored => {}); // Make sure to await actual process exit.

View File

@ -30,6 +30,7 @@ import { Events } from '../events';
import { PipeTransport } from './pipeTransport';
import { Progress, runAbortableTask } from '../progress';
import { ProxySettings } from '../types';
import { TimeoutSettings } from '../timeoutSettings';
export type BrowserArgOptions = {
headless?: boolean,
@ -109,7 +110,7 @@ export abstract class BrowserTypeBase implements BrowserType {
assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead');
assert(!(options as any).port, 'Cannot specify a port without launching as a server.');
const logger = new RootLogger(options.logger);
const browser = await runAbortableTask(progress => this._innerLaunch(progress, options, logger, undefined), options, logger);
const browser = await runAbortableTask(progress => this._innerLaunch(progress, options, logger, undefined), logger, TimeoutSettings.timeout(options));
return browser;
}
@ -117,7 +118,7 @@ export abstract class BrowserTypeBase implements BrowserType {
assert(!(options as any).port, 'Cannot specify a port without launching as a server.');
const persistent = validatePersistentContextOptions(options);
const logger = new RootLogger(options.logger);
const browser = await runAbortableTask(progress => this._innerLaunch(progress, options, logger, persistent, userDataDir), options, logger);
const browser = await runAbortableTask(progress => this._innerLaunch(progress, options, logger, persistent, userDataDir), logger, TimeoutSettings.timeout(options));
return browser._defaultContext!;
}
@ -152,7 +153,7 @@ export abstract class BrowserTypeBase implements BrowserType {
const { browserServer, transport } = await this._launchServer(progress, options, false, logger);
browserServer._webSocketWrapper = this._wrapTransportWithWebSocket(transport, logger, port);
return browserServer;
}, options, logger);
}, logger, TimeoutSettings.timeout(options));
}
async connect(options: ConnectOptions): Promise<Browser> {
@ -164,7 +165,7 @@ export abstract class BrowserTypeBase implements BrowserType {
await (options as any).__testHookBeforeCreateBrowser();
const browser = await this._connectToTransport(transport, { slowMo: options.slowMo, logger });
return browser;
}, options, logger);
}, logger, TimeoutSettings.timeout(options));
}
private async _launchServer(progress: Progress, options: LaunchServerOptions, isPersistent: boolean, logger: RootLogger, userDataDir?: string): Promise<{ browserServer: BrowserServer, downloadsPath: string, transport: ConnectionTransport }> {
@ -225,7 +226,7 @@ export abstract class BrowserTypeBase implements BrowserType {
},
});
browserServer = new BrowserServer(launchedProcess, gracefullyClose, kill);
progress.cleanupWhenAborted(() => browserServer && browserServer._closeOrKill(progress.deadline));
progress.cleanupWhenAborted(() => browserServer && browserServer._closeOrKill(progress.timeUntilDeadline()));
if (this._webSocketNotPipe) {
const match = await waitForLine(progress, launchedProcess, this._webSocketNotPipe.stream === 'stdout' ? launchedProcess.stdout : launchedProcess.stderr, this._webSocketNotPipe.webSocketRegex);

View File

@ -204,6 +204,6 @@ export class Electron {
app = new ElectronApplication(logger, browser, nodeConnection);
await app._init();
return app;
}, options, logger);
}, logger, TimeoutSettings.timeout(options));
}
}

View File

@ -16,10 +16,9 @@
*/
import { TimeoutOptions } from './types';
import { helper } from './helper';
import * as debugSupport from './debug/debugSupport';
export const DEFAULT_TIMEOUT = debugSupport.isDebugMode() ? 0 : 30000;
const DEFAULT_TIMEOUT = debugSupport.isDebugMode() ? 0 : 30000;
export class TimeoutSettings {
private _parent: TimeoutSettings | undefined;
@ -38,31 +37,31 @@ export class TimeoutSettings {
this._defaultNavigationTimeout = timeout;
}
navigationTimeout(): number {
navigationTimeout(options: TimeoutOptions): number {
if (typeof options.timeout === 'number')
return options.timeout;
if (this._defaultNavigationTimeout !== null)
return this._defaultNavigationTimeout;
if (this._defaultTimeout !== null)
return this._defaultTimeout;
if (this._parent)
return this._parent.navigationTimeout();
return this._parent.navigationTimeout(options);
return DEFAULT_TIMEOUT;
}
timeout(): number {
timeout(options: TimeoutOptions): number {
if (typeof options.timeout === 'number')
return options.timeout;
if (this._defaultTimeout !== null)
return this._defaultTimeout;
if (this._parent)
return this._parent.timeout();
return this._parent.timeout(options);
return DEFAULT_TIMEOUT;
}
computeDeadline(options: TimeoutOptions = {}) {
return TimeoutSettings.computeDeadline(options.timeout, this.timeout());
}
static computeDeadline(timeout: number | undefined, defaultValue = DEFAULT_TIMEOUT): number {
if (typeof timeout !== 'number')
timeout = defaultValue;
return timeout ? helper.monotonicTime() + timeout : Number.MAX_SAFE_INTEGER;
static timeout(options: TimeoutOptions): number {
if (typeof options.timeout === 'number')
return options.timeout;
return DEFAULT_TIMEOUT;
}
}

View File

@ -154,7 +154,7 @@ export class WebSocketTransport implements ConnectionTransport {
this._ws = new WebSocket(url, [], {
perMessageDeflate: false,
maxPayload: 256 * 1024 * 1024, // 256Mb,
handshakeTimeout: helper.timeUntilDeadline(progress.deadline)
handshakeTimeout: progress.timeUntilDeadline(),
});
this._progress = progress;
// The 'ws' module in node sometimes sends us multiple messages in a single task.