Skip to content

chore: add some progress apis #36189

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: add some progress apis #36189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
87 changes: 36 additions & 51 deletions packages/playwright-core/src/server/frames.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import * as network from './network';
import { Page } from './page';
import { ProgressController } from './progress';
import * as types from './types';
import { LongStandingScope, asLocator, assert, constructURLBasedOnBaseURL, makeWaitForNextTask, monotonicTime, renderTitleForCall } from '../utils';
import { LongStandingScope, asLocator, assert, constructURLBasedOnBaseURL, makeWaitForNextTask, renderTitleForCall } from '../utils';
import { isSessionClosedError } from './protocolError';
import { debugLogger } from './utils/debugLogger';
import { eventsHelper } from './utils/eventsHelper';
Expand Down Expand Up @@ -1383,73 +1383,58 @@ export class Frame extends SdkObject {
}

async expect(metadata: CallMetadata, selector: string, options: FrameExpectParams): Promise<{ matches: boolean, received?: any, log?: string[], timedOut?: boolean }> {
const result = await this._expectImpl(metadata, selector, options);
// Library mode special case for the expect errors which are return values, not exceptions.
if (result.matches === options.isNot)
metadata.error = { error: { name: 'Expect', message: 'Expect failed' } };
return result;
}
const controller = new ProgressController(metadata, this);
return await controller.run(async progress => {
const lastIntermediateResult: { received?: any, isSet: boolean } = { isSet: false };

private async _expectImpl(metadata: CallMetadata, selector: string, options: FrameExpectParams): Promise<{ matches: boolean, received?: any, log?: string[], timedOut?: boolean }> {
const lastIntermediateResult: { received?: any, isSet: boolean } = { isSet: false };
try {
let timeout = options.timeout;
const start = timeout > 0 ? monotonicTime() : 0;
progress.setCustomErrorHandler((e: any) => {
// Q: Why not throw upon isSessionClosedError(e) as in other places?
// A: We want user to receive a friendly message containing the last intermediate result.
if (js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e))
throw e;
const result: { matches: boolean, received?: any, log?: string[], timedOut?: boolean } = { matches: options.isNot, log: compressCallLog(metadata.log) };
if (lastIntermediateResult.isSet)
result.received = lastIntermediateResult.received;
if (e instanceof TimeoutError)
result.timedOut = true;
// Library mode special case for the expect errors which are return values, not exceptions.
metadata.error = { error: { name: 'Expect', message: 'Expect failed' } };
return result;
});

// Step 1: perform locator handlers checkpoint with a specified timeout.
await (new ProgressController(metadata, this)).run(async progress => {
progress.log(`${renderTitleForCall(metadata)}${timeout ? ` with timeout ${timeout}ms` : ''}`);
progress.log(`waiting for ${this._asLocator(selector)}`);
await this._page.performActionPreChecks(progress);
}, timeout);
progress.log(`${renderTitleForCall(metadata)}${options.timeout ? ` with timeout ${options.timeout}ms` : ''}`);
progress.log(`waiting for ${this._asLocator(selector)}`);
await this._page.performActionPreChecks(progress);

// Step 2: perform one-shot expect check without a timeout.
// Supports the case of `expect(locator).toBeVisible({ timeout: 1 })`
// that should succeed when the locator is already visible.
progress.pause();
try {
const resultOneShot = await (new ProgressController(metadata, this)).run(async progress => {
return await this._expectInternal(progress, selector, options, lastIntermediateResult);
});
const resultOneShot = await this._expectInternal(progress, selector, options, lastIntermediateResult);
if (resultOneShot.matches !== options.isNot)
return resultOneShot;
} catch (e) {
if (js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e))
throw e;
// Ignore any other errors from one-shot, we'll handle them during retries.
}
if (timeout > 0) {
const elapsed = monotonicTime() - start;
timeout -= elapsed;
}
if (timeout < 0)
return { matches: options.isNot, log: compressCallLog(metadata.log), timedOut: true, received: lastIntermediateResult.received };
progress.resume();

// Step 3: auto-retry expect with increasing timeouts. Bounded by the total remaining time.
return await (new ProgressController(metadata, this)).run(async progress => {
return await this.retryWithProgressAndTimeouts(progress, [100, 250, 500, 1000], async continuePolling => {
await this._page.performActionPreChecks(progress);
const { matches, received } = await this._expectInternal(progress, selector, options, lastIntermediateResult);
if (matches === options.isNot) {
// Keep waiting in these cases:
// expect(locator).conditionThatDoesNotMatch
// expect(locator).not.conditionThatDoesMatch
return continuePolling;
}
return { matches, received };
});
}, timeout);
} catch (e) {
// Q: Why not throw upon isSessionClosedError(e) as in other places?
// A: We want user to receive a friendly message containing the last intermediate result.
if (js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e))
throw e;
const result: { matches: boolean, received?: any, log?: string[], timedOut?: boolean } = { matches: options.isNot, log: compressCallLog(metadata.log) };
if (lastIntermediateResult.isSet)
result.received = lastIntermediateResult.received;
if (e instanceof TimeoutError)
result.timedOut = true;
return result;
}
return await this.retryWithProgressAndTimeouts(progress, [100, 250, 500, 1000], async continuePolling => {
await this._page.performActionPreChecks(progress);
const { matches, received } = await this._expectInternal(progress, selector, options, lastIntermediateResult);
if (matches === options.isNot) {
// Keep waiting in these cases:
// expect(locator).conditionThatDoesNotMatch
// expect(locator).not.conditionThatDoesMatch
return continuePolling;
}
return { matches, received };
});
}, options.timeout);
}

private async _expectInternal(progress: Progress, selector: string, options: FrameExpectParams, lastIntermediateResult: { received?: any, isSet: boolean }) {
Expand Down
32 changes: 17 additions & 15 deletions packages/playwright-core/src/server/page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,8 +623,24 @@ export class Page extends SdkObject {
intermediateResult = { errorMessage: comparatorResult.errorMessage, diff: comparatorResult.diff, actual, previous };
return false;
};
const errorHandler = (e: any) => {
// Q: Why not throw upon isSessionClosedError(e) as in other places?
// A: We want user to receive a friendly diff between actual and expected/previous.
if (js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e))
throw e;
let errorMessage = e.message;
if (e instanceof TimeoutError && intermediateResult?.previous)
errorMessage = `Failed to take two consecutive stable screenshots.`;
return {
log: compressCallLog(e.message ? [...metadata.log, e.message] : metadata.log),
...intermediateResult,
errorMessage,
timedOut: (e instanceof TimeoutError),
};
};
const callTimeout = options.timeout;
return controller.run(async progress => {
progress.setCustomErrorHandler(errorHandler);
let actual: Buffer | undefined;
let previous: Buffer | undefined;
const pollIntervals = [0, 100, 250, 500];
Expand Down Expand Up @@ -673,21 +689,7 @@ export class Page extends SdkObject {
return {};
}
throw new Error(intermediateResult!.errorMessage);
}, callTimeout).catch(e => {
// Q: Why not throw upon isSessionClosedError(e) as in other places?
// A: We want user to receive a friendly diff between actual and expected/previous.
if (js.isJavaScriptErrorInEvaluate(e) || isInvalidSelectorError(e))
throw e;
let errorMessage = e.message;
if (e instanceof TimeoutError && intermediateResult?.previous)
errorMessage = `Failed to take two consecutive stable screenshots.`;
return {
log: compressCallLog(e.message ? [...metadata.log, e.message] : metadata.log),
...intermediateResult,
errorMessage,
timedOut: (e instanceof TimeoutError),
};
});
}, callTimeout);
}

async screenshot(metadata: CallMetadata, options: ScreenshotOptions & types.TimeoutOptions): Promise<Buffer> {
Expand Down
30 changes: 25 additions & 5 deletions packages/playwright-core/src/server/progress.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ export interface Progress {
cleanupWhenAborted(cleanup: () => any): void;
throwIfAborted(): void;
metadata: CallMetadata;
setCustomErrorHandler(handler: (error: Error) => any): void;
pause(): void;
resume(): void;
}

export class ProgressController {
Expand All @@ -40,6 +43,7 @@ export class ProgressController {
private _state: 'before' | 'running' | 'aborted' | 'finished' = 'before';
private _deadline: number = 0;
private _timeout: number = 0;
private _errorHandler: (error: Error) => any;
readonly metadata: CallMetadata;
readonly instrumentation: Instrumentation;
readonly sdkObject: SdkObject;
Expand All @@ -49,6 +53,7 @@ export class ProgressController {
this.sdkObject = sdkObject;
this.instrumentation = sdkObject.instrumentation;
this._forceAbortPromise.catch(e => null); // Prevent unhandled promise rejection.
this._errorHandler = error => { throw error; }; // Default error handler does not handle the error.
}

setLogName(logName: LogName) {
Expand All @@ -68,6 +73,8 @@ export class ProgressController {
assert(this._state === 'before');
this._state = 'running';
this.sdkObject.attribution.context?._activeProgressControllers.add(this);
let timer: NodeJS.Timeout | undefined;
const timeoutError = new TimeoutError(`Timeout ${this._timeout}ms exceeded.`);

const progress: Progress = {
log: message => {
Expand All @@ -88,23 +95,36 @@ export class ProgressController {
if (this._state === 'aborted')
throw new AbortedError();
},
metadata: this.metadata
metadata: this.metadata,
setCustomErrorHandler: handler => this._errorHandler = handler,
pause: () => {
if (timer) {
clearTimeout(timer);
timer = undefined;
}
},
resume: () => {
const remaining = progress.timeUntilDeadline();
if (remaining <= 0)
this._forceAbortPromise.reject(timeoutError);
else
timer = setTimeout(() => this._forceAbortPromise.reject(timeoutError), remaining);
},
};

const timeoutError = new TimeoutError(`Timeout ${this._timeout}ms exceeded.`);
const timer = setTimeout(() => this._forceAbortPromise.reject(timeoutError), progress.timeUntilDeadline());
try {
progress.resume();
const promise = task(progress);
const result = await Promise.race([promise, this._forceAbortPromise]);
this._state = 'finished';
return result;
} catch (e) {
this._state = 'aborted';
await Promise.all(this._cleanups.splice(0).map(runCleanup));
throw e;
return this._errorHandler(e);
} finally {
this.sdkObject.attribution.context?._activeProgressControllers.delete(this);
clearTimeout(timer);
progress.pause();
}
}
}
Expand Down
Loading