[JS]: Handle optional dependency for @bazel/runfiles

[harsha509]

User description

Description

In an npm project, requiring @bazel/runfiles with const { runfiles } = require('@bazel/runfiles') throws an exception because @bazel/runfiles is not available outside of Bazel builds.

The fix safely attempts to require @bazel/runfiles and logs a message if it fails.

Types of changes

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

Checklist

• 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.
• All new and existing tests passed.

---

PR Type

Bug fix

---

Description

• Removed direct import of @bazel/runfiles which caused exceptions in non-Bazel environments.
• Added a try-catch block to safely attempt requiring @bazel/runfiles.
• Logged informative error messages if @bazel/runfiles is not available.
• Set runfiles to null if the module is not available to prevent further issues.

---

Changes walkthrough 📝

	Relevant files
Bug fix	index.js Handle optional dependency for `@bazel/runfiles` in tests javascript/node/selenium-webdriver/testing/index.js Removed direct import of @bazel/runfiles. Added a try-catch block to optionally require @bazel/runfiles. Logged error messages if @bazel/runfiles is not available. Set runfiles to null if the module is not available. +12/-1

---

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

[qodo-merge-pro]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Error Handling Consider refining the error handling by specifying different actions or messages based on the type of error when requiring @bazel/runfiles. This can help in understanding the specific issue and how to resolve it, rather than a generic error message.

[qodo-merge-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Maintainability	Consolidate multiple error messages into a single console.error call for clarity Refactor the error handling for requiring @bazel/runfiles to avoid multiple console.error calls and consolidate the error message into a single output. This will make the error logs cleaner and more manageable. javascript/node/selenium-webdriver/testing/index.js [48-57] try { // Attempt to require @bazel/runfiles runfiles = require('@bazel/runfiles').runfiles; } catch (error) { - // Handle error if @bazel/runfiles is not supported by mocha - console.error('Error requiring @bazel/runfiles:', error.message); - console.error('Note: If you are running tests with Mocha or Jasmine, this module is not needed.'); - console.error('For more details, see: https://github.com/bazelbuild/rules_nodejs/issues/3770'); + const errorMessage = `Error requiring @bazel/runfiles: ${error.message}\n` + + `Note: If you are running tests with Mocha or Jasmine, this module is not needed.\n` + + `For more details, see: https://github.com/bazelbuild/rules_nodejs/issues/3770`; + console.error(errorMessage); runfiles = null; // Set to null if not available } Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion improves code maintainability by consolidating multiple console.error calls into a single call, making the error logs cleaner and easier to manage. This is a minor improvement but beneficial for readability and debugging.	7

[pujagani]

I have faced this few times. I think this looks good to me.

[shs96c]

Do we properly wrap all usages of runfiles later in this file? Maybe we do a check before we use it, and exit quickly if we know it'll never succeed?

[harsha509]

Added a check for runfiles and implemented a safe, silent exit if not found.

[shs96c]

LGTM! Thank you!