From d4c466868b4a82c3be7ebd697698dd9c129af98d Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 12 Jun 2020 14:59:26 -0700 Subject: [PATCH] chore: explicitly plumb various errors through the retries (#2554) We are about to have more errors, so explicit plumbing helps with visibility. --- src/chromium/crPage.ts | 11 +- src/dom.ts | 223 ++++++++++++++++++++------------- src/errors.ts | 6 - src/firefox/ffPage.ts | 7 +- src/frames.ts | 29 ++--- src/injected/injectedScript.ts | 74 +++++------ src/page.ts | 2 +- src/types.ts | 5 +- src/webkit/wkPage.ts | 9 +- test/click.spec.js | 2 - test/waittask.spec.js | 12 +- 11 files changed, 204 insertions(+), 176 deletions(-) diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index 88de64f6ad..88f9d3693c 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -35,7 +35,6 @@ import { CRPDF } from './crPdf'; import { CRBrowserContext } from './crBrowser'; import * as types from '../types'; import { ConsoleMessage } from '../console'; -import { NotConnectedError } from '../errors'; import * as sourceMap from '../utils/sourceMap'; import { rewriteErrorMessage } from '../utils/stackTrace'; @@ -245,7 +244,7 @@ export class CRPage implements PageDelegate { return this._sessionForHandle(handle)._getBoundingBox(handle); } - async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'> { + async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'notvisible' | 'notconnected' | 'done'> { return this._sessionForHandle(handle)._scrollRectIntoViewIfNeeded(handle, rect); } @@ -832,15 +831,15 @@ class FrameSession { return {x, y, width, height}; } - async _scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'> { + async _scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'notvisible' | 'notconnected' | 'done'> { return await this._client.send('DOM.scrollIntoViewIfNeeded', { objectId: handle._objectId, rect, - }).then(() => 'success' as const).catch(e => { + }).then(() => 'done' as const).catch(e => { if (e instanceof Error && e.message.includes('Node does not have a layout object')) - return 'invisible'; + return 'notvisible'; if (e instanceof Error && e.message.includes('Node is detached from document')) - throw new NotConnectedError(); + return 'notconnected'; throw e; }); } diff --git a/src/dom.ts b/src/dom.ts index 3aa057743c..9b549689ee 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -28,7 +28,6 @@ import * as js from './javascript'; import { Page } from './page'; import { selectors } from './selectors'; import * as types from './types'; -import { NotConnectedError } from './errors'; import { apiLog } from './logger'; import { Progress, runAbortableTask } from './progress'; import DebugScript from './debug/injected/debugScript'; @@ -208,15 +207,15 @@ export class ElementHandle extends js.JSHandle { injected.dispatchEvent(node, type, eventInit), { type, eventInit }); } - async _scrollRectIntoViewIfNeeded(rect?: types.Rect): Promise<'success' | 'invisible'> { + async _scrollRectIntoViewIfNeeded(rect?: types.Rect): Promise<'notvisible' | 'notconnected' | 'done'> { return await this._page._delegate.scrollRectIntoViewIfNeeded(this, rect); } async scrollIntoViewIfNeeded() { - await this._scrollRectIntoViewIfNeeded(); + throwIfNotConnected(await this._scrollRectIntoViewIfNeeded()); } - private async _clickablePoint(): Promise { + private async _clickablePoint(): Promise { const intersectQuadWithViewport = (quad: types.Quad): types.Quad => { return quad.map(point => ({ x: Math.min(Math.max(point.x, 0), metrics.width), @@ -241,11 +240,11 @@ export class ElementHandle extends js.JSHandle { this._page._delegate.layoutViewport(), ] as const); if (!quads || !quads.length) - return 'invisible'; + return 'notvisible'; const filtered = quads.map(quad => intersectQuadWithViewport(quad)).filter(quad => computeQuadArea(quad) > 1); if (!filtered.length) - return 'outsideviewport'; + return 'notinviewport'; // Return the middle point of the first quad. const result = { x: 0, y: 0 }; for (const point of filtered[0]) { @@ -255,13 +254,13 @@ export class ElementHandle extends js.JSHandle { return result; } - private async _offsetPoint(offset: types.Point): Promise { + private async _offsetPoint(offset: types.Point): Promise { const [box, border] = await Promise.all([ this.boundingBox(), this._evaluateInUtility(([injected, node]) => injected.getElementBorderWidth(node), {}).catch(e => {}), ]); if (!box || !border) - return 'invisible'; + return 'notvisible'; // Make point relative to the padding box to align with offsetX/offsetY. return { x: box.x + border.left + offset.x, @@ -269,60 +268,73 @@ export class ElementHandle extends js.JSHandle { }; } - async _retryPointerAction(progress: Progress, action: (point: types.Point) => Promise, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise { + async _retryPointerAction(progress: Progress, action: (point: types.Point) => Promise, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'notconnected' | 'done'> { let first = true; while (progress.isRunning()) { progress.log(apiLog, `${first ? 'attempting' : 'retrying'} ${progress.apiName} action`); const result = await this._performPointerAction(progress, action, options); - if (result === 'done') - return; first = false; + if (result === 'notvisible') { + if (options.force) + throw new Error('Element is not visible'); + progress.log(apiLog, ' element is not visible'); + continue; + } + if (result === 'notinviewport') { + if (options.force) + throw new Error('Element is outside of the viewport'); + progress.log(apiLog, ' element is outside of the viewport'); + continue; + } + if (result === 'nothittarget') { + if (options.force) + throw new Error('Element does not receive pointer events'); + progress.log(apiLog, ' element does not receive pointer events'); + continue; + } + if (result === 'notconnected') + return result; + break; } + return 'done'; } - async _performPointerAction(progress: Progress, action: (point: types.Point) => Promise, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'done' | 'retry'> { + async _performPointerAction(progress: Progress, action: (point: types.Point) => Promise, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'notvisible' | 'notconnected' | 'notinviewport' | 'nothittarget' | 'done'> { const { force = false, position } = options; - if (!force) - await this._waitForDisplayedAtStablePositionAndEnabled(progress); + if (!force) { + const result = await this._waitForDisplayedAtStablePositionAndEnabled(progress); + if (result === 'notconnected') + return 'notconnected'; + } if ((options as any).__testHookAfterStable) await (options as any).__testHookAfterStable(); progress.log(apiLog, ' scrolling into view if needed'); progress.throwIfAborted(); // Avoid action that has side-effects. const scrolled = await this._scrollRectIntoViewIfNeeded(position ? { x: position.x, y: position.y, width: 0, height: 0 } : undefined); - if (scrolled === 'invisible') { - if (force) - throw new Error('Element is not visible'); - progress.log(apiLog, ' element is not visible'); - return 'retry'; - } + if (scrolled === 'notvisible') + return 'notvisible'; + if (scrolled === 'notconnected') + return 'notconnected'; progress.log(apiLog, ' done scrolling'); const maybePoint = position ? await this._offsetPoint(position) : await this._clickablePoint(); - if (maybePoint === 'invisible') { - if (force) - throw new Error('Element is not visible'); - progress.log(apiLog, ' element is not visibile'); - return 'retry'; - } - if (maybePoint === 'outsideviewport') { - if (force) - throw new Error('Element is outside of the viewport'); - progress.log(apiLog, ' element is outside of the viewport'); - return 'retry'; - } + if (maybePoint === 'notvisible') + return 'notvisible'; + if (maybePoint === 'notinviewport') + return 'notinviewport'; const point = roundPoint(maybePoint); if (!force) { if ((options as any).__testHookBeforeHitTarget) await (options as any).__testHookBeforeHitTarget(); progress.log(apiLog, ` checking that element receives pointer events at (${point.x},${point.y})`); - const matchesHitTarget = await this._checkHitTargetAt(point); - if (!matchesHitTarget) { - progress.log(apiLog, ' element does not receive pointer events'); - return 'retry'; - } - progress.log(apiLog, ` element does receive pointer events, continuing input action`); + const hitTargetResult = await this._checkHitTargetAt(point); + if (hitTargetResult === 'notconnected') + return 'notconnected'; + if (hitTargetResult === 'nothittarget') + return 'nothittarget'; + progress.log(apiLog, ` element does receive pointer events`); } await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { @@ -347,34 +359,42 @@ export class ElementHandle extends js.JSHandle { } hover(options: PointerActionOptions & types.PointerActionWaitOptions = {}): Promise { - return runAbortableTask(progress => this._hover(progress, options), this._page._logger, this._page._timeoutSettings.timeout(options)); + return runAbortableTask(async progress => { + throwIfNotConnected(await this._hover(progress, options)); + }, this._page._logger, this._page._timeoutSettings.timeout(options)); } - _hover(progress: Progress, options: PointerActionOptions & types.PointerActionWaitOptions): Promise { + _hover(progress: Progress, options: PointerActionOptions & types.PointerActionWaitOptions): Promise<'notconnected' | 'done'> { return this._retryPointerAction(progress, point => this._page.mouse.move(point.x, point.y), options); } click(options: ClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise { - return runAbortableTask(progress => this._click(progress, options), this._page._logger, this._page._timeoutSettings.timeout(options)); + return runAbortableTask(async progress => { + throwIfNotConnected(await this._click(progress, options)); + }, this._page._logger, this._page._timeoutSettings.timeout(options)); } - _click(progress: Progress, options: ClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise { + _click(progress: Progress, options: ClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'notconnected' | 'done'> { return this._retryPointerAction(progress, point => this._page.mouse.click(point.x, point.y, options), options); } dblclick(options: MultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise { - return runAbortableTask(progress => this._dblclick(progress, options), this._page._logger, this._page._timeoutSettings.timeout(options)); + return runAbortableTask(async progress => { + throwIfNotConnected(await this._dblclick(progress, options)); + }, this._page._logger, this._page._timeoutSettings.timeout(options)); } - _dblclick(progress: Progress, options: MultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise { + _dblclick(progress: Progress, options: MultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'notconnected' | 'done'> { return this._retryPointerAction(progress, point => this._page.mouse.dblclick(point.x, point.y, options), options); } async selectOption(values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[] | null, options: types.NavigatingActionWaitOptions = {}): Promise { - return runAbortableTask(progress => this._selectOption(progress, values, options), this._page._logger, this._page._timeoutSettings.timeout(options)); + return runAbortableTask(async progress => { + return throwIfNotConnected(await this._selectOption(progress, values, options)); + }, this._page._logger, this._page._timeoutSettings.timeout(options)); } - async _selectOption(progress: Progress, values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[] | null, options: types.NavigatingActionWaitOptions): Promise { + async _selectOption(progress: Progress, values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[] | null, options: types.NavigatingActionWaitOptions): Promise { let vals: string[] | ElementHandle[] | types.SelectOption[]; if (values === null) vals = []; @@ -394,61 +414,68 @@ export class ElementHandle extends js.JSHandle { if (option.index !== undefined) assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"'); } - return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { + return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { progress.throwIfAborted(); // Avoid action that has side-effects. const injectedResult = await this._evaluateInUtility(([injected, node, selectOptions]) => injected.selectOptions(node, selectOptions), selectOptions); - return handleInjectedResult(injectedResult); + return throwIfError(injectedResult); }); } async fill(value: string, options: types.NavigatingActionWaitOptions = {}): Promise { - return runAbortableTask(progress => this._fill(progress, value, options), this._page._logger, this._page._timeoutSettings.timeout(options)); + return runAbortableTask(async progress => { + throwIfNotConnected(await this._fill(progress, value, options)); + }, this._page._logger, this._page._timeoutSettings.timeout(options)); } - async _fill(progress: Progress, value: string, options: types.NavigatingActionWaitOptions): Promise { + async _fill(progress: Progress, value: string, options: types.NavigatingActionWaitOptions): Promise<'notconnected' | 'done'> { progress.log(apiLog, `elementHandle.fill("${value}")`); assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"'); - await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { + return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { progress.log(apiLog, ' waiting for element to be visible, enabled and editable'); const poll = await this._evaluateHandleInUtility(([injected, node, value]) => { return injected.waitForEnabledAndFill(node, value); }, value); const pollHandler = new InjectedScriptPollHandler(progress, poll); - const injectedResult = await pollHandler.finish(); - const needsInput = handleInjectedResult(injectedResult); - progress.log(apiLog, ' element is visible, enabled and editable'); + const filled = throwIfError(await pollHandler.finish()); progress.throwIfAborted(); // Avoid action that has side-effects. - if (needsInput) { + if (filled === 'notconnected') + return 'notconnected'; + progress.log(apiLog, ' element is visible, enabled and editable'); + if (filled === 'needsinput') { if (value) await this._page.keyboard.insertText(value); else await this._page.keyboard.press('Delete'); } + return 'done'; }, 'input'); } async selectText(): Promise { return runAbortableTask(async progress => { progress.throwIfAborted(); // Avoid action that has side-effects. - const injectedResult = await this._evaluateInUtility(([injected, node]) => injected.selectText(node), {}); - handleInjectedResult(injectedResult); + const selected = throwIfError(await this._evaluateInUtility(([injected, node]) => injected.selectText(node), {})); + throwIfNotConnected(selected); }, this._page._logger, 0); } async setInputFiles(files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions = {}) { - return runAbortableTask(async progress => this._setInputFiles(progress, files, options), this._page._logger, this._page._timeoutSettings.timeout(options)); + return runAbortableTask(async progress => { + throwIfNotConnected(await this._setInputFiles(progress, files, options)); + }, this._page._logger, this._page._timeoutSettings.timeout(options)); } - async _setInputFiles(progress: Progress, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions) { - const injectedResult = await this._evaluateInUtility(([injected, node]): types.InjectedScriptResult => { + async _setInputFiles(progress: Progress, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions): Promise<'notconnected' | 'done'> { + const multiple = throwIfError(await this._evaluateInUtility(([injected, node]): types.InjectedScriptResult<'notconnected' | boolean> => { if (node.nodeType !== Node.ELEMENT_NODE || (node as Node as Element).tagName !== 'INPUT') - return { status: 'error', error: 'Node is not an HTMLInputElement' }; + return { error: 'Node is not an HTMLInputElement', value: false }; if (!node.isConnected) - return { status: 'notconnected' }; + return { value: 'notconnected' }; const input = node as Node as HTMLInputElement; - return { status: 'success', value: input.multiple }; - }, {}); - const multiple = handleInjectedResult(injectedResult); + return { value: input.multiple }; + }, {})); + if (multiple === 'notconnected') + return 'notconnected'; let ff: string[] | types.FilePayload[]; if (!Array.isArray(files)) ff = [ files ] as string[] | types.FilePayload[]; @@ -472,41 +499,53 @@ export class ElementHandle extends js.JSHandle { progress.throwIfAborted(); // Avoid action that has side-effects. await this._page._delegate.setInputFiles(this as any as ElementHandle, filePayloads); }); + return 'done'; } - async focus() { - return runAbortableTask(progress => this._focus(progress), this._page._logger, 0); + async focus(): Promise { + return runAbortableTask(async progress => { + throwIfNotConnected(await this._focus(progress)); + }, this._page._logger, 0); } - async _focus(progress: Progress) { + async _focus(progress: Progress): Promise<'notconnected' | 'done'> { progress.throwIfAborted(); // Avoid action that has side-effects. - const injectedResult = await this._evaluateInUtility(([injected, node]) => injected.focusNode(node), {}); - handleInjectedResult(injectedResult); + return throwIfError(await this._evaluateInUtility(([injected, node]) => injected.focusNode(node), {})); } - async type(text: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) { - return runAbortableTask(progress => this._type(progress, text, options), this._page._logger, this._page._timeoutSettings.timeout(options)); + async type(text: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}): Promise { + return runAbortableTask(async progress => { + throwIfNotConnected(await this._type(progress, text, options)); + }, this._page._logger, this._page._timeoutSettings.timeout(options)); } - async _type(progress: Progress, text: string, options: { delay?: number } & types.NavigatingActionWaitOptions) { + async _type(progress: Progress, text: string, options: { delay?: number } & types.NavigatingActionWaitOptions): Promise<'notconnected' | 'done'> { progress.log(apiLog, `elementHandle.type("${text}")`); return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { - await this._focus(progress); + const focused = await this._focus(progress); + if (focused === 'notconnected') + return 'notconnected'; progress.throwIfAborted(); // Avoid action that has side-effects. await this._page.keyboard.type(text, options); + return 'done'; }, 'input'); } - async press(key: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) { - return runAbortableTask(progress => this._press(progress, key, options), this._page._logger, this._page._timeoutSettings.timeout(options)); + async press(key: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}): Promise { + return runAbortableTask(async progress => { + throwIfNotConnected(await this._press(progress, key, options)); + }, this._page._logger, this._page._timeoutSettings.timeout(options)); } - async _press(progress: Progress, key: string, options: { delay?: number } & types.NavigatingActionWaitOptions) { + async _press(progress: Progress, key: string, options: { delay?: number } & types.NavigatingActionWaitOptions): Promise<'notconnected' | 'done'> { progress.log(apiLog, `elementHandle.press("${key}")`); return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { - await this._focus(progress); + const focused = await this._focus(progress); + if (focused === 'notconnected') + return 'notconnected'; progress.throwIfAborted(); // Avoid action that has side-effects. await this._page.keyboard.press(key, options); + return 'done'; }, 'input'); } @@ -562,32 +601,32 @@ export class ElementHandle extends js.JSHandle { return result; } - async _waitForDisplayedAtStablePositionAndEnabled(progress: Progress): Promise { + async _waitForDisplayedAtStablePositionAndEnabled(progress: Progress): Promise<'notconnected' | 'done'> { progress.log(apiLog, ' waiting for element to be visible, enabled and not moving'); const rafCount = this._page._delegate.rafCountForStablePosition(); const poll = this._evaluateHandleInUtility(([injected, node, rafCount]) => { return injected.waitForDisplayedAtStablePositionAndEnabled(node, rafCount); }, rafCount); - const pollHandler = new InjectedScriptPollHandler(progress, await poll); - const injectedResult = await pollHandler.finish(); - handleInjectedResult(injectedResult); + const pollHandler = new InjectedScriptPollHandler(progress, await poll); + const result = throwIfError(await pollHandler.finish()); progress.log(apiLog, ' element is visible, enabled and does not move'); + return result; } - async _checkHitTargetAt(point: types.Point): Promise { + async _checkHitTargetAt(point: types.Point): Promise<'notconnected' | 'nothittarget' | 'done'> { const frame = await this.ownerFrame(); if (frame && frame.parentFrame()) { const element = await frame.frameElement(); const box = await element.boundingBox(); if (!box) - throw new NotConnectedError(); + return 'notconnected'; // Translate from viewport coordinates to frame coordinates. point = { x: point.x - box.x, y: point.y - box.y }; } const injectedResult = await this._evaluateInUtility(([injected, node, point]) => { return injected.checkHitTargetAt(node, point); }, point); - return handleInjectedResult(injectedResult); + return throwIfError(injectedResult); } } @@ -668,12 +707,16 @@ export function toFileTransferPayload(files: types.FilePayload[]): types.FileTra })); } -function handleInjectedResult(injectedResult: types.InjectedScriptResult): T { - if (injectedResult.status === 'notconnected') - throw new NotConnectedError(); - if (injectedResult.status === 'error') +function throwIfError(injectedResult: types.InjectedScriptResult): T { + if (injectedResult.error) throw new Error(injectedResult.error); - return injectedResult.value as T; + return injectedResult.value!; +} + +function throwIfNotConnected(result: 'notconnected' | T): T { + if (result === 'notconnected') + throw new Error('Element is not attached to the DOM'); + return result; } function roundPoint(point: types.Point): types.Point { diff --git a/src/errors.ts b/src/errors.ts index fbe67c8a3e..3044091584 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -23,10 +23,4 @@ class CustomError extends Error { } } -export class NotConnectedError extends CustomError { - constructor() { - super('Element is not attached to the DOM'); - } -} - export class TimeoutError extends CustomError {} diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 153285eb2c..882a12d43f 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -31,7 +31,6 @@ import { RawKeyboardImpl, RawMouseImpl } from './ffInput'; import { FFNetworkManager, headersArray } from './ffNetworkManager'; import { Protocol } from './protocol'; import { selectors } from '../selectors'; -import { NotConnectedError } from '../errors'; import { rewriteErrorMessage } from '../utils/stackTrace'; const UTILITY_WORLD_NAME = '__playwright_utility_world__'; @@ -411,14 +410,14 @@ export class FFPage implements PageDelegate { return { x: minX, y: minY, width: maxX - minX, height: maxY - minY }; } - async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'> { + async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'notvisible' | 'notconnected' | 'done'> { return await this._session.send('Page.scrollIntoViewIfNeeded', { frameId: handle._context.frame._id, objectId: handle._objectId, rect, - }).then(() => 'success' as const).catch(e => { + }).then(() => 'done' as const).catch(e => { if (e instanceof Error && e.message.includes('Node is detached from document')) - throw new NotConnectedError(); + return 'notconnected'; throw e; }); } diff --git a/src/frames.ts b/src/frames.ts index e9924c787e..a01425e51e 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -19,7 +19,6 @@ import * as fs from 'fs'; import * as util from 'util'; import { ConsoleMessage } from './console'; import * as dom from './dom'; -import { NotConnectedError } from './errors'; import { Events } from './events'; import { assert, helper, RegisteredListener, assertMaxArguments, debugAssert } from './helper'; import * as js from './javascript'; @@ -452,7 +451,7 @@ export class Frame { throw new Error('options.waitFor is not supported, did you mean options.state?'); const { state = 'visible' } = options; if (!['attached', 'detached', 'visible', 'hidden'].includes(state)) - throw new Error(`Unsupported waitFor option "${state}"`); + throw new Error(`Unsupported state option "${state}"`); const { world, task } = selectors._waitForSelectorTask(selector, state); return runAbortableTask(async progress => { progress.log(apiLog, `waiting for selector "${selector}"${state === 'attached' ? '' : ' to be ' + state}`); @@ -707,23 +706,21 @@ export class Frame { private async _retryWithSelectorIfNotConnected( selector: string, options: types.TimeoutOptions, - action: (progress: Progress, handle: dom.ElementHandle) => Promise): Promise { + action: (progress: Progress, handle: dom.ElementHandle) => Promise): Promise { return runAbortableTask(async progress => { while (progress.isRunning()) { - try { - progress.log(apiLog, `waiting for selector "${selector}"`); - const { world, task } = selectors._waitForSelectorTask(selector, 'attached'); - const handle = await this._scheduleRerunnableTask(progress, world, task); - const element = handle.asElement() as dom.ElementHandle; - progress.cleanupWhenAborted(() => element.dispose()); - const result = await action(progress, element); - element.dispose(); - return result; - } catch (e) { - if (!(e instanceof NotConnectedError)) - throw e; + progress.log(apiLog, `waiting for selector "${selector}"`); + const { world, task } = selectors._waitForSelectorTask(selector, 'attached'); + const handle = await this._scheduleRerunnableTask(progress, world, task); + const element = handle.asElement() as dom.ElementHandle; + progress.cleanupWhenAborted(() => element.dispose()); + const result = await action(progress, element); + element.dispose(); + if (result === 'notconnected') { progress.log(apiLog, 'element was detached from the DOM, retrying'); + continue; } + return result; } return undefined as any; }, this._page._logger, this._page._timeoutSettings.timeout(options)); @@ -803,7 +800,7 @@ export class Frame { else if (helper.isNumber(polling)) assert(polling > 0, 'Cannot poll with non-positive interval: ' + polling); else - throw new Error('Unknown polling options: ' + polling); + throw new Error('Unknown polling option: ' + polling); const predicateBody = helper.isString(pageFunction) ? 'return (' + pageFunction + ')' : 'return (' + pageFunction + ')(arg)'; const task = async (context: dom.FrameExecutionContext) => { const injectedScript = await context.injectedScript(); diff --git a/src/injected/injectedScript.ts b/src/injected/injectedScript.ts index 6ba3f5aa40..dbe2f2c423 100644 --- a/src/injected/injectedScript.ts +++ b/src/injected/injectedScript.ts @@ -200,11 +200,11 @@ export default class InjectedScript { return { left: parseInt(style.borderLeftWidth || '', 10), top: parseInt(style.borderTopWidth || '', 10) }; } - selectOptions(node: Node, optionsToSelect: (Node | types.SelectOption)[]): types.InjectedScriptResult { + selectOptions(node: Node, optionsToSelect: (Node | types.SelectOption)[]): types.InjectedScriptResult { if (node.nodeName.toLowerCase() !== 'select') - return { status: 'error', error: 'Element is not a element.' }; if (!node.isConnected) - return { status: 'notconnected' }; + return { value: 'notconnected' }; const element = node as HTMLSelectElement; const options = Array.from(element.options); @@ -228,16 +228,16 @@ export default class InjectedScript { } element.dispatchEvent(new Event('input', { 'bubbles': true })); element.dispatchEvent(new Event('change', { 'bubbles': true })); - return { status: 'success', value: options.filter(option => option.selected).map(option => option.value) }; + return { value: options.filter(option => option.selected).map(option => option.value) }; } - waitForEnabledAndFill(node: Node, value: string): types.InjectedScriptPoll> { + waitForEnabledAndFill(node: Node, value: string): types.InjectedScriptPoll> { return this.poll('raf', progress => { if (node.nodeType !== Node.ELEMENT_NODE) - return { status: 'error', error: 'Node is not of type HTMLElement' }; + return { error: 'Node is not of type HTMLElement' }; const element = node as HTMLElement; if (!element.isConnected) - return { status: 'notconnected' }; + return { value: 'notconnected' }; if (!this.isVisible(element)) { progress.logRepeating(' element is not visible - waiting...'); return false; @@ -248,11 +248,11 @@ export default class InjectedScript { const kDateTypes = new Set(['date', 'time', 'datetime', 'datetime-local']); const kTextInputTypes = new Set(['', 'email', 'number', 'password', 'search', 'tel', 'text', 'url']); if (!kTextInputTypes.has(type) && !kDateTypes.has(type)) - return { status: 'error', error: 'Cannot fill input of type "' + type + '".' }; + return { error: 'Cannot fill input of type "' + type + '".' }; if (type === 'number') { value = value.trim(); if (isNaN(Number(value))) - return { status: 'error', error: 'Cannot type text into input[type=number].' }; + return { error: 'Cannot type text into input[type=number].' }; } if (input.disabled) { progress.logRepeating(' element is disabled - waiting...'); @@ -267,10 +267,10 @@ export default class InjectedScript { input.focus(); input.value = value; if (input.value !== value) - return { status: 'error', error: `Malformed ${type} "${value}"` }; + return { error: `Malformed ${type} "${value}"` }; element.dispatchEvent(new Event('input', { 'bubbles': true })); element.dispatchEvent(new Event('change', { 'bubbles': true })); - return { status: 'success', value: false }; // We have already changed the value, no need to input it. + return { value: 'done' }; // We have already changed the value, no need to input it. } } else if (element.nodeName.toLowerCase() === 'textarea') { const textarea = element as HTMLTextAreaElement; @@ -283,54 +283,56 @@ export default class InjectedScript { return false; } } else if (!element.isContentEditable) { - return { status: 'error', error: 'Element is not an ,