From e04e461b7b96962a5af89f6208cf5edf419ee705 Mon Sep 17 00:00:00 2001 From: Peter Hedenskog Date: Tue, 17 Mar 2026 05:32:44 +0100 Subject: [PATCH] Refactor command to simplify try/catch Change-Id: Iacf91cd5ff7a1a116bf6c54c4b73264c61d5caf1 --- lib/core/engine/command/click.js | 150 ++++++++---------- lib/core/engine/command/commandHelper.js | 30 ++++ lib/core/engine/command/javaScript.js | 12 +- lib/core/engine/command/mouse/singleClick.js | 157 +++++++++---------- 4 files changed, 175 insertions(+), 174 deletions(-) create mode 100644 lib/core/engine/command/commandHelper.js diff --git a/lib/core/engine/command/click.js b/lib/core/engine/command/click.js index 2f1df42da..1dc4819e4 100644 --- a/lib/core/engine/command/click.js +++ b/lib/core/engine/command/click.js @@ -1,4 +1,5 @@ import { getLogger } from '@sitespeed.io/log'; +import { executeCommand } from './commandHelper.js'; const log = getLogger('browsertime.command.click'); function addClick(js) { @@ -45,14 +46,16 @@ export class Click { * @throws {Error} Throws an error if the element is not found. */ async byClassName(className) { - try { - const script = `document.getElementsByClassName('${className}')[0].click();`; - await this.browser.runScript(script, 'CUSTOM'); - } catch (error) { - log.error('Could not find element by class name %s', className); - log.verbose(error); - throw new Error('Could not find element by class name ' + className); - } + return executeCommand( + log, + 'Could not find element by class name %s', + className, + () => + this.browser.runScript( + `document.getElementsByClassName('${className}')[0].click();`, + 'CUSTOM' + ) + ); } /** @@ -76,14 +79,9 @@ export class Click { * @throws {Error} Throws an error if the link is not found. */ async byLinkText(text) { - try { - const xpath = `//a[text()='${text}']`; - return this.byXpath(xpath); - } catch (error) { - log.error('Could not find link by text %s', text); - log.verbose(error); - throw new Error('Could not find link by text ' + text); - } + return executeCommand(log, 'Could not find link by text %s', text, () => + this.byXpath(`//a[text()='${text}']`) + ); } /** @@ -95,14 +93,9 @@ export class Click { * @throws {Error} Throws an error if the link is not found. */ async byLinkTextAndWait(text) { - try { - const xpath = `//a[text()='${text}']`; - return this.byXpathAndWait(xpath); - } catch (error) { - log.error('Could not find link with text %s', text); - log.verbose(error); - throw new Error('Could not find link by text ' + text); - } + return executeCommand(log, 'Could not find link by text %s', text, () => + this.byXpathAndWait(`//a[text()='${text}']`) + ); } /** @@ -114,14 +107,12 @@ export class Click { * @throws {Error} Throws an error if the link is not found. */ async byPartialLinkText(text) { - try { - const xpath = `//a[contains(text(),'${text}')]`; - return this.byXpath(xpath); - } catch (error) { - log.error('Could not find link by partial text %s', text); - log.verbose(error); - throw new Error('Could not find link by partial text ' + text); - } + return executeCommand( + log, + 'Could not find link by partial text %s', + text, + () => this.byXpath(`//a[contains(text(),'${text}')]`) + ); } /** @@ -133,14 +124,12 @@ export class Click { * @throws {Error} Throws an error if the link is not found. */ async byPartialLinkTextAndWait(text) { - try { - const xpath = `//a[contains(text(),'${text}')]`; - return this.byXpathAndWait(xpath); - } catch (error) { - log.error('Could not find link by partial text %s', text); - log.verbose(error); - throw new Error('Could not find link by partial text ' + text); - } + return executeCommand( + log, + 'Could not find link by partial text %s', + text, + () => this.byXpathAndWait(`//a[contains(text(),'${text}')]`) + ); } /** @@ -152,16 +141,17 @@ export class Click { * @throws {Error} Throws an error if the element is not found. */ async byXpath(xpath) { - try { - // This is how Selenium do internally - const replaced = xpath.replaceAll('"', "'"); - const script = `document.evaluate("${replaced}", document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue.click();`; - return this.browser.runScript(script, 'CUSTOM'); - } catch (error) { - log.error('Could not find element by xpath %s', xpath); - log.verbose(error); - throw new Error('Could not find element by xpath ' + xpath); - } + return executeCommand( + log, + 'Could not find element by xpath %s', + xpath, + () => { + // This is how Selenium do internally + const replaced = xpath.replaceAll('"', "'"); + const script = `document.evaluate("${replaced}", document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue.click();`; + return this.browser.runScript(script, 'CUSTOM'); + } + ); } /** * Clicks on an element that matches a given XPath selector and waits for the page complete check to finish. @@ -184,14 +174,12 @@ export class Click { * @throws {Error} Throws an error if the element is not found. */ async byJs(js) { - try { - const script = addClick(js); - await this.browser.runScript(script, 'CUSTOM'); - } catch (error) { - log.error('Could not find element by JavaScript %s', js); - log.verbose(error); - throw new Error('Could not find element by JavaScript ' + js); - } + return executeCommand( + log, + 'Could not find element by JavaScript %s', + js, + () => this.browser.runScript(addClick(js), 'CUSTOM') + ); } /** @@ -215,14 +203,12 @@ export class Click { * @throws {Error} Throws an error if the element is not found. */ async byId(id) { - try { - const script = `document.getElementById('${id}').click();`; - await this.browser.runScript(script, 'CUSTOM'); - } catch (error) { - log.error('Could not find element by id %s', id); - log.verbose(error); - throw new Error('Could not find element by id ' + id); - } + return executeCommand(log, 'Could not find element by id %s', id, () => + this.browser.runScript( + `document.getElementById('${id}').click();`, + 'CUSTOM' + ) + ); } /** @@ -234,14 +220,12 @@ export class Click { * @throws {Error} Throws an error if the element is not found. */ async byName(name) { - try { - const script = `document.querySelector("[name='${name}']").click()`; - await this.browser.runScript(script, 'CUSTOM'); - } catch (error) { - log.error('Could not find element by name %s', name); - log.verbose(error); - throw new Error('Could not find element by name ' + name); - } + return executeCommand(log, 'Could not find element by name %s', name, () => + this.browser.runScript( + `document.querySelector("[name='${name}']").click()`, + 'CUSTOM' + ) + ); } /** @@ -263,14 +247,16 @@ export class Click { * @throws {Error} Throws an error if the element is not found. */ async bySelector(selector) { - try { - const script = `document.querySelector('${selector}').click();`; - await this.browser.runScript(script, 'CUSTOM'); - } catch (error) { - log.error('Could not click using selector %s', selector); - log.verbose(error); - throw new Error('Could not click using selector ' + selector); - } + return executeCommand( + log, + 'Could not click using selector %s', + selector, + () => + this.browser.runScript( + `document.querySelector('${selector}').click();`, + 'CUSTOM' + ) + ); } /** diff --git a/lib/core/engine/command/commandHelper.js b/lib/core/engine/command/commandHelper.js new file mode 100644 index 000000000..e18377bd8 --- /dev/null +++ b/lib/core/engine/command/commandHelper.js @@ -0,0 +1,30 @@ +/** + * Shared error handling wrapper for command methods. + * + * Most command methods follow the same try/catch pattern: + * log.error + log.verbose + throw new Error. This helper + * eliminates that boilerplate. + * + * @param {Object} log - The logger instance. + * @param {string} errorMessage - printf-style message (with %s placeholder). + * @param {string|undefined} identifier - Value to interpolate into the message, or undefined if none. + * @param {Function} fn - The async function to execute. + * @returns {Promise<*>} The return value of fn. + */ +export async function executeCommand(log, errorMessage, identifier, fn) { + try { + return await fn(); + } catch (error) { + if (identifier === undefined) { + log.error(errorMessage); + } else { + log.error(errorMessage, identifier); + } + log.verbose(error); + throw new Error( + identifier === undefined + ? errorMessage + : errorMessage.replace('%s', String(identifier)) + ); + } +} diff --git a/lib/core/engine/command/javaScript.js b/lib/core/engine/command/javaScript.js index 50fb1ca10..096f5d427 100644 --- a/lib/core/engine/command/javaScript.js +++ b/lib/core/engine/command/javaScript.js @@ -1,4 +1,5 @@ import { getLogger } from '@sitespeed.io/log'; +import { executeCommand } from './commandHelper.js'; const log = getLogger('browsertime.command.javascript'); /** * Provides functionality to execute JavaScript code in the context of the current page. @@ -36,14 +37,9 @@ export class JavaScript { * @throws {Error} Throws an error if the JavaScript cannot be executed. */ async run(js) { - try { - const value = await this.browser.runScript(js, 'CUSTOM'); - return value; - } catch (error) { - log.error('Could not run JavaScript %s ', js); - log.verbose(error); - throw new Error(`Could not run JavaScript ${js}`); - } + return executeCommand(log, 'Could not run JavaScript %s', js, () => + this.browser.runScript(js, 'CUSTOM') + ); } /** diff --git a/lib/core/engine/command/mouse/singleClick.js b/lib/core/engine/command/mouse/singleClick.js index 2df7f1b51..804d64642 100644 --- a/lib/core/engine/command/mouse/singleClick.js +++ b/lib/core/engine/command/mouse/singleClick.js @@ -1,5 +1,6 @@ import { getLogger } from '@sitespeed.io/log'; import { By } from 'selenium-webdriver'; +import { executeCommand } from '../commandHelper.js'; const log = getLogger('browsertime.command.mouse'); /** @@ -42,22 +43,23 @@ export class SingleClick { * @throws {Error} Throws an error if the element is not found. */ async byXpath(xpath, options) { - try { - const element = await this.browser - .getDriver() - .findElement(By.xpath(xpath)); - await this.actions.click(element).perform(); - if (options && 'wait' in options && options.wait === true) { - log.warn( - 'Please use the byXpathAndWait method instead. We want to deprecate and remove the options from this method so we follow the same pattern everywhere.' - ); - return this.browser.extraWait(this.pageCompleteCheck); + return executeCommand( + log, + 'Could not single click on element with xpath %s', + xpath, + async () => { + const element = await this.browser + .getDriver() + .findElement(By.xpath(xpath)); + await this.actions.click(element).perform(); + if (options && 'wait' in options && options.wait === true) { + log.warn( + 'Please use the byXpathAndWait method instead. We want to deprecate and remove the options from this method so we follow the same pattern everywhere.' + ); + return this.browser.extraWait(this.pageCompleteCheck); + } } - } catch (error) { - log.error('Could not single click on element with xpath %s', xpath); - log.verbose(error); - throw new Error('Could not single click on element with xpath ' + xpath); - } + ); } /** @@ -82,24 +84,23 @@ export class SingleClick { * @throws {Error} Throws an error if the element is not found. */ async bySelector(selector, options) { - try { - const element = await this.browser - .getDriver() - .findElement(By.css(selector)); - await this.actions.click(element).perform(); - if (options && 'wait' in options && options.wait === true) { - log.warn( - 'Please use the bySelectorAndWait method instead. We want to deprecate and remove the options from this method so we follow the same pattern everywhere.' - ); - return this.browser.extraWait(this.pageCompleteCheck); + return executeCommand( + log, + 'Could not single click on element with selector %s', + selector, + async () => { + const element = await this.browser + .getDriver() + .findElement(By.css(selector)); + await this.actions.click(element).perform(); + if (options && 'wait' in options && options.wait === true) { + log.warn( + 'Please use the bySelectorAndWait method instead. We want to deprecate and remove the options from this method so we follow the same pattern everywhere.' + ); + return this.browser.extraWait(this.pageCompleteCheck); + } } - } catch (error) { - log.error('Could not single click on element with selector %s', selector); - log.verbose(error); - throw new Error( - 'Could not single click on element with selector ' + selector - ); - } + ); } /** @@ -123,19 +124,20 @@ export class SingleClick { * @throws {Error} Throws an error if the single click action cannot be performed. */ async atCursor(options) { - try { - await this.actions.click().perform(); - if (options && 'wait' in options && options.wait === true) { - log.warn( - 'Please use the atCursorAndWait method instead.We want to deprecate and remove the options from this method so we follow the same pattern everywhere.' - ); - return this.browser.extraWait(this.pageCompleteCheck); + return executeCommand( + log, + 'Could not perform single click', + undefined, + async () => { + await this.actions.click().perform(); + if (options && 'wait' in options && options.wait === true) { + log.warn( + 'Please use the atCursorAndWait method instead.We want to deprecate and remove the options from this method so we follow the same pattern everywhere.' + ); + return this.browser.extraWait(this.pageCompleteCheck); + } } - } catch (error) { - log.error('Could not perform single click'); - log.verbose(error); - throw new Error('Could not perform single click'); - } + ); } /** @@ -159,14 +161,9 @@ export class SingleClick { * @throws {Error} Throws an error if the link is not found. */ async byLinkText(text) { - try { - const xpath = `//a[text()='${text}']`; - return this.byXpath(xpath); - } catch (error) { - log.error('Could not find link by text %s', text); - log.verbose(error); - throw new Error('Could not find link by text ' + text); - } + return executeCommand(log, 'Could not find link by text %s', text, () => + this.byXpath(`//a[text()='${text}']`) + ); } /** @@ -178,14 +175,9 @@ export class SingleClick { * @throws {Error} Throws an error if the link is not found. */ async byLinkTextAndWait(text) { - try { - const xpath = `//a[text()='${text}']`; - return this.byXpathAndWait(xpath); - } catch (error) { - log.error('Could not find link by text %s', text); - log.verbose(error); - throw new Error('Could not find link by text ' + text); - } + return executeCommand(log, 'Could not find link by text %s', text, () => + this.byXpathAndWait(`//a[text()='${text}']`) + ); } /** @@ -197,14 +189,12 @@ export class SingleClick { * @throws {Error} Throws an error if the link is not found. */ async byPartialLinkText(text) { - try { - const xpath = `//a[contains(text(),'${text}')]`; - return this.byXpath(xpath); - } catch (error) { - log.error('Could not find link by partial text %s', text); - log.verbose(error); - throw new Error('Could not find link by partial text ' + text); - } + return executeCommand( + log, + 'Could not find link by partial text %s', + text, + () => this.byXpath(`//a[contains(text(),'${text}')]`) + ); } /** @@ -217,14 +207,12 @@ export class SingleClick { * @throws {Error} Throws an error if the link is not found. */ async byPartialLinkTextAndWait(text) { - try { - const xpath = `//a[contains(text(),'${text}')]`; - return this.byXpathAndWait(xpath); - } catch (error) { - log.error('Could not find link by partial text %s', text); - log.verbose(error); - throw new Error('Could not find link by partial text ' + text); - } + return executeCommand( + log, + 'Could not find link by partial text %s', + text, + () => this.byXpathAndWait(`//a[contains(text(),'${text}')]`) + ); } /** @@ -236,14 +224,15 @@ export class SingleClick { * @throws {Error} Throws an error if the id is not found. */ async byId(id) { - try { - const element = await this.browser.getDriver().findElement(By.id(id)); - return this.actions.click(element).perform(); - } catch (error) { - log.error('Could not find the element with id %s', id); - log.verbose(error); - throw new Error('Could not the element with id ' + id); - } + return executeCommand( + log, + 'Could not find the element with id %s', + id, + async () => { + const element = await this.browser.getDriver().findElement(By.id(id)); + return this.actions.click(element).perform(); + } + ); } /**