Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions packages/wdio-browserstack-service/src/cli/grpcClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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!)
Expand Down
10 changes: 7 additions & 3 deletions packages/wdio-browserstack-service/src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`)
}

Expand Down
138 changes: 138 additions & 0 deletions packages/wdio-browserstack-service/src/exitHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
23 changes: 21 additions & 2 deletions packages/wdio-browserstack-service/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand All @@ -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`
Expand Down
Loading