...

Author: pujagani

javascript/selenium-webdriver/bidi/index.js

@@ -79,10 +79,15 @@ class Index extends EventEmitter {
         }
         return
       }
-      // Messages without a numeric id are BiDi events, not command
-      // responses; they are routed via subscribe/EventEmitter elsewhere
-      // and intentionally ignored by this dispatcher.
+      // Messages without a numeric id are BiDi events, not command responses.
+      // Emit them by method name so that generated domain classes and other
+      // consumers can use bidi.on(methodName, callback) rather than attaching
+      // their own raw ws.on('message', ...) listeners (which accumulate and
+      // trigger MaxListeners warnings).
       if (payload == null || typeof payload.id !== 'number') {
+        if (payload != null && typeof payload.method === 'string') {
+          this.emit(payload.method, payload.params)
+        }
         return
       }
       const entry = this._pending.get(payload.id)

javascript/selenium-webdriver/generate_bidi.mjs

@@ -138,8 +138,13 @@ const DOMAIN_CLASSES = {
 function resolveInputPath(p) {
   if (!p) return null
   if (!process.env.BAZEL_BINDIR) return resolve(p)
-  const prefix = process.env.BAZEL_BINDIR + '/'
-  return resolve(p.startsWith(prefix) ? p.slice(prefix.length) : p)
+  // Normalize both strings to forward slashes before prefix-stripping so that
+  // mixed separators on Windows (BAZEL_BINDIR uses '\', $(location) uses '/')
+  // do not cause the startsWith check to silently fail.
+  const normalizedP = p.replaceAll('\\', '/')
+  const normalizedBindir = process.env.BAZEL_BINDIR.replaceAll('\\', '/')
+  const prefix = normalizedBindir + '/'
+  return resolve(normalizedP.startsWith(prefix) ? normalizedP.slice(prefix.length) : normalizedP)
 }
 
 // ============================================================
@@ -169,6 +174,10 @@ async function main() {
   // The --output-dir arg is passed as <pkg>/<rel_path>, which is already
   // relative to BAZEL_BINDIR (= CWD), so it needs no special handling.
   const cddlPath = resolveInputPath(args.cddl)
+  if (!existsSync(cddlPath)) {
+    console.error(`Error: CDDL file not found: ${cddlPath}`)
+    process.exit(1)
+  }
   const outputDir = resolve(args['output-dir'])
   const specVersion = args['spec-version']
 
@@ -241,7 +250,16 @@ function loadEnhancements(manifestPath) {
     console.warn(`Warning: enhancements manifest not found: ${fullPath}`)
     return {}
   }
-  return JSON.parse(readFileSync(fullPath, 'utf8'))
+  let parsed
+  try {
+    parsed = JSON.parse(readFileSync(fullPath, 'utf8'))
+  } catch (err) {
+    throw new Error(`Failed to parse enhancements manifest at ${fullPath}: ${err.message}`)
+  }
+  if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
+    throw new Error(`Enhancements manifest at ${fullPath} must be a JSON object, got ${Array.isArray(parsed) ? 'array' : typeof parsed}`)
+  }
+  return parsed
 }
 
 // ============================================================
@@ -752,8 +770,6 @@ function generateDomainFile({
     parts.push(`  send(command: Record<string, unknown>): Promise<unknown>`)
     parts.push(`  subscribe(event: string | string[], contexts?: string[]): Promise<void>`)
     parts.push(`  on(event: string, listener: (params: unknown) => void): void`)
-    parts.push(`  /** Raw WebSocket — used by event listeners to attach message handlers. */`)
-    parts.push(`  readonly socket: { on(event: 'message', listener: (data: { toString(): string }) => void): void }`)
     parts.push(`}`)
     parts.push('')
   }
@@ -826,6 +842,10 @@ function generateClass({ className, commands, events, enhancement, emptyResultTy
   lines.push(`  private constructor(private readonly bidi: BidiConnection) {}`)
   lines.push('')
   lines.push(`  static async create(driver: unknown): Promise<${className}> {`)
+  lines.push(`    const caps = await (driver as { getCapabilities(): Promise<{ get(key: string): unknown }> }).getCapabilities()`)
+  lines.push(`    if (!caps.get('webSocketUrl')) {`)
+  lines.push(`      throw new Error('WebDriver instance must support BiDi protocol')`)
+  lines.push(`    }`)
   lines.push(`    const bidi = await (driver as { getBidi(): Promise<BidiConnection> }).getBidi()`)
   lines.push(`    return new ${className}(bidi)`)
   lines.push(`  }`)
@@ -901,19 +921,13 @@ function generateEventMethod(evt) {
   const lines = []
   lines.push(`  async ${onMethodName}(callback: ${cbType}): Promise<void> {`)
   lines.push(`    await this.bidi.subscribe('${methodStr}')`)
-  // Use the raw WebSocket message listener — the same pattern as the hand-coded
-  // logInspector.js / browsingContext.js classes.  bidi/index.js intentionally
-  // ignores WebSocket messages without a numeric "id" in its response dispatcher,
-  // so event payloads must be captured directly from the socket instead.
-  lines.push(`    const ws = this.bidi.socket`)
-  lines.push(`    ws.on('message', (data: { toString(): string }) => {`)
-  lines.push(`      const msg = JSON.parse(data.toString()) as {`)
-  lines.push(`        method?: string`)
-  lines.push(`        params?: unknown`)
-  lines.push(`      }`)
-  lines.push(`      if (msg.method === '${methodStr}') {`)
-  lines.push(`        callback(${paramsTypeName ? `msg.params as ${paramsTypeName}` : 'msg.params'})`)
-  lines.push(`      }`)
+  // bidi/index.js emits BiDi events by method name through its single shared
+  // message dispatcher (which already handles JSON parsing and closed-state
+  // guards). Using bidi.on() here avoids attaching a new ws.on('message', ...)
+  // listener on every subscription call, preventing listener accumulation and
+  // MaxListeners warnings.
+  lines.push(`    this.bidi.on('${methodStr}', (params: unknown) => {`)
+  lines.push(`      callback(${paramsTypeName ? `params as ${paramsTypeName}` : 'params'})`)
   lines.push(`    })`)
   lines.push(`  }`)
   return lines.join('\n')

javascript/selenium-webdriver/private/generate_bidi.bzl

@@ -21,6 +21,36 @@ _DOMAIN_TS_FILES = [
     "webextension.ts",
 ]
 
+def _merge_cddl_impl(ctx):
+    """Merges one or more CDDL files into a single output file."""
+    out = ctx.outputs.out
+    args = ctx.actions.args()
+    args.add(out)
+    args.add_all(ctx.files.srcs)
+    ctx.actions.run(
+        inputs = ctx.files.srcs,
+        outputs = [out],
+        executable = ctx.executable.tool,
+        arguments = [args],
+        mnemonic = "MergeCddl",
+        progress_message = "Merging CDDL files into %s" % out.short_path,
+    )
+    return [DefaultInfo(files = depset([out]))]
+
+_merge_cddl = rule(
+    implementation = _merge_cddl_impl,
+    attrs = {
+        "srcs": attr.label_list(allow_files = True, mandatory = True),
+        "out": attr.output(mandatory = True),
+        "tool": attr.label(
+            executable = True,
+            cfg = "exec",
+            mandatory = True,
+        ),
+    },
+    doc = "Merges CDDL specification files into a single file using an external merge tool.",
+)
+
 def _compile_bidi_ts_impl(ctx):
     ts_files = ctx.files.srcs
     output_subdir = ctx.attr.output_subdir
@@ -41,7 +71,6 @@ def _compile_bidi_ts_impl(ctx):
     args.add("--module", "NodeNext")
     args.add("--moduleResolution", "NodeNext")
     args.add("--declaration")
-    args.add("--removeComments")
     args.add("--outDir", js_outputs[0].dirname)
     for f in ts_files:
         args.add(f.path)
@@ -108,14 +137,14 @@ def generate_bidi_library(
 
     # Step 1: merge CDDL files into one.
     # merge_cddl signature: <output> <input1> [<input2> ...]
-    # genrule: $@ = output, $(SRCS) = inputs in order.
+    # Uses ctx.actions.run so arguments are passed as an argv list rather than
+    # a shell command string, avoiding quoting/escaping issues with special chars.
     merged_name = name + "_merged_cddl"
-    native.genrule(
+    _merge_cddl(
         name = merged_name,
         srcs = [cddl_file] + extra_cddl_files,
-        outs = [name + "_merged.cddl"],
-        cmd = "$(location " + merge_tool + ") $@ $(SRCS)",
-        tools = [merge_tool],
+        out = name + "_merged.cddl",
+        tool = merge_tool,
     )
 
     # Step 2: run generate_bidi.mjs → 15 .ts files (one per BiDi domain).

javascript/selenium-webdriver/test/bidi/generated/browsing_context_test.js

@@ -223,6 +223,7 @@ suite(
         })
 
         await browsingContext.navigate({ context: contextId, url: Pages.emptyPage, wait: 'complete' })
+        await driver.wait(() => navEvent !== null, 5000)
 
         assert.ok(navEvent, 'navigationCommitted event should have fired')
         assert.strictEqual(navEvent.context, contextId)

javascript/selenium-webdriver/test/bidi/generated/log_test.js

@@ -183,9 +183,10 @@ suite(
 
         // Trigger a JS error to produce a javascript log entry
         await driver.findElement({ id: 'jsException' }).click()
+        await driver.wait(() => entries.length > 0, 5000)
 
-        // onJavascriptLog covers JS-level log entries
-        assert.ok(entries.length >= 0) // may or may not fire depending on driver impl
+        assert.ok(entries.length > 0, 'onJavascriptLog should have received at least one entry')
+        assert.strictEqual(entries[0].type, 'javascript')
       })
     })
   },

javascript/selenium-webdriver/test/bidi/generated/network_test.js

@@ -56,7 +56,7 @@ suite(
         assert.ok(event, 'beforeRequestSent should have fired')
         assert.strictEqual(event.request.method, 'GET')
         assert.ok(event.request.url)
-        assert.ok(event.request.headers.length >= 0)
+        assert.ok(event.request.headers.length > 0, 'request should have at least one header')
       })
 
       it('receives cookies in beforeRequestSent', async function () {
@@ -110,9 +110,12 @@ suite(
           errorEvent = params
         })
 
-        // Navigate to a non-existent host to trigger a fetch error
+        // Navigate to a non-existent host to trigger a fetch error.
+        // Use wait: 'none' so the call returns immediately — waiting for
+        // 'complete' on a non-existent host blocks for the full TCP/DNS
+        // timeout before the fetchError event can be observed.
         await browsingContext
-          .navigate({ context: contextId, url: 'http://doesnotexist.invalid/', wait: 'complete' })
+          .navigate({ context: contextId, url: 'http://doesnotexist.invalid/', wait: 'none' })
           .catch(() => {})
         await driver.wait(() => errorEvent !== null, 5000)