diff --git a/packages/wdio-browserstack-service/src/cli/grpcClient.ts b/packages/wdio-browserstack-service/src/cli/grpcClient.ts index ab3f5dbd5f4..89c05172e1d 100644 --- 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} * @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 = { 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).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 index 8b54814d61b..b9d5709fa7c 100644 --- 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} */ - 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 index b8e1e13d421..de917b28713 100644 --- 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 = { + 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((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 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 index ef95e92dc51..df71d538c35 100644 --- 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 = { '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`