fix(observability): make TRA build-stop survive corporate-network blips (SDK-7229)

.changeset/pr-134.md

@@ -0,0 +1,7 @@
+---
+"@wdio/browserstack-service": patch
+---
+
+- Made the Test Reporting build-completion signal more resilient on restricted corporate networks, so builds are less likely to be left showing as "running" after a run ends.
+- Each delivery attempt is now individually time-bounded, so a hung connection can no longer stall the end of a run.
+- When the signal still cannot be delivered, the log now records the underlying network reason (for example a DNS or proxy failure) instead of a generic `fetch failed`.

packages/browserstack-service/src/constants.ts

@@ -157,6 +157,15 @@ export const SPAWN_RETRY_DELAY_MS = 1000
 export const WDIO_NAMING_PREFIX = 'WebdriverIO-'
 export const PERF_METRICS_WAIT_TIME = 2000
 
+// Build-stop delivery budget. The stop PUT is the only signal that closes a TRA build,
+// so a transient transport failure is worth retrying past a short network blip — but the
+// whole thing runs during shutdown, so the total cost is capped by a wall-clock deadline
+// rather than by attempt count alone.
+export const STOP_BUILD_MAX_ATTEMPTS = 4
+export const STOP_BUILD_ATTEMPT_TIMEOUT_MS = 10000
+export const STOP_BUILD_TOTAL_BUDGET_MS = 30000
+export const STOP_BUILD_BACKOFF_BASE_MS = 1000
+
 // API Endpoint constants
 export const UPDATED_CLI_ENDPOINT = 'sdk/v1/update_cli'
 

packages/browserstack-service/src/util.ts

@@ -44,7 +44,11 @@ import {
     APP_ALLY_ISSUES_SUMMARY_ENDPOINT,
     APP_ALLY_ISSUES_ENDPOINT,
     CLI_DEBUG_LOGS_FILE,
-    WDIO_NAMING_PREFIX
+    WDIO_NAMING_PREFIX,
+    STOP_BUILD_MAX_ATTEMPTS,
+    STOP_BUILD_ATTEMPT_TIMEOUT_MS,
+    STOP_BUILD_TOTAL_BUDGET_MS,
+    STOP_BUILD_BACKOFF_BASE_MS
 } from './constants.js'
 import CrashReporter from './crash-reporter.js'
 import { BStackLogger } from './bstackLogger.js'
@@ -732,6 +736,26 @@ export const getA11yResultsSummary = PerformanceTester.measureWrapper(PERFORMANC
     }
 })
 
+/**
+ * Render an error together with its `cause` chain.
+ *
+ * Node's native fetch reports every transport failure as the same opaque
+ * `TypeError: fetch failed`; the actionable detail (ENOTFOUND, ECONNRESET, a proxy
+ * refusal) only lives on `error.cause`, which plain interpolation drops. Without this
+ * a failed build-stop is indistinguishable from any other network fault in the logs.
+ */
+export const describeErrorWithCause = (error: unknown, depth = 3): string => {
+    if (error === null || error === undefined) {
+        return String(error)
+    }
+    const described = error instanceof Error ? `${error.name}: ${error.message}` : String(error)
+    const cause = (error as { cause?: unknown }).cause
+    if (cause === null || cause === undefined || depth <= 1) {
+        return described
+    }
+    return `${described} <- caused by ${describeErrorWithCause(cause, depth - 1)}`
+}
+
 export const stopBuildUpstream = PerformanceTester.measureWrapper(PERFORMANCE_SDK_EVENTS.TESTHUB_EVENTS.STOP, o11yErrorHandler(async function stopBuildUpstream() {
     const stopBuildUsage = UsageStats.getInstance().stopBuildUsage
     stopBuildUsage.triggered()
@@ -766,17 +790,30 @@ export const stopBuildUpstream = PerformanceTester.measureWrapper(PERFORMANCE_SD
     // best-effort request means a transient failure/timeout leaves the build running
     // until the ~60-min server-side inactivity timeout. Retry with backoff and treat a
     // non-2xx response as a failure so the build reliably reaches a terminal state.
-    const maxAttempts = 3
+    // SDK-7229: a corporate DNS/proxy blip routinely outlasts a sub-second retry window,
+    // and the request had no timeout at all, so a hung connection could stall shutdown
+    // indefinitely. Each attempt is now individually bounded, and the retry loop as a
+    // whole is capped by a wall-clock deadline (this runs during shutdown).
+    const deadline = Date.now() + STOP_BUILD_TOTAL_BUDGET_MS
     let lastError: unknown
-    for (let attempt = 1; attempt <= maxAttempts; attempt++) {
+    let attempts = 0
+    for (let attempt = 1; attempt <= STOP_BUILD_MAX_ATTEMPTS; attempt++) {
+        const budgetLeft = deadline - Date.now()
+        if (budgetLeft <= 0) {
+            break
+        }
+        attempts = attempt
+        const controller = new AbortController()
+        const timeoutId = setTimeout(() => controller.abort(), Math.min(STOP_BUILD_ATTEMPT_TIMEOUT_MS, budgetLeft))
         try {
             const response = await fetch(url, {
                 method: 'PUT',
                 headers: {
                     ...DEFAULT_REQUEST_CONFIG.headers,
                     'Authorization': `Bearer ${process.env[BROWSERSTACK_TESTHUB_JWT]}`
                 },
-                body: JSON.stringify(data)
+                body: JSON.stringify(data),
+                signal: controller.signal
             })
             if (!response.ok) {
                 throw new Error(`HTTP ${response.status} ${response.statusText}`)
@@ -788,15 +825,23 @@ export const stopBuildUpstream = PerformanceTester.measureWrapper(PERFORMANCE_SD
                 message: ''
             }
         } catch (error: unknown) {
-            lastError = error
-            BStackLogger.debug(`[STOP_BUILD] Attempt ${attempt}/${maxAttempts} failed. Error: ${error}`)
-            if (attempt < maxAttempts) {
-                await new Promise((resolve) => setTimeout(resolve, 500 * attempt))
+            // An aborted attempt surfaces as a generic AbortError; name the real reason so
+            // a slow endpoint is not mistaken for an outright transport failure.
+            lastError = controller.signal.aborted
+                ? new Error(`build stop request timed out after ${Math.min(STOP_BUILD_ATTEMPT_TIMEOUT_MS, budgetLeft)}ms`)
+                : error
+            BStackLogger.debug(`[STOP_BUILD] Attempt ${attempt}/${STOP_BUILD_MAX_ATTEMPTS} failed. Error: ${describeErrorWithCause(lastError)}`)
+            const backoff = STOP_BUILD_BACKOFF_BASE_MS * Math.pow(2, attempt - 1)
+            if (attempt >= STOP_BUILD_MAX_ATTEMPTS || Date.now() + backoff >= deadline) {
+                break
             }
+            await new Promise((resolve) => setTimeout(resolve, backoff))
+        } finally {
+            clearTimeout(timeoutId)
         }
     }
     stopBuildUsage.failed(lastError)
-    BStackLogger.debug(`[STOP_BUILD] Failed after ${maxAttempts} attempts. Error: ${lastError}`)
+    BStackLogger.debug(`[STOP_BUILD] Failed after ${attempts} attempt(s). Error: ${describeErrorWithCause(lastError)}`)
     return {
         status: 'error',
         message: (lastError as Error)?.message ?? 'stop build failed'

packages/browserstack-service/tests/util.test.ts

@@ -54,11 +54,12 @@ import {
     getAppA11yResults,
     isMultiRemoteCaps,
     getTestPlanId,
+    describeErrorWithCause,
 } from '../src/util.js'
 import * as bstackLogger from '../src/bstackLogger.js'
 import PerformanceTester from '../src/instrumentation/performance/performance-tester.js'
 import * as PERFORMANCE_SDK_EVENTS from '../src/instrumentation/performance/constants.js'
-import { BROWSERSTACK_OBSERVABILITY, TESTOPS_BUILD_COMPLETED_ENV, BROWSERSTACK_TESTHUB_JWT, BROWSERSTACK_ACCESSIBILITY, BROWSERSTACK_TEST_PLAN_ID } from '../src/constants.js'
+import { BROWSERSTACK_OBSERVABILITY, TESTOPS_BUILD_COMPLETED_ENV, BROWSERSTACK_TESTHUB_JWT, BROWSERSTACK_ACCESSIBILITY, BROWSERSTACK_TEST_PLAN_ID, STOP_BUILD_MAX_ATTEMPTS } from '../src/constants.js'
 import * as testHubUtils from '../src/testHub/utils.js'
 import * as fs from 'node:fs/promises'
 import * as os from 'node:os'
@@ -823,6 +824,36 @@ describe('getScenarioExamples', () => {
     })
 })
 
+describe('describeErrorWithCause', () => {
+    it('unwraps the cause chain that native fetch hides behind "fetch failed"', () => {
+        // SDK-7229: this exact shape — an opaque TypeError wrapping the real DNS failure —
+        // is what a corporate-network build stop produces. Interpolating the error alone
+        // yields only "TypeError: fetch failed", which is not actionable.
+        const cause = Object.assign(new Error('getaddrinfo ENOTFOUND collector-observability.browserstack.com'), {
+            code: 'ENOTFOUND', syscall: 'getaddrinfo'
+        })
+        const error = Object.assign(new TypeError('fetch failed'), { cause })
+
+        expect(describeErrorWithCause(error)).toEqual(
+            'TypeError: fetch failed <- caused by Error: getaddrinfo ENOTFOUND collector-observability.browserstack.com'
+        )
+    })
+
+    it('returns a plain error unchanged when there is no cause', () => {
+        expect(describeErrorWithCause(new Error('HTTP 502 Bad Gateway'))).toEqual('Error: HTTP 502 Bad Gateway')
+    })
+
+    it('handles non-error values and stops at the depth limit', () => {
+        expect(describeErrorWithCause('boom')).toEqual('boom')
+        expect(describeErrorWithCause(undefined)).toEqual('undefined')
+
+        const deep = Object.assign(new Error('a'), {
+            cause: Object.assign(new Error('b'), { cause: new Error('c') })
+        })
+        expect(describeErrorWithCause(deep, 2)).toEqual('Error: a <- caused by Error: b')
+    })
+})
+
 describe('stopBuildUpstream', () => {
     // Fake setTimeout (in addition to the module-level Date) so the SDK-7061 retry
     // backoff can be advanced deterministically without real wall-clock delay.
@@ -864,24 +895,67 @@ describe('stopBuildUpstream', () => {
         process.env[TESTOPS_BUILD_COMPLETED_ENV] = 'true'
         process.env[BROWSERSTACK_TESTHUB_JWT] = 'jwt'
 
-        // SDK-7061: the stop PUT is now retried up to 3x with a 500ms*attempt backoff.
-        // Reject every attempt so the loop exhausts and returns an error. Fake timers keep
-        // the ~1.5s of cumulative backoff out of wall-clock and make the run deterministic.
+        // SDK-7061 / SDK-7229: the stop PUT is retried up to STOP_BUILD_MAX_ATTEMPTS times
+        // with an exponential backoff. Reject every attempt so the loop exhausts and returns
+        // an error. Fake timers keep the cumulative backoff out of wall-clock time.
         useBackoffTimers()
         vi.mocked(fetch)
             .mockImplementationOnce(() => Promise.reject(new Error('network')))
             .mockImplementationOnce(() => Promise.reject(new Error('network')))
             .mockImplementationOnce(() => Promise.reject(new Error('network')))
+            .mockImplementationOnce(() => Promise.reject(new Error('network')))
 
         const promise = stopBuildUpstream()
-        await vi.advanceTimersByTimeAsync(2000)
+        await vi.advanceTimersByTimeAsync(60000)
         const result: any = await promise
 
         expect(vi.mocked(fetch).mock.calls[0][1]?.method).toEqual('PUT')
-        expect(vi.mocked(fetch).mock.calls.length).toEqual(3)
+        expect(vi.mocked(fetch).mock.calls.length).toEqual(STOP_BUILD_MAX_ATTEMPTS)
         expect(result.status).toEqual('error')
     })
 
+    it('passes an abort signal so a hung stop request cannot stall shutdown', async () => {
+        process.env[TESTOPS_BUILD_COMPLETED_ENV] = 'true'
+        process.env[BROWSERSTACK_TESTHUB_JWT] = 'jwt'
+
+        vi.mocked(fetch).mockReturnValueOnce(Promise.resolve(Response.json({})))
+
+        await stopBuildUpstream()
+
+        // SDK-7229: previously the stop PUT carried no signal at all, so a connection that
+        // never settled blocked onComplete indefinitely.
+        expect((vi.mocked(fetch).mock.calls[0][1] as RequestInit)?.signal).toBeInstanceOf(AbortSignal)
+    })
+
+    it('times out a hung attempt and gives up within the total budget', async () => {
+        process.env[TESTOPS_BUILD_COMPLETED_ENV] = 'true'
+        process.env[BROWSERSTACK_TESTHUB_JWT] = 'jwt'
+
+        // SDK-7229: a request that never settles must be aborted per attempt, and the loop
+        // as a whole must stop at STOP_BUILD_TOTAL_BUDGET_MS rather than run the full
+        // attempt count. Capture/restore the shared default implementation so the persistent
+        // mockImplementation below cannot leak into later suites (afterEach only clears).
+        useBackoffTimers()
+        const defaultImpl = vi.mocked(fetch).getMockImplementation()
+        vi.mocked(fetch).mockImplementation(((_url: unknown, init: RequestInit) => new Promise((_resolve, reject) => {
+            init.signal?.addEventListener('abort', () => reject(new Error('The operation was aborted')))
+        })) as unknown as typeof fetch)
+
+        try {
+            const promise = stopBuildUpstream()
+            await vi.advanceTimersByTimeAsync(120000)
+            const result: any = await promise
+
+            expect(result.status).toEqual('error')
+            expect(result.message).toContain('timed out')
+            // Budget-bound, so strictly fewer than the full attempt allowance.
+            expect(vi.mocked(fetch).mock.calls.length).toBeLessThan(STOP_BUILD_MAX_ATTEMPTS)
+            expect(vi.mocked(fetch).mock.calls.length).toBeGreaterThan(1)
+        } finally {
+            vi.mocked(fetch).mockImplementation(defaultImpl as typeof fetch)
+        }
+    })
+
     it('retries on a non-2xx response and returns error when all attempts fail', async () => {
         process.env[TESTOPS_BUILD_COMPLETED_ENV] = 'true'
         process.env[BROWSERSTACK_TESTHUB_JWT] = 'jwt'
@@ -893,12 +967,13 @@ describe('stopBuildUpstream', () => {
             .mockReturnValueOnce(Promise.resolve(Response.json({}, { status: 500, statusText: 'Server Error' })))
             .mockReturnValueOnce(Promise.resolve(Response.json({}, { status: 502, statusText: 'Bad Gateway' })))
             .mockReturnValueOnce(Promise.resolve(Response.json({}, { status: 503, statusText: 'Service Unavailable' })))
+            .mockReturnValueOnce(Promise.resolve(Response.json({}, { status: 503, statusText: 'Service Unavailable' })))
 
         const promise = stopBuildUpstream()
-        await vi.advanceTimersByTimeAsync(2000)
+        await vi.advanceTimersByTimeAsync(60000)
         const result: any = await promise
 
-        expect(vi.mocked(fetch).mock.calls.length).toEqual(3)
+        expect(vi.mocked(fetch).mock.calls.length).toEqual(STOP_BUILD_MAX_ATTEMPTS)
         expect(result.status).toEqual('error')
     })
 
@@ -914,7 +989,7 @@ describe('stopBuildUpstream', () => {
             .mockReturnValueOnce(Promise.resolve(Response.json({})))
 
         const promise = stopBuildUpstream()
-        await vi.advanceTimersByTimeAsync(2000)
+        await vi.advanceTimersByTimeAsync(60000)
         const result: any = await promise
 
         expect(vi.mocked(fetch).mock.calls.length).toEqual(2)