Skip to content

Commit

Permalink
fix(actions): wait for some time before retrying the action (#4001)
Browse files Browse the repository at this point in the history
This saves some CPU cycles while waiting for the page to
change the state, e.g. for animations to complete.

Note that retrying logic is only applicable in rare
circumstances like unexpected scroll in the middle of an
action, or some overlay blocking the click. Usually,
action times out in this cases while retrying.
  • Loading branch information
dgozman authored Sep 29, 2020
1 parent 109688a commit b3497b3
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
17 changes: 14 additions & 3 deletions src/server/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,22 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {

async _retryPointerAction(progress: Progress, actionName: string, waitForEnabled: boolean, action: (point: types.Point) => Promise<void>,
options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
let first = true;
let retry = 0;
// We progressively wait longer between retries, up to 500ms.
const waitTime = [0, 20, 100, 500];
while (progress.isRunning()) {
progress.log(`${first ? 'attempting' : 'retrying'} ${actionName} action`);
if (retry) {
progress.log(`retrying ${actionName} action, attempt #${retry}`);
const timeout = waitTime[Math.min(retry - 1, waitTime.length - 1)];
if (timeout) {
progress.log(` waiting ${timeout}ms`);
await this._evaluateInUtility(([injected, node, timeout]) => new Promise(f => setTimeout(f, timeout)), timeout);
}
} else {
progress.log(`attempting ${actionName} action`);
}
const result = await this._performPointerAction(progress, actionName, waitForEnabled, action, options);
first = false;
++retry;
if (result === 'error:notvisible') {
if (options.force)
throw new Error('Element is not visible');
Expand Down
1 change: 1 addition & 0 deletions test/click-timeout-3.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ it('should timeout waiting for hit target', async ({page, server}) => {
expect(error.message).toContain('elementHandle.click: Timeout 5000ms exceeded.');
expect(error.message).toContain('<div id="blocker"></div> intercepts pointer events');
expect(error.message).toContain('retrying click action');
expect(error.message).toContain('waiting 500ms');
});

it('should report wrong hit target subtree', async ({page, server}) => {
Expand Down

0 comments on commit b3497b3

Please sign in to comment.