Skip to content

Commit d980ed7

Browse files
dgozmandgozman
authored
chore: introduce Progress concept (#2350)
A progress roughly corresponds to an api call. It is used: - to collect logs related to the call; - to handle timeout; - to provide "cancellation token" behavior so that cancelable process can either early-exit with progress.throwIfCanceled() or race against it with progress.race(); - to ensure resources are disposed in the case of a failure with progress.cleanupWhenCanceled(); - (possibly) to log api calls if needed; - (in the future) to augment async stacks.
1 parent 4bd9b30 commit d980ed7

File tree

10 files changed

+243
-222
lines changed

10 files changed

+243
-222
lines changed

‎src/logger.ts

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -57,21 +57,6 @@ export class RootLogger implements InnerLogger {
5757
if (this._logger.isEnabled(log.name, log.severity || 'info'))
5858
this._logger.log(log.name, log.severity || 'info', message, args, log.color ? { color: log.color } : {});
5959
}
60-
61-
startLaunchRecording() {
62-
this._logger.add(`launch`, new RecordingLogger('browser'));
63-
}
64-
65-
launchRecording(): string {
66-
const logger = this._logger.get(`launch`) as RecordingLogger;
67-
if (logger)
68-
return logger.recording();
69-
return '';
70-
}
71-
72-
stopLaunchRecording() {
73-
this._logger.remove(`launch`);
74-
}
7560
}
7661

7762
const colorMap = new Map<string, number>([
@@ -113,27 +98,6 @@ class MultiplexingLogger implements Logger {
11398
}
11499
}
115100

116-
export class RecordingLogger implements Logger {
117-
private _prefix: string;
118-
private _recording: string[] = [];
119-
120-
constructor(prefix: string) {
121-
this._prefix = prefix;
122-
}
123-
124-
isEnabled(name: string, severity: LoggerSeverity): boolean {
125-
return name.startsWith(this._prefix);
126-
}
127-
128-
log(name: string, severity: LoggerSeverity, message: string | Error, args: any[], hints: { color?: string }) {
129-
this._recording.push(String(message));
130-
}
131-
132-
recording(): string {
133-
return this._recording.join('\n');
134-
}
135-
}
136-
137101
class DebugLogger implements Logger {
138102
private _debuggers = new Map<string, debug.IDebugger>();
139103

‎src/progress.ts

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/**
2+
* Copyright (c) Microsoft Corporation.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { InnerLogger, Log } from './logger';
18+
import { TimeoutError } from './errors';
19+
import { helper } from './helper';
20+
import * as types from './types';
21+
import { DEFAULT_TIMEOUT, TimeoutSettings } from './timeoutSettings';
22+
import { getCurrentApiCall, rewriteErrorMessage } from './debug/stackTrace';
23+
24+
class AbortError extends Error {}
25+
26+
export class Progress {
27+
static async runCancelableTask<T>(task: (progress: Progress) => Promise<T>, timeoutOptions: types.TimeoutOptions, logger: InnerLogger, apiName?: string): Promise<T> {
28+
let resolveCancelation = () => {};
29+
const progress = new Progress(timeoutOptions, logger, new Promise(resolve => resolveCancelation = resolve), apiName);
30+
31+
const { timeout = DEFAULT_TIMEOUT } = timeoutOptions;
32+
const timeoutError = new TimeoutError(`Timeout ${timeout}ms exceeded during ${progress.apiName}.`);
33+
let rejectWithTimeout: (error: Error) => void;
34+
const timeoutPromise = new Promise<T>((resolve, x) => rejectWithTimeout = x);
35+
const timeoutTimer = setTimeout(() => rejectWithTimeout(timeoutError), helper.timeUntilDeadline(progress.deadline));
36+
37+
try {
38+
const promise = task(progress);
39+
const result = await Promise.race([promise, timeoutPromise]);
40+
clearTimeout(timeoutTimer);
41+
progress._running = false;
42+
progress._logRecording = [];
43+
return result;
44+
} catch (e) {
45+
resolveCancelation();
46+
rewriteErrorMessage(e, e.message + formatLogRecording(progress._logRecording, progress.apiName));
47+
clearTimeout(timeoutTimer);
48+
progress._running = false;
49+
progress._logRecording = [];
50+
await Promise.all(progress._cleanups.splice(0).map(cleanup => runCleanup(cleanup)));
51+
throw e;
52+
}
53+
}
54+
55+
readonly apiName: string;
56+
readonly deadline: number; // To be removed?
57+
readonly _canceled: Promise<any>;
58+
59+
private _logger: InnerLogger;
60+
private _logRecording: string[] = [];
61+
private _cleanups: (() => any)[] = [];
62+
private _running = true;
63+
64+
constructor(options: types.TimeoutOptions, logger: InnerLogger, canceled: Promise<any>, apiName?: string) {
65+
this.apiName = apiName || getCurrentApiCall();
66+
this.deadline = TimeoutSettings.computeDeadline(options.timeout);
67+
this._canceled = canceled;
68+
this._logger = logger;
69+
}
70+
71+
cleanupWhenCanceled(cleanup: () => any) {
72+
if (this._running)
73+
this._cleanups.push(cleanup);
74+
else
75+
runCleanup(cleanup);
76+
}
77+
78+
throwIfCanceled() {
79+
if (!this._running)
80+
throw new AbortError();
81+
}
82+
83+
race<T>(promise: Promise<T>, cleanup?: () => any): Promise<T> {
84+
const canceled = this._canceled.then(async error => {
85+
if (cleanup)
86+
await runCleanup(cleanup);
87+
throw error;
88+
});
89+
const success = promise.then(result => {
90+
cleanup = undefined;
91+
return result;
92+
});
93+
return Promise.race<T>([success, canceled]);
94+
}
95+
96+
log(log: Log, message: string | Error): void {
97+
if (this._running)
98+
this._logRecording.push(message.toString());
99+
this._logger._log(log, message);
100+
}
101+
}
102+
103+
async function runCleanup(cleanup: () => any) {
104+
try {
105+
await cleanup();
106+
} catch (e) {
107+
}
108+
}
109+
110+
function formatLogRecording(log: string[], name: string): string {
111+
name = ` ${name} logs `;
112+
const headerLength = 60;
113+
const leftLength = (headerLength - name.length) / 2;
114+
const rightLength = headerLength - name.length - leftLength;
115+
return `\n${'='.repeat(leftLength)}${name}${'='.repeat(rightLength)}\n${log.join('\n')}\n${'='.repeat(headerLength)}`;
116+
}

‎src/server/browserServer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
*/
1616

1717
import { ChildProcess } from 'child_process';
18-
import { EventEmitter } from 'events';
1918
import { helper } from '../helper';
19+
import { EventEmitter } from 'events';
2020

2121
export class WebSocketWrapper {
2222
readonly wsEndpoint: string;

‎src/server/browserType.ts

Lines changed: 46 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,11 @@ import * as browserPaths from '../install/browserPaths';
2424
import { Logger, RootLogger, InnerLogger } from '../logger';
2525
import { ConnectionTransport, WebSocketTransport } from '../transport';
2626
import { BrowserBase, BrowserOptions, Browser } from '../browser';
27-
import { assert, helper } from '../helper';
28-
import { TimeoutSettings } from '../timeoutSettings';
27+
import { assert } from '../helper';
2928
import { launchProcess, Env, waitForLine } from './processLauncher';
3029
import { Events } from '../events';
31-
import { rewriteErrorMessage } from '../debug/stackTrace';
32-
import { TimeoutError } from '../errors';
3330
import { PipeTransport } from './pipeTransport';
31+
import { Progress } from '../progress';
3432

3533
export type BrowserArgOptions = {
3634
headless?: boolean,
@@ -102,56 +100,36 @@ export abstract class BrowserTypeBase implements BrowserType {
102100
async launch(options: LaunchOptions = {}): Promise<Browser> {
103101
assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead');
104102
assert(!(options as any).port, 'Cannot specify a port without launching as a server.');
105-
return this._innerLaunch(options, undefined);
103+
const logger = new RootLogger(options.logger);
104+
const browser = await Progress.runCancelableTask(progress => this._innerLaunch(progress, options, logger, undefined), options, logger);
105+
return browser;
106106
}
107107

108108
async launchPersistentContext(userDataDir: string, options: LaunchOptions & PersistentContextOptions = {}): Promise<BrowserContext> {
109109
assert(!(options as any).port, 'Cannot specify a port without launching as a server.');
110110
const persistent = validatePersistentContextOptions(options);
111-
const browser = await this._innerLaunch(options, persistent, userDataDir);
112-
return browser._defaultContext!;
113-
}
114-
115-
async _innerLaunch(options: LaunchOptions, persistent: PersistentContextOptions | undefined, userDataDir?: string): Promise<BrowserBase> {
116-
const deadline = TimeoutSettings.computeDeadline(options.timeout);
117111
const logger = new RootLogger(options.logger);
118-
logger.startLaunchRecording();
119-
120-
let browserServer: BrowserServer | undefined;
121-
try {
122-
const launched = await this._launchServer(options, !!persistent, logger, deadline, userDataDir);
123-
browserServer = launched.browserServer;
124-
const browserOptions: BrowserOptions = {
125-
slowMo: options.slowMo,
126-
persistent,
127-
headful: !processBrowserArgOptions(options).headless,
128-
logger,
129-
downloadsPath: launched.downloadsPath,
130-
ownedServer: browserServer,
131-
};
132-
copyTestHooks(options, browserOptions);
133-
const hasCustomArguments = !!options.ignoreDefaultArgs && !Array.isArray(options.ignoreDefaultArgs);
134-
const promise = this._innerCreateBrowser(launched.transport, browserOptions, hasCustomArguments);
135-
const browser = await helper.waitWithDeadline(promise, 'the browser to launch', deadline, 'pw:browser*');
136-
return browser;
137-
} catch (e) {
138-
rewriteErrorMessage(e, e.message + '\n=============== Process output during launch: ===============\n' +
139-
logger.launchRecording() +
140-
'\n=============================================================');
141-
if (browserServer)
142-
await browserServer._closeOrKill(deadline);
143-
throw e;
144-
} finally {
145-
logger.stopLaunchRecording();
146-
}
112+
const browser = await Progress.runCancelableTask(progress => this._innerLaunch(progress, options, logger, persistent, userDataDir), options, logger);
113+
return browser._defaultContext!;
147114
}
148115

149-
async _innerCreateBrowser(transport: ConnectionTransport, browserOptions: BrowserOptions, hasCustomArguments: boolean): Promise<BrowserBase> {
150-
if ((browserOptions as any).__testHookBeforeCreateBrowser)
151-
await (browserOptions as any).__testHookBeforeCreateBrowser();
116+
async _innerLaunch(progress: Progress, options: LaunchOptions, logger: RootLogger, persistent: PersistentContextOptions | undefined, userDataDir?: string): Promise<BrowserBase> {
117+
const { browserServer, downloadsPath, transport } = await this._launchServer(progress, options, !!persistent, logger, userDataDir);
118+
if ((options as any).__testHookBeforeCreateBrowser)
119+
await (options as any).__testHookBeforeCreateBrowser();
120+
const browserOptions: BrowserOptions = {
121+
slowMo: options.slowMo,
122+
persistent,
123+
headful: !processBrowserArgOptions(options).headless,
124+
logger,
125+
downloadsPath,
126+
ownedServer: browserServer,
127+
};
128+
copyTestHooks(options, browserOptions);
152129
const browser = await this._connectToTransport(transport, browserOptions);
153130
// We assume no control when using custom arguments, and do not prepare the default context in that case.
154-
if (browserOptions.persistent && !hasCustomArguments)
131+
const hasCustomArguments = !!options.ignoreDefaultArgs && !Array.isArray(options.ignoreDefaultArgs);
132+
if (persistent && !hasCustomArguments)
155133
await browser._defaultContext!._loadDefaultContext();
156134
return browser;
157135
}
@@ -160,44 +138,26 @@ export abstract class BrowserTypeBase implements BrowserType {
160138
assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launchServer`. Use `browserType.launchPersistentContext` instead');
161139
const { port = 0 } = options;
162140
const logger = new RootLogger(options.logger);
163-
const { browserServer, transport } = await this._launchServer(options, false, logger, TimeoutSettings.computeDeadline(options.timeout));
164-
browserServer._webSocketWrapper = this._wrapTransportWithWebSocket(transport, logger, port);
165-
return browserServer;
141+
return Progress.runCancelableTask(async progress => {
142+
const { browserServer, transport } = await this._launchServer(progress, options, false, logger);
143+
browserServer._webSocketWrapper = this._wrapTransportWithWebSocket(transport, logger, port);
144+
return browserServer;
145+
}, options, logger);
166146
}
167147

168148
async connect(options: ConnectOptions): Promise<Browser> {
169-
const deadline = TimeoutSettings.computeDeadline(options.timeout);
170149
const logger = new RootLogger(options.logger);
171-
logger.startLaunchRecording();
172-
173-
let transport: ConnectionTransport | undefined;
174-
try {
175-
transport = await WebSocketTransport.connect(options.wsEndpoint, logger, deadline);
176-
const browserOptions: BrowserOptions = {
177-
slowMo: options.slowMo,
178-
logger,
179-
};
180-
copyTestHooks(options, browserOptions);
181-
const promise = this._innerCreateBrowser(transport, browserOptions, false);
182-
const browser = await helper.waitWithDeadline(promise, 'connect to browser', deadline, 'pw:browser*');
183-
logger.stopLaunchRecording();
150+
return Progress.runCancelableTask(async progress => {
151+
const transport = await WebSocketTransport.connect(progress, options.wsEndpoint);
152+
progress.cleanupWhenCanceled(() => transport.closeAndWait());
153+
if ((options as any).__testHookBeforeCreateBrowser)
154+
await (options as any).__testHookBeforeCreateBrowser();
155+
const browser = await this._connectToTransport(transport, { slowMo: options.slowMo, logger });
184156
return browser;
185-
} catch (e) {
186-
rewriteErrorMessage(e, e.message + '\n=============== Process output during connect: ===============\n' +
187-
logger.launchRecording() +
188-
'\n=============================================================');
189-
try {
190-
if (transport)
191-
transport.close();
192-
} catch (e) {
193-
}
194-
throw e;
195-
} finally {
196-
logger.stopLaunchRecording();
197-
}
157+
}, options, logger);
198158
}
199159

200-
private async _launchServer(options: LaunchServerOptions, isPersistent: boolean, logger: RootLogger, deadline: number, userDataDir?: string): Promise<{ browserServer: BrowserServer, downloadsPath: string, transport: ConnectionTransport }> {
160+
private async _launchServer(progress: Progress, options: LaunchServerOptions, isPersistent: boolean, logger: RootLogger, userDataDir?: string): Promise<{ browserServer: BrowserServer, downloadsPath: string, transport: ConnectionTransport }> {
201161
const {
202162
ignoreDefaultArgs = false,
203163
args = [],
@@ -238,7 +198,7 @@ export abstract class BrowserTypeBase implements BrowserType {
238198
handleSIGINT,
239199
handleSIGTERM,
240200
handleSIGHUP,
241-
logger,
201+
progress,
242202
pipe: !this._webSocketRegexNotPipe,
243203
tempDirectories,
244204
attemptToGracefullyClose: async () => {
@@ -254,23 +214,17 @@ export abstract class BrowserTypeBase implements BrowserType {
254214
browserServer.emit(Events.BrowserServer.Close, exitCode, signal);
255215
},
256216
});
257-
258-
try {
259-
if (this._webSocketRegexNotPipe) {
260-
const timeoutError = new TimeoutError(`Timed out while trying to connect to the browser!`);
261-
const match = await waitForLine(launchedProcess, launchedProcess.stdout, this._webSocketRegexNotPipe, helper.timeUntilDeadline(deadline), timeoutError);
262-
const innerEndpoint = match[1];
263-
transport = await WebSocketTransport.connect(innerEndpoint, logger, deadline);
264-
} else {
265-
const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
266-
transport = new PipeTransport(stdio[3], stdio[4], logger);
267-
}
268-
} catch (e) {
269-
// If we can't establish a connection, kill the process and exit.
270-
helper.killProcess(launchedProcess);
271-
throw e;
272-
}
273217
browserServer = new BrowserServer(launchedProcess, gracefullyClose, kill);
218+
progress.cleanupWhenCanceled(() => browserServer && browserServer._closeOrKill(progress.deadline));
219+
220+
if (this._webSocketRegexNotPipe) {
221+
const match = await waitForLine(progress, launchedProcess, launchedProcess.stdout, this._webSocketRegexNotPipe);
222+
const innerEndpoint = match[1];
223+
transport = await WebSocketTransport.connect(progress, innerEndpoint);
224+
} else {
225+
const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
226+
transport = new PipeTransport(stdio[3], stdio[4], logger);
227+
}
274228
return { browserServer, downloadsPath, transport };
275229
}
276230

0 commit comments

Comments
 (0)