Skip to content

Commit

Permalink
chore: rename timeouts
Browse files Browse the repository at this point in the history
  • Loading branch information
OrKoN committed May 5, 2023
1 parent e3c067a commit 9b6118a
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 42 deletions.
58 changes: 23 additions & 35 deletions packages/puppeteer-core/src/api/Locator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {AbortError, TimeoutError} from '../common/Errors.js';
import {TimeoutError} from '../common/Errors.js';
import {EventEmitter} from '../common/EventEmitter.js';
import {debugError} from '../common/util.js';
import {isErrorLike} from '../util/ErrorLike.js';
Expand All @@ -34,15 +34,17 @@ export interface LocatorOptions {
* Total timeout for the entire locator operation.
*/
timeout: number;
/**
* Timeout for individual operations inside the locator. On errors the
* operation is retried as long as {@link LocatorOptions.timeout} is not
* exceeded. This timeout should be generally much lower as locating an
* element means multiple asynchronious operations.
*/
operationTimeout: number;
}

/**
* Timeout for individual operations inside the locator. On errors the
* operation is retried as long as {@link LocatorOptions.timeout} is not
* exceeded. This timeout should be generally much lower as locating an
* element means multiple asynchronious operations.
*/
const CONDITION_TIMEOUT = 1_000;
const WAIT_FOR_FUNCTION_DELAY = 100;

/**
* @internal
*/
Expand Down Expand Up @@ -96,7 +98,6 @@ export class Locator extends EventEmitter {
options: LocatorOptions = {
visibility: 'visible',
timeout: page.getDefaultTimeout(),
operationTimeout: 1000,
}
) {
super();
Expand Down Expand Up @@ -132,10 +133,9 @@ export class Locator extends EventEmitter {
async #waitForFunction(
fn: (signal: AbortSignal) => unknown,
signal?: AbortSignal,
timeout = this.#options.operationTimeout
timeout = CONDITION_TIMEOUT
): Promise<void> {
let isActive = true;
let isUserAborted = false;
let controller: AbortController;
// If the loop times out, we abort only the last iteration's controller.
const timeoutId = setTimeout(() => {
Expand All @@ -147,11 +147,11 @@ export class Locator extends EventEmitter {
'abort',
() => {
controller?.abort();
isUserAborted = true;
isActive = false;
},
{once: true}
);
while (isActive && !isUserAborted) {
while (isActive) {
controller = new AbortController();
try {
const result = await fn(controller.signal);
Expand All @@ -167,7 +167,7 @@ export class Locator extends EventEmitter {
continue;
}
// Abort error are ignored as they only affect one iteration.
if (err instanceof AbortError) {
if (err.name === 'AbortError') {
continue;
}
}
Expand All @@ -178,12 +178,10 @@ export class Locator extends EventEmitter {
controller.abort();
}
await new Promise(resolve => {
return setTimeout(resolve, 100);
return setTimeout(resolve, WAIT_FOR_FUNCTION_DELAY);
});
}
if (isUserAborted) {
throw new AbortError(`waitForFunction was aborted.`);
}
signal?.throwIfAborted();
throw new TimeoutError(
`waitForFunction timed out. The timeout is ${timeout}ms.`
);
Expand All @@ -196,25 +194,20 @@ export class Locator extends EventEmitter {
element: ElementHandle,
signal?: AbortSignal
): Promise<void> => {
function checkAbortSignal() {
if (signal?.aborted) {
throw new AbortError(`ensureElementIsInTheViewport was aborted.`);
}
}
// Side-effect: this also checks if it is connected.
const isIntersectingViewport = await element.isIntersectingViewport({
threshold: 0,
});
checkAbortSignal();
signal?.throwIfAborted();
if (!isIntersectingViewport) {
await element.scrollIntoView();
checkAbortSignal();
signal?.throwIfAborted();
await this.#waitForFunction(async () => {
return await element.isIntersectingViewport({
threshold: 0,
});
}, signal);
checkAbortSignal();
signal?.throwIfAborted();
}
};

Expand Down Expand Up @@ -254,7 +247,7 @@ export class Locator extends EventEmitter {
return true;
},
{
timeout: this.#options.operationTimeout,
timeout: CONDITION_TIMEOUT,
signal,
},
element
Expand Down Expand Up @@ -310,32 +303,27 @@ export class Locator extends EventEmitter {
action: (el: ElementHandle) => Promise<void>,
options?: ActionOptions
) {
function checkAbortSignal() {
if (options?.signal?.aborted) {
throw new AbortError(`Locator was aborted.`);
}
}
await this.#waitForFunction(
async signal => {
// 1. Select the element without visibility checks.
const element = await this.#page.waitForSelector(this.#selector, {
visible: false,
timeout: this.#options.operationTimeout,
timeout: this.#options.timeout,
signal,
});
// Retry if no element is found.
if (!element) {
return false;
}
try {
checkAbortSignal();
signal?.throwIfAborted();
// 2. Perform action specific checks.
await Promise.all(
options?.conditions.map(check => {
return check(element, signal);
}) || []
);
checkAbortSignal();
signal?.throwIfAborted();
// 3. Perform the action
this.emit(LocatorEmittedEvents.Action);
await action(element);
Expand Down
12 changes: 5 additions & 7 deletions test/src/locator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import expect from 'expect';
import {TimeoutError, AbortError} from 'puppeteer-core';
import {TimeoutError} from 'puppeteer-core';
import {LocatorEmittedEvents} from 'puppeteer-core/internal/api/Locator.js';
import sinon from 'sinon';

Expand Down Expand Up @@ -181,7 +181,7 @@ describe('Locator', function () {
new TimeoutError('waitForFunction timed out. The timeout is 5000ms.')
);
} finally {
clock?.restore();
clock.restore();
}
});

Expand All @@ -200,7 +200,7 @@ describe('Locator', function () {
new TimeoutError('waitForFunction timed out. The timeout is 5000ms.')
);
} finally {
clock?.restore();
clock.restore();
}
});

Expand All @@ -220,11 +220,9 @@ describe('Locator', function () {
});
clock.tick(2000);
abortController.abort();
await expect(result).rejects.toEqual(
new AbortError('waitForFunction was aborted.')
);
await expect(result).rejects.toThrow(/aborted/);
} finally {
clock?.restore();
clock.restore();
}
});
});
Expand Down

0 comments on commit 9b6118a

Please sign in to comment.