fix(logs): streaming logs from InjectedScriptPoll without exception (#2712)

We used to get undefined messages, because we were mistakenly
fulfilling the logs multiple times.
This commit is contained in:
Dmitry Gozman 2020-06-25 13:13:10 -07:00 committed by GitHub
parent 807dc1f324
commit 5d5cf26a0e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 28 additions and 27 deletions

View file

@ -697,20 +697,17 @@ export class InjectedScriptPollHandler<T> {
// - no unnecessary work in the page; // - no unnecessary work in the page;
// - no possible side effects after progress promsie rejects. // - no possible side effects after progress promsie rejects.
this._progress.cleanupWhenAborted(() => this.cancel()); this._progress.cleanupWhenAborted(() => this.cancel());
this._streamLogs(poll.evaluateHandle(poll => poll.logs)); this._streamLogs();
} }
private _streamLogs(logsPromise: Promise<js.JSHandle<types.InjectedScriptLogs>>) { private async _streamLogs() {
// We continuously get a chunk of logs, stream them to the progress and wait for the next chunk. while (this._poll && this._progress.isRunning()) {
logsPromise.catch(e => null).then(logs => { const messages = await this._poll.evaluate(poll => poll.takeNextLogs()).catch(e => [] as string[]);
if (!logs || !this._poll || !this._progress.isRunning()) if (!this._poll || !this._progress.isRunning())
return; return;
logs.evaluate(logs => logs.current).catch(e => [] as string[]).then(messages => { for (const message of messages)
for (const message of messages) this._progress.logger.info(message);
this._progress.logger.info(message); }
});
this._streamLogs(logs.evaluateHandle(logs => logs.next));
});
} }
async finishHandle(): Promise<js.SmartHandle<T>> { async finishHandle(): Promise<js.SmartHandle<T>> {

View file

@ -158,14 +158,20 @@ export default class InjectedScript {
} }
private _runAbortableTask<T>(task: (progess: types.InjectedScriptProgress) => Promise<T>): types.InjectedScriptPoll<T> { private _runAbortableTask<T>(task: (progess: types.InjectedScriptProgress) => Promise<T>): types.InjectedScriptPoll<T> {
let currentLogs: string[] = []; let unsentLogs: string[] = [];
let logReady = () => {}; let takeNextLogsCallback: ((logs: string[]) => void) | undefined;
const createLogsPromise = () => new Promise<types.InjectedScriptLogs>(fulfill => { const logReady = () => {
logReady = () => { if (!takeNextLogsCallback)
const current = currentLogs; return;
currentLogs = []; takeNextLogsCallback(unsentLogs);
fulfill({ current, next: createLogsPromise() }); unsentLogs = [];
}; takeNextLogsCallback = undefined;
};
const takeNextLogs = () => new Promise<string[]>(fulfill => {
takeNextLogsCallback = fulfill;
if (unsentLogs.length)
logReady();
}); });
let lastLog = ''; let lastLog = '';
@ -173,7 +179,7 @@ export default class InjectedScript {
aborted: false, aborted: false,
log: (message: string) => { log: (message: string) => {
lastLog = message; lastLog = message;
currentLogs.push(message); unsentLogs.push(message);
logReady(); logReady();
}, },
logRepeating: (message: string) => { logRepeating: (message: string) => {
@ -182,14 +188,11 @@ export default class InjectedScript {
}, },
}; };
// It is important to create logs promise before running the poll to capture logs from the first run.
const logs = createLogsPromise();
return { return {
logs, takeNextLogs,
result: task(progress), result: task(progress),
cancel: () => { progress.aborted = true; }, cancel: () => { progress.aborted = true; },
takeLastLogs: () => currentLogs, takeLastLogs: () => unsentLogs,
}; };
} }

View file

@ -143,10 +143,11 @@ export type InjectedScriptProgress = {
logRepeating: (message: string) => void, logRepeating: (message: string) => void,
}; };
export type InjectedScriptLogs = { current: string[], next: Promise<InjectedScriptLogs> };
export type InjectedScriptPoll<T> = { export type InjectedScriptPoll<T> = {
result: Promise<T>, result: Promise<T>,
logs: Promise<InjectedScriptLogs>, // Takes more logs, waiting until at least one message is available.
takeNextLogs: () => Promise<string[]>,
// Takes all current logs without waiting.
takeLastLogs: () => string[], takeLastLogs: () => string[],
cancel: () => void, cancel: () => void,
}; };