Service+Nightwatch withTimeout timer leak

Author: vishnuv688

README.md

@@ -82,7 +82,7 @@ The zip contains:
 - `trace.trace` — NDJSON `context-options` + `before`/`after` action events
 - `trace.network` — HAR-style network entries derived from the existing capture
 - `resources/page@<id>-<ts>.jpeg` — screenshot per user-facing action
-- `resources/elements-page@<id>-<ts>.json` — flat interactable element list from `@wdio/elements`
+- `resources/elements-page@<id>-<ts>.json` — flat interactable element list extracted by the page-injected scripts in `@wdio/devtools-core/element-scripts`
 - `resources/snapshot-page@<id>-<ts>.txt` — depth-indented accessibility-tree snapshot (AI-friendly)
 
 What counts as a user-facing action is filtered through an allow-list in `@wdio/devtools-core/action-mapping.ts` (`url`, `click`, `setValue`, `sendKeys`, `get`, etc.). Internal commands like `findElement`/`waitUntil`/`executeScript` don't produce trace entries.
@@ -95,7 +95,7 @@ Trace mode and live mode are **mutually exclusive** — `screencast` options are
 
 #### 📱 Mobile testing
 
-Adapters detect mobile sessions via `platformName: 'android' | 'ios'` (and Appium `bstack:options`) and adjust the per-action snapshot to extract elements from the mobile XML tree instead of the DOM. The trace's `context-options` records `title: 'android'` / `'ios'` so the viewer labels frames correctly.
+Adapters detect mobile sessions via `platformName: 'android' | 'ios'` (case-insensitive) and adjust the per-action snapshot to extract elements from the mobile XML tree instead of the DOM. The trace's `context-options` records `title: 'android' — <deviceName>` / `'ios' — <deviceName>` so the viewer labels frames correctly.
 
 A reference WDIO config is at [examples/wdio/wdio.mobile.conf.ts](examples/wdio/wdio.mobile.conf.ts). Prereqs to run it end-to-end with a local emulator:
 

examples/nightwatch/nightwatch.conf.cjs

@@ -45,7 +45,7 @@ module.exports = {
       //   off to avoid duplicate entries.
       globals: nightwatchDevtools({
         port: 3000,
-        mode: 'trace',
+        mode: 'live',
         screencast: { enabled: true, pollIntervalMs: 200 },
         bidi: true
       })

examples/wdio/wdio.conf.ts

@@ -127,7 +127,15 @@ export const config: Options.Testrunner = {
   // Services take over a specific job you don't want to take care of. They enhance
   // your test setup with almost no effort. Unlike plugins, they don't add new
   // commands. Instead, they hook themselves up into the test process.
-  services: [['devtools', { mode: 'trace' as const }]],
+  services: [
+    [
+      'devtools',
+      {
+        mode: 'live' as const,
+        screencast: { enabled: true, pollIntervalMs: 200 }
+      }
+    ]
+  ],
   //
   // Framework you want to run your specs with.
   // The following are supported: Mocha, Jasmine, and Cucumber

packages/core/src/with-timeout.ts

@@ -12,15 +12,14 @@ export function withTimeout<T>(
   const timeout = new Promise<T>((resolve) => {
     timer = setTimeout(() => resolve(fallback), ms)
   })
-  return Promise.race([
-    promise.then((v) => {
-      if (timer) {
-        clearTimeout(timer)
-      }
-      return v
-    }),
-    timeout
-  ])
+  // .finally on the race result clears the timer in BOTH the resolved AND
+  // rejected branch — earlier code only cleared on fulfillment, leaving a
+  // dangling timer per failed probe that delayed process exit by up to `ms`.
+  return Promise.race([promise, timeout]).finally(() => {
+    if (timer) {
+      clearTimeout(timer)
+    }
+  })
 }
 
 /** Default ceiling for a single in-page snapshot probe. */

packages/nightwatch-devtools/src/session.ts

@@ -456,17 +456,11 @@ export class SessionCapturer extends SessionCapturerBase {
     // Capture network requests from Chrome performance logs
     await this.captureNetworkFromPerformanceLogs(browser)
 
-    // Also try the injected wdioTraceCollector script for XHR/fetch and mutations
+    // Also try the injected wdioTraceCollector script for XHR/fetch and mutations.
+    // Atomic check+read — the inline `typeof === 'undefined' → null` guard is
+    // the only safe form; a separate existence check would race page navigation
+    // (the collector can disappear between the two round-trips).
     try {
-      const checkResult = await browser.execute(
-        'return typeof window.wdioTraceCollector !== "undefined"'
-      )
-      const collectorExists = unwrapDriverValue<unknown>(checkResult) === true
-
-      if (!collectorExists) {
-        return
-      }
-
       const result = await browser.execute(`
         if (typeof window.wdioTraceCollector === 'undefined') {
           return null;

packages/nightwatch-devtools/tests/session.test.ts

@@ -314,11 +314,8 @@ describe('SessionCapturer.captureTrace', () => {
       getLog: vi.fn(async () => []),
       execute: vi.fn(async () => {
         call++
-        if (call === 1) {
-          // collector check
-          return { value: true }
-        }
-        // getTraceData
+        // Single atomic check+read — the inline `typeof === 'undefined' → null`
+        // guard in the script body avoids the navigation TOCTOU race.
         return {
           value: {
             mutations: [
@@ -332,6 +329,6 @@ describe('SessionCapturer.captureTrace', () => {
     })
     const cap = makeCapturer(browser)
     await cap.captureTrace(browser)
-    expect(call).toBe(2)
+    expect(call).toBe(1)
   })
 })

packages/selenium-devtools/tests/session.test.ts

@@ -221,20 +221,18 @@ describe('selenium SessionCapturer (with stashed executeScript)', () => {
     let call = 0
     stubExec(async () => {
       call++
-      return call === 1 ? true : null
+      return null
     })
     const cap = makeCapturer({ id: 'd' })
     await expect(cap.captureTrace()).resolves.toBeUndefined()
-    expect(call).toBe(2)
+    // Single atomic check+read in one executeScript — see session.ts comment.
+    expect(call).toBe(1)
   })
 
   it('captureTrace processes payload when collector returns data', async () => {
     let call = 0
     stubExec(async () => {
       call++
-      if (call === 1) {
-        return true
-      }
       return {
         mutations: [],
         networkRequests: [],
@@ -243,7 +241,7 @@ describe('selenium SessionCapturer (with stashed executeScript)', () => {
     })
     const cap = makeCapturer({ id: 'd' })
     await cap.captureTrace()
-    expect(call).toBe(2)
+    expect(call).toBe(1)
   })
 
   it('captureTrace swallows ECONNREFUSED / no-such-session errors silently', async () => {

packages/service/src/session.ts

@@ -207,20 +207,21 @@ export class SessionCapturer extends SessionCapturerBase {
     }
 
     try {
-      const collectorExists = await browser.execute(
-        () => typeof window.wdioTraceCollector !== 'undefined'
+      // Atomic check+read in a single browser.execute so the collector can't
+      // disappear (page navigation) between the existence check and the
+      // getTraceData call. Two round-trips left a TOCTOU race that surfaced
+      // spurious "Cannot read properties of undefined" errors.
+      const payload = await browser.execute(() =>
+        typeof window.wdioTraceCollector !== 'undefined'
+          ? window.wdioTraceCollector.getTraceData()
+          : null
       )
-
-      if (!collectorExists) {
+      if (!payload) {
         log.warn(
           'wdioTraceCollector not loaded yet - page loaded before preload script took effect'
         )
         return
       }
-
-      const payload = await browser.execute(() =>
-        window.wdioTraceCollector.getTraceData()
-      )
       this.processTracePayload(payload as Record<string, unknown>)
     } catch (err) {
       log.error(`Failed to capture trace: ${errorMessage(err)}`)