fix(browserstack-service): flush build-stop event on process kill (SDK-6050)

harshit-browserstack · harshit-browserstack · commit 442821a0c834 · 2026-05-20T16:17:10.000+05:30
When the WDIO process is killed mid-run (Ctrl+C / SIGINT / SIGTERM), the
service previously relied on a synchronous process.on('exit') handler that
cannot await the async build-stop call. As a result, TestHub never received
a terminal event for the build and waited out its inactivity timeout,
leaving the build in "Unknown Test Status" / TEST_TIMEOUT_WITH_BUILD_SUCCESS
for hours.

This change mirrors browserstack-node-agent's intExitHandler /
TestHubHandler.stop(signal) pattern:

- Register async handlers for SIGINT, SIGTERM, SIGHUP, SIGABRT, SIGQUIT
  (and SIGBREAK on Windows). Each awaits the existing build-stop call
  with a bounded 10-second grace, then exits with the POSIX code (128 +
  signum). Idempotent: first signal wins; subsequent signals are ignored.

- Snapshot and remove pre-existing listeners for those signals at install
  time, so create-wdio's synchronous process.exit hook and async-exit-hook
  (registered transitively by @wdio/cli) cannot kill our async cleanup
  mid-await.

- Thread the signal through BrowserstackCLI.stop(signal) and
  GrpcClient.stopBinSession(signal) so the StopBinSessionRequest carries
  { exitSignal, exitReason: 'user_killed', exitCode: 1 }. The binary's
  onStop reads these and forwards them to the TestHub stop API as
  finished_metadata: [{ reason: 'user_killed', signal }] -- matching the
  shape node-sdk and java-agent already send.

- For Direct (non-CLI) mode, stopBuildUpstream(signal) adds the same
  finished_metadata payload directly to the PUT body.

Verified end-to-end with a 6-cell matrix (Direct + CLI x normal/SIGINT/
SIGTERM). Kill cells now emit the stop tokens, return POSIX exit codes
(130 for SIGINT, 143 for SIGTERM), and TestHub receives the build-stop
PUT with finished_metadata.{reason: 'user_killed', signal} as confirmed
by binary CLI logs.
diff --git a/packages/wdio-browserstack-service/src/cli/grpcClient.ts b/packages/wdio-browserstack-service/src/cli/grpcClient.ts
@@ -244,12 +244,17 @@ export class GrpcClient {
 
     /**
      * Stop the bin session
+     * @param {string} [signal] - Optional POSIX signal name. When set, populates
+     *   `exitSignal`, `exitReason='user_killed'`, `exitCode=1` on the StopBinSessionRequest
+     *   so the binary's testhub `onStop` (`browserstack-binary/packages/@browserstack/testhub/index.js:63-89`)
+     *   reads them and pushes `finished_metadata: [{reason, signal}]` to the TestHub stop API.
+     *   Mirrors browserstack-node-agent `src/bin/v2/grpcClient.js stopBinSession()` (SDK-6050).
      * @returns {Promise<void>}
      * @private
      */
-    async stopBinSession() {
+    async stopBinSession(signal: NodeJS.Signals | null = null) {
         PerformanceTester.start(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_CLI_ON_STOP)
-        this.logger.debug('Stopping bin session')
+        this.logger.debug(`Stopping bin session${signal ? ` (signal=${signal})` : ''}`)
 
         try {
             if (!this.binSessionId) {
@@ -261,12 +266,18 @@ export class GrpcClient {
             }
 
             const clientWorkerId = CLIUtils.getClientWorkerId()
-            const request = StopBinSessionRequestConstructor.create({
+            const requestPayload: Record<string, unknown> = {
                 binSessionId: this.binSessionId
-            })
+            }
+            if (signal) {
+                requestPayload.exitSignal = signal
+                requestPayload.exitReason = 'user_killed'
+                requestPayload.exitCode = 1
+            }
+            const request = StopBinSessionRequestConstructor.create(requestPayload)
             // Add clientWorkerId to request (proto field 500)
             ;(request as unknown as Record<string, unknown>).clientWorkerId = clientWorkerId
-            this.logger.debug(`StopBinSession with clientWorkerId: ${clientWorkerId}`)
+            this.logger.debug(`StopBinSession with clientWorkerId: ${clientWorkerId}${signal ? ` exitSignal=${signal} exitReason=user_killed` : ''}`)
 
             // Get response from gRPC call
             const stopBinSessionPromise = promisify(this.client!.stopBinSession).bind(this.client!)
diff --git a/packages/wdio-browserstack-service/src/cli/index.ts b/packages/wdio-browserstack-service/src/cli/index.ts
@@ -327,14 +327,18 @@ export class BrowserstackCLI {
 
     /**
      * Stop the CLI
+     * @param {string} [signal] - Optional POSIX signal name (e.g. 'SIGINT') when stop is
+     *   triggered by a process signal. Threaded into the gRPC StopBinSessionRequest so the
+     *   binary can mark TestHub `finished_metadata.{reason:'user_killed', signal}` —
+     *   mirrors browserstack-node-agent's `intExitHandler`/`stopBinSession` (see SDK-6050).
      * @returns {Promise<void>}
      */
-    async stop() {
+    async stop(signal: NodeJS.Signals | null = null) {
         PerformanceTester.start(PerformanceEvents.SDK_CLI_ON_STOP)
-        this.logger.debug('stop: CLI stop triggered')
+        this.logger.debug(`stop: CLI stop triggered${signal ? ` (signal=${signal})` : ''}`)
         try {
             if (this.isMainConnected) {
-                const response = await GrpcClient.getInstance().stopBinSession()
+                const response = await GrpcClient.getInstance().stopBinSession(signal)
                 BStackLogger.debug(`stop: stopBinSession response=${JSON.stringify(response)}`)
             }
 
diff --git a/packages/wdio-browserstack-service/src/exitHandler.ts b/packages/wdio-browserstack-service/src/exitHandler.ts
@@ -8,11 +8,149 @@ import PerformanceTester from './instrumentation/performance/performance-tester.
 import TestOpsConfig from './testOps/testOpsConfig.js'
 import { BStackLogger } from './bstackLogger.js'
 import { BrowserstackCLI } from './cli/index.js'
+import { stopBuildUpstream } from './util.js'
 
 const __filename = fileURLToPath(import.meta.url)
 const __dirname = path.dirname(__filename)
 
+// SDK-6050: bounded grace period for async signal-driven build-stop cleanup
+const SIGNAL_CLEANUP_GRACE_MS = 10_000
+
+// Idempotency guard — once cleanup starts, ignore subsequent signals (R5/R6).
+// Module-scope so multiple service instances / multiple signals converge here.
+let signalCleanupInFlight = false
+let signalHandlersInstalled = false
+
+/**
+ * SDK-6050: async signal handler that mirrors the graceful `onComplete()` build-stop
+ * (`launcher.ts:586-590`) so killing the WDIO process mid-run still emits a stop event
+ * to TestHub. Without this, only the synchronous `process.on('exit')` runs — which
+ * cannot await `BrowserstackCLI.stop()` or `stopBuildUpstream()`.
+ *
+ * Behavior:
+ *  - SIGINT, SIGTERM, SIGHUP, SIGABRT, SIGQUIT (+ SIGBREAK on Windows).
+ *  - First signal triggers cleanup; subsequent signals are ignored.
+ *  - Cleanup runs with a {@link SIGNAL_CLEANUP_GRACE_MS} timeout — process exits anyway after.
+ *  - After cleanup (or timeout), exit code 128+signum so callers can distinguish.
+ */
+function setupSignalHandlers() {
+    if (signalHandlersInstalled) {
+        return
+    }
+    signalHandlersInstalled = true
+
+    const signals: NodeJS.Signals[] = ['SIGINT', 'SIGTERM', 'SIGHUP', 'SIGABRT', 'SIGQUIT']
+    if (process.platform === 'win32') {
+        signals.push('SIGBREAK')
+    }
+
+    // Signal-number mapping for exit-code (POSIX convention: 128 + signum).
+    const signalToNum: Record<string, number> = {
+        SIGINT: 2,
+        SIGTERM: 15,
+        SIGHUP: 1,
+        SIGABRT: 6,
+        SIGQUIT: 3,
+        SIGBREAK: 21,
+    }
+
+    const handler = async (signal: NodeJS.Signals) => {
+        if (signalCleanupInFlight) {
+            BStackLogger.debug(`Signal ${signal} received again; cleanup already in flight, ignoring`)
+            return
+        }
+        signalCleanupInFlight = true
+
+        // Defense-in-depth: re-prune any listeners that may have been added
+        // after install. Note that Node snapshots the listener array at the
+        // start of emit() — removals here do NOT prevent already-snapshotted
+        // listeners from firing in THIS emit cycle. That's why the primary
+        // prune happens at install time (see setupSignalHandlers below).
+        try {
+            const listeners = process.listeners(signal).slice()
+            for (const l of listeners) {
+                if (l !== currentHandlerWrappers.get(signal)) {
+                    process.removeListener(signal, l as (...args: unknown[]) => void)
+                }
+            }
+        } catch {
+            // best-effort; never let listener-manipulation errors block cleanup
+        }
+
+        BStackLogger.debug(`Signal ${signal} received; starting async build-stop cleanup (grace ${SIGNAL_CLEANUP_GRACE_MS}ms)`)
+
+        const exitCode = 128 + (signalToNum[signal] ?? 0)
+
+        const cleanup = (async () => {
+            try {
+                const isCLIEnabled = BrowserstackCLI.getInstance().isRunning()
+                BStackLogger.debug(`Sending stop launch event (signal=${signal}, mode=${isCLIEnabled ? 'cli' : 'direct'})`)
+                await (isCLIEnabled ? BrowserstackCLI.getInstance().stop(signal) : stopBuildUpstream(signal))
+                BrowserStackConfig.getInstance().testObservability.buildStopped = true
+                BStackLogger.debug(`Build-stop completed for signal ${signal}`)
+            } catch (err) {
+                BStackLogger.debug(`Build-stop on signal ${signal} failed: ${err}`)
+            }
+        })()
+
+        const timeout = new Promise<void>((resolve) => setTimeout(() => {
+            BStackLogger.debug(`Signal cleanup grace period (${SIGNAL_CLEANUP_GRACE_MS}ms) elapsed for ${signal}; exiting`)
+            resolve()
+        }, SIGNAL_CLEANUP_GRACE_MS))
+
+        await Promise.race([cleanup, timeout])
+
+        // After cleanup (or timeout), exit so Node doesn't continue indefinitely.
+        process.exit(exitCode)
+    }
+
+    // Track which wrapper we registered per-signal, so the listener-pruning
+    // above can identify and preserve ours while removing competing handlers.
+    const currentHandlerWrappers = new Map<NodeJS.Signals, (...args: unknown[]) => void>()
+
+    for (const sig of signals) {
+        const wrapper = () => {
+            // Fire-and-forget; node ignores the returned promise on signal handlers anyway.
+            handler(sig).catch((err) => {
+                BStackLogger.debug(`Unhandled error in signal cleanup for ${sig}: ${err}`)
+                process.exit(128 + (signalToNum[sig] ?? 0))
+            })
+        }
+        currentHandlerWrappers.set(sig, wrapper)
+
+        // Critical: snapshot AND remove existing listeners for THIS signal at
+        // install time. Once an emit() begins, Node iterates a frozen snapshot
+        // of the listener array — removals during emit have NO effect on the
+        // current emit cycle. Competing handlers (e.g. create-wdio's top-level
+        // `process.on('SIGINT', () => process.exit(1))` at
+        // packages/create-wdio/src/utils.ts:29, and `async-exit-hook`'s SIGINT
+        // hook registered transitively by @wdio/cli) call `process.exit()`
+        // synchronously, killing our async cleanup mid-await.
+        //
+        // setupExitHandlers() is invoked from the BrowserstackLauncherService
+        // constructor (launcher.ts:88), which runs AFTER @wdio/cli has loaded
+        // create-wdio and async-exit-hook — so by this point their handlers
+        // are already registered and we can prune them cleanly.
+        try {
+            const existing = process.listeners(sig).slice()
+            for (const l of existing) {
+                process.removeListener(sig, l as (...args: unknown[]) => void)
+            }
+            if (existing.length > 0) {
+                BStackLogger.debug(`Removed ${existing.length} pre-existing ${sig} listener(s) at install time`)
+            }
+        } catch {
+            // best-effort; never let listener-manipulation errors block install
+        }
+
+        // Now register ours. prependListener (rather than on) is belt-and-braces
+        // in case a later library still manages to slip a listener in front.
+        process.prependListener(sig, wrapper)
+    }
+}
+
 export function setupExitHandlers() {
+    setupSignalHandlers()
     const handleCLICleanup = () => {
         BStackLogger.debug('Handling CLI cleanup in exit handler')
         try {
diff --git a/packages/wdio-browserstack-service/src/util.ts b/packages/wdio-browserstack-service/src/util.ts
@@ -708,7 +708,15 @@ export const getA11yResultsSummary = PerformanceTester.measureWrapper(PERFORMANC
     }
 })
 
-export const stopBuildUpstream = PerformanceTester.measureWrapper(PERFORMANCE_SDK_EVENTS.TESTHUB_EVENTS.STOP, o11yErrorHandler(async function stopBuildUpstream() {
+/**
+ * Stop the TestHub build (Direct flow).
+ * @param {string} [signal] - Optional POSIX signal name (e.g. 'SIGINT'). When set, includes
+ *   `finished_at` and `finished_metadata: [{reason: 'user_killed', signal, failure_data: ''}]`
+ *   in the PUT payload so the TestHub dashboard renders "Stopped/Aborted" instead of "Unknown".
+ *   Mirrors browserstack-node-agent `src/helpers/testhub/testhubHandler.js TestHubHandler.stop(signal)`
+ *   (SDK-6050).
+ */
+export const stopBuildUpstream = PerformanceTester.measureWrapper(PERFORMANCE_SDK_EVENTS.TESTHUB_EVENTS.STOP, o11yErrorHandler(async function stopBuildUpstream(signal: NodeJS.Signals | null = null) {
     const stopBuildUsage = UsageStats.getInstance().stopBuildUsage
     stopBuildUsage.triggered()
     if (!process.env[TESTOPS_BUILD_COMPLETED_ENV]) {
@@ -727,9 +735,20 @@ export const stopBuildUpstream = PerformanceTester.measureWrapper(PERFORMANCE_SD
             message: 'Token/buildID is undefined, build creation might have failed'
         }
     }
-    const data = {
+    const data: Record<string, unknown> = {
         'stop_time': (new Date()).toISOString()
     }
+    if (signal) {
+        // SDK-6050: signal-aware build stop. Server reads `finished_metadata[0]` and
+        // marks the build "Stopped" instead of the "Unknown / inactivity timeout" state.
+        data['finished_at'] = data['stop_time']
+        data['finished_metadata'] = [{
+            reason: 'user_killed',
+            signal,
+            failure_data: ''
+        }]
+        BStackLogger.debug(`[STOP_BUILD] signal=${signal} reason=user_killed`)
+    }
 
     try {
         const url = `${APIUtils.DATA_ENDPOINT}/api/v1/builds/${process.env[BROWSERSTACK_TESTHUB_UUID]}/stop`
