bugfix: prevent this error when running WebDriver without Bidi protocol: 'Error: Failed to execute WebDriver Bidi command "sessionSubscribe"'

[ngraf]

…d "sessionSubscribe" as no Bidi session was established. Make sure you enable it by setting "webSocketUrl: true" in your capabilities and verify that your environment and browser supports it.' being printed to stdout when user deactivated Bidi protocol with "bidiProtocol" : false in Webdriver config.

Motivation/Description of the PR

Situation:

• I use "WebDriver" against Selenoid
• Selenoid does not support Bidi protocol, because of reasons
• Since Bidi protocol active became the new default behavior in CodeceptJS recently, my tests time out when executed against Selenoid 😕
• Applying workaround "bidiProtocol" : "false" in WebDriver helper enables me to still run my test against Selenoid 🙂

Problem:

• BUT, you always see this annoying error in STDOUT and you don't have the power to prevent it: 😕
  ❌ "Error: Failed to execute WebDriver Bidi command "sessionSubscribe" as no Bidi session was established. Make sure you enable it by setting "webSocketUrl: true" in your capabilities and verify that your environment and browser supports it."

Solution presented in the pull-request:

• before

await this.browser.sessionSubscribe({ events: ['log.entryAdded'] })

• after

    // Check for Bidi, because "sessionSubscribe" is an exclusive Bidi protocol feature. Otherwise, error will be thrown.
    if (this.browser.capabilities && this.browser.capabilities.webSocketUrl) {
      await this.browser.sessionSubscribe({ events: ['log.entryAdded'] })
    }

Applicable helpers:

• Playwright
• Puppeteer
• WebDriver
• REST
• FileHelper
• Appium
• TestCafe

Type of change

• 🔥 Breaking changes
• 🚀 New functionality
• 🐛 Bug fix
• 🧹 Chore
• 📋 Documentation changes/updates
• ♨️ Hot fix
• 🔨 Markdown files fix - not related to source code
• 💅 Polish code

Checklist:

• Tests have been added
• Documentation has been added (Run npm run docs)
• Lint checking (Run npm run lint)
• Local tests are passed (Run npm test)

[Copilot]

Pull Request Overview

This PR fixes an error that occurs when running WebDriver without the Bidi protocol enabled. The fix prevents the error message about failed "sessionSubscribe" command from being printed when users explicitly disable Bidi protocol support.

• Adds conditional check before calling sessionSubscribe to only execute when Bidi protocol is enabled
• Updates comment to reflect that Bidi protocol default is now true
• Prevents error output when using "bidiProtocol": false configuration with environments that don't support Bidi

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The event listener for 'log.entryAdded' is being registered regardless of whether Bidi protocol is enabled, but the corresponding sessionSubscribe call that enables these events is now conditional. This will result in the event listener never being triggered when Bidi is disabled, creating dead code.

Suggested change

	}
	this.browser.on('log.entryAdded', logEvents)
	this.browser.on('log.entryAdded', logEvents)
	}

[kobenguyent]

@ngraf may you advise this please? Thanks!

[ngraf]

Fair point from Copilot.

I made an additional commit and moved
"this.browser.on('log.entryAdded', logEvents)" into the "if" condition:

    if (this.browser.capabilities && this.browser.capabilities.webSocketUrl) {
      await this.browser.sessionSubscribe({ events: ['log.entryAdded'] })
      this.browser.on('log.entryAdded', logEvents)
    }

@kobenguyent , what is actually the downpart of not having the browser logs added?
Will it break some essential functionality? Or is it just a bonus feature?
I mean, as of today the feature does not work for "bidiProtocol: false" anyway. I wonder if this fact needs to be documented in the documentation that the feature of tracking browser logs is only supported when "bidiProtocol" is true. Alternatively the feature to collect logs could be rewritten in a way, so that the feature works for "bidiProtocol" false and true. But I guess it may be hard to impossible to collect logs without "bidiProtocol" enabled.

[ngraf]

The unit test failed. Isn a flaky test failing. At least the failing unit test is about some timing and wait mechanism. I don't really see a relationship to my tiny changes.

[kobenguyent]

Fair point from Copilot.

I made an additional commit and moved "this.browser.on('log.entryAdded', logEvents)" into the "if" condition:

    if (this.browser.capabilities && this.browser.capabilities.webSocketUrl) {
      await this.browser.sessionSubscribe({ events: ['log.entryAdded'] })
      this.browser.on('log.entryAdded', logEvents)
    }

@kobenguyent , what is actually the downpart of not having the browser logs added? Will it break some essential functionality? Or is it just a bonus feature? I mean, as of today the feature does not work for "bidiProtocol: false" anyway. I wonder if this fact needs to be documented in the documentation that the feature of tracking browser logs is only supported when "bidiProtocol" is true. Alternatively the feature to collect logs could be rewritten in a way, so that the feature works for "bidiProtocol" false and true. But I guess it may be hard to impossible to collect logs without "bidiProtocol" enabled.

imho, the current implementation is quite basic and only handles log events.