[bidi][js] Add high-level logging API

[pujagani]

User description

Thanks for contributing to Selenium! A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines. Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Partial implementation of #13992

Motivation and Context

Provide high-level API that is easy for users to use.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [X] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• [X] I have read the contributing document.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [ ] I have added tests to cover my changes.
• [X] All new and existing tests passed.

---

PR Type

Enhancement, Tests

---

Description

• Added a new Script class to handle JavaScript error and console message logging.
• Integrated the Script class into the WebDriver class with a new script method.
• Enhanced error handling in LogInspector for removing non-existent callbacks.
• Added comprehensive tests for the Script class methods, including error handling.

---

Changes walkthrough 📝

	Relevant files
Enhancement	logInspector.js Add error handling for callback removal                                    javascript/node/selenium-webdriver/bidi/logInspector.js Added error handling for removing non-existent callbacks. Introduced a check to verify if the callback ID exists before deletion. +6/-0      script.js Introduce Script class for logging handlers                            javascript/node/selenium-webdriver/lib/script.js Introduced a new Script class to handle JavaScript error and console message logging. Added methods to add and remove JavaScript error and console message handlers. +63/-0    webdriver.js Integrate Script class into WebDriver                                        javascript/node/selenium-webdriver/lib/webdriver.js Integrated the new Script class into the WebDriver class. Added a script method to initialize and return a Script instance. +12/-0
Tests	webdriver_script_test.js Add tests for Script class logging handlers                            javascript/node/selenium-webdriver/test/lib/webdriver_script_test.js Added tests for the new Script class methods. Included tests for adding and removing JavaScript error and console message handlers. Added a test for error handling when removing a non-existent handler. +91/-0

---

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	3
🧪 Relevant tests	Yes
🔒 Security concerns	No
⚡ Key issues to review	Error Handling: The `removeCallback` method in `logInspector.js` now throws an error if the callback ID is not found. Ensure that all calling code properly handles this exception.
	Initialization Pattern: The `#init` method in `Script` class uses a pattern that might be confusing as it mixes async initialization with regular constructor usage. Consider documenting this pattern clearly or revising the approach to initialization.

[codiumai-pr-agent-pro[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	Return early after deleting the callback to simplify the code ___ **Instead of using a let variable to track if the callback ID was found, you can return early once the callback is deleted. This will make the code more concise and potentially more efficient.** [javascript/node/selenium-webdriver/bidi/logInspector.js [69-79]](https://github.com/SeleniumHQ/selenium/pull/14135/files#diff-c8f0c0185fabe12906d45a35bc7bf890b65f2fba0d6b80040552c5c2176a6d87R69-R79) ```diff -let hasId = false for (const [, callbacks] of this.listener) { if (callbacks.has(id)) { callbacks.delete(id) - hasId = true + return } } -if (!hasId) { - throw Error(`Callback with id ${id} not found`) -} +throw Error(`Callback with id ${id} not found`) ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies an opportunity to simplify the code by using an early return instead of a flag variable. This change enhances readability and efficiency.	8
Performance	Initialize the #logInspector in the constructor to avoid multiple initializations ___ **The #init method should be called only once in the constructor to avoid multiple initializations. You can use a flag to check if initialization is complete.** [javascript/node/selenium-webdriver/lib/script.js [24-39]](https://github.com/SeleniumHQ/selenium/pull/14135/files#diff-c905a3b55dc121eee1ed81ed41659372f4e9eb47971bbdf7a876a10c44f3ff48R24-R39) ```diff constructor(driver) { this.#driver = driver + this.#initPromise = this.#init() } async #init() { if (this.#logInspector !== undefined) { return } this.#logInspector = await logInspector(this.#driver) } ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why: The suggestion is valid and improves the initialization pattern of `#logInspector`, ensuring it's done once, which is a good practice for performance and maintainability.	7
Possible issue	Mark the script method as async to ensure proper initialization ___ **The script method should be marked as async to ensure that the Script instance is fully initialized before being used.** [javascript/node/selenium-webdriver/lib/webdriver.js [1109-1117]](https://github.com/SeleniumHQ/selenium/pull/14135/files#diff-4530acbaed1fe859529c5e9d4020af22cace559c3a0561fb139c23c68939bb16R1109-R1117) ```diff -script() { +async script() { if (this.#script === undefined) { this.#script = new Script(this) + await this.#script.#init() } return this.#script } ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 6 Why: The suggestion correctly points out the need for the `script` method to be asynchronous to ensure the `Script` instance is fully initialized. This is important for correct operation but is a moderate issue.	6
	Add a timeout to the delay function to prevent infinite waits ___ **Add a timeout to the delay function to avoid potential infinite waits if the promise is not resolved.** [javascript/node/selenium-webdriver/test/lib/webdriver_script_test.js [37-39]](https://github.com/SeleniumHQ/selenium/pull/14135/files#diff-f26ade6bb564903944cc83de7387e152f2c0086c73c39739b418a1aea1510597R37-R39) ```diff function delay(ms) { - return new Promise((resolve) => setTimeout(resolve, ms)) + return new Promise((resolve, reject) => { + const timeout = setTimeout(resolve, ms) + setTimeout(() => { + clearTimeout(timeout) + reject(new Error('Delay timed out')) + }, ms + 5000) + }) } ``` - [x] **Apply this suggestion** Suggestion importance[1-10]: 5 Why: Adding a timeout to the `delay` function is a good practice to avoid potential infinite waits. However, the context of usage (in tests) and the specific implementation suggested might not be necessary or optimal, thus the moderate score.	5