[js] Ensure BiDi is not exposed on Driver

javascript/selenium-webdriver/lib/bidi_connection.js

@@ -0,0 +1,82 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+const BiDi = require('../bidi')
+
+/**
+ * One BiDi connection per driver, kept outside the WebDriver class itself so
+ * it isn't a discoverable method on it — see
+ * docs/decisions/17670-bidi-implementation-boundaries.md. Not re-exported from
+ * the package's public entry point.
+ *
+ * Stores the in-flight promise, not the resolved connection, so a second
+ * concurrent call for the same driver awaits the same creation instead of
+ * racing its own and leaking whichever connection loses.
+ * @type {WeakMap<object, Promise<BiDi>>}
+ */
+const connections = new WeakMap()
+
+/**
+ * Returns the BiDi connection for `driver`, creating it on first access.
+ * @param {object} driver
+ * @returns {Promise<BiDi>}
+ */
+function getBidiConnection(driver) {
+  if (!connections.has(driver)) {
+    connections.set(driver, createConnection(driver))
+  }
+  return connections.get(driver)
+}
+
+/**
+ * @param {object} driver
+ * @returns {Promise<BiDi>}
+ */
+async function createConnection(driver) {
+  const caps = await driver.getCapabilities()
+  const webSocketUrl = caps['map_'].get('webSocketUrl')
+  if (!webSocketUrl) {
+    throw new Error('WebDriver instance must support BiDi protocol')
+  }
+  return new BiDi(webSocketUrl.replace('localhost', '127.0.0.1'))
+}
+
+/**
+ * Closes `driver`'s BiDi connection, if one was ever opened. A no-op
+ * otherwise — must not lazily create a connection just to close it.
+ *
+ * Callers (e.g. quit()) invoke this fire-and-forget, so it must never reject:
+ * if the original connection attempt itself had failed, `pending` is already
+ * rejected and there is nothing live to close.
+ * @param {object} driver
+ * @returns {Promise<void>}
+ */
+async function closeBidiConnection(driver) {
+  const pending = connections.get(driver)
+  if (pending === undefined) {
+    return
+  }
+  connections.delete(driver)
+  try {
+    const connection = await pending
+    await connection.close()
+  } catch {
+    // Nothing to close — the original connection attempt failed.
+  }
+}
+
+module.exports = { getBidiConnection, closeBidiConnection }

javascript/selenium-webdriver/lib/webdriver.js

@@ -35,12 +35,13 @@ const http = require('../http/index')
 const fs = require('node:fs')
 const { Capabilities } = require('./capabilities')
 const path = require('node:path')
+const util = require('node:util')
 const { NoSuchElementError } = require('./error')
 const cdpTargets = ['page', 'browser']
 const { Credential } = require('./virtual_authenticator')
 const webElement = require('./webelement')
 const { isObject } = require('./util')
-const BIDI = require('../bidi')
+const { getBidiConnection, closeBidiConnection } = require('./bidi_connection')
 const { PinnedScript } = require('./pinnedScript')
 const JSZip = require('jszip')
 const Script = require('./script')
@@ -794,10 +795,8 @@ class WebDriver {
         this._cdpWsConnection.close()
       }
 
-      // Close the BiDi websocket connection
-      if (this._bidiConnection !== undefined) {
-        this._bidiConnection.close()
-      }
+      // Close the BiDi websocket connection, if one was ever opened
+      closeBidiConnection(this)
     })
   }
 
@@ -1312,19 +1311,6 @@ class WebDriver {
     this._cdpConnection.execute('Target.getTargets')
   }
 
-  /**
-   * Initiates bidi connection using 'webSocketUrl'
-   * @returns {BIDI}
-   */
-  async getBidi() {
-    if (this._bidiConnection === undefined) {
-      const caps = await this.getCapabilities()
-      let WebSocketUrl = caps['map_'].get('webSocketUrl')
-      this._bidiConnection = new BIDI(WebSocketUrl.replace('localhost', '127.0.0.1'))
-    }
-    return this._bidiConnection
-  }
-
   /**
    * Retrieves 'webSocketDebuggerUrl' by sending a http request using debugger address
    * @param {string} debuggerAddress
@@ -1789,6 +1775,22 @@ class WebDriver {
   }
 }
 
+/**
+ * Returns the WebDriver BiDi connection for this session.
+ *
+ * @deprecated BiDi is an internal implementation detail (see
+ * docs/decisions/17670-bidi-implementation-boundaries.md) — this accessor hands
+ * back the raw transport directly, which is no longer supported public API.
+ * Use a composed BiDi module instead, e.g. `Network.create(driver)` or
+ * `require('selenium-webdriver/bidi/network')`.
+ * @function
+ * @name WebDriver#getBidi
+ * @returns {Promise<import('../bidi')>}
+ */
+WebDriver.prototype.getBidi = util.deprecate(function () {
+  return getBidiConnection(this)
+}, 'WebDriver#getBidi() is deprecated. Use a composed BiDi module instead, e.g. Network.create(driver) or ' + "require('selenium-webdriver/bidi/network'). See docs/decisions/17670-bidi-implementation-boundaries.md.")
+
 /**
  * Interface for navigating back and forth in the browser history.
  *

javascript/selenium-webdriver/test/lib/bidi_connection_test.js

@@ -0,0 +1,96 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+'use strict'
+
+const assert = require('node:assert')
+const { WebSocketServer } = require('ws')
+const { getBidiConnection, closeBidiConnection } = require('../../lib/bidi_connection')
+
+function startEchoServer() {
+  return new Promise((resolve) => {
+    const server = new WebSocketServer({ port: 0 }, () => {
+      const { port } = server.address()
+      resolve({ server, url: `ws://127.0.0.1:${port}` })
+    })
+    server.on('connection', (ws) => {
+      ws.on('message', (data) => {
+        const { id } = JSON.parse(data.toString())
+        ws.send(JSON.stringify({ id, result: {} }))
+      })
+    })
+  })
+}
+
+describe('bidi_connection', function () {
+  let server
+
+  afterEach(async function () {
+    if (server !== undefined) {
+      await new Promise((resolve) => server.close(resolve))
+      server = undefined
+    }
+  })
+
+  it('creates exactly one BiDi connection under concurrent first access', async function () {
+    const started = await startEchoServer()
+    server = started.server
+
+    let capabilitiesCalls = 0
+    const driver = {
+      async getCapabilities() {
+        capabilitiesCalls++
+        // Yield the event loop before resolving, so concurrent callers race.
+        await new Promise((resolve) => setTimeout(resolve, 20))
+        return { map_: new Map([['webSocketUrl', started.url]]) }
+      },
+    }
+
+    const [a, b, c] = await Promise.all([
+      getBidiConnection(driver),
+      getBidiConnection(driver),
+      getBidiConnection(driver),
+    ])
+
+    assert.strictEqual(capabilitiesCalls, 1, 'getCapabilities() should only be called once')
+    assert.strictEqual(a, b)
+    assert.strictEqual(b, c)
+
+    await closeBidiConnection(driver)
+  })
+
+  it('closeBidiConnection() is a no-op when no connection was ever opened', async function () {
+    const driver = {
+      async getCapabilities() {
+        throw new Error('should not be called')
+      },
+    }
+    await assert.doesNotReject(closeBidiConnection(driver))
+  })
+
+  it('closeBidiConnection() does not reject when the original connection attempt failed', async function () {
+    const driver = {
+      async getCapabilities() {
+        throw new Error('driver does not support BiDi')
+      },
+    }
+    await assert.rejects(getBidiConnection(driver), /does not support BiDi/)
+    // quit() calls this fire-and-forget (no await/catch at the call site), so
+    // it must swallow the earlier failure rather than re-throwing it.
+    await assert.doesNotReject(closeBidiConnection(driver))
+  })
+})