Fix/sdk 7061 buildstop retry

.changeset/pr-99.md

@@ -0,0 +1,5 @@
+---
+"@wdio/browserstack-service": patch
+---
+
+- Fixed WebdriverIO test results sometimes not appearing (builds staying "in progress") when the build-completion signal failed or the test runner was interrupted.

packages/browserstack-service/src/launcher.ts

@@ -629,8 +629,12 @@ export default class BrowserstackLauncherService implements Services.ServiceInst
             // SDK-4671: before stopping the build, synthesize TestRunFinished for any
             // test runs whose worker died mid-test, else they stay 'in progress' on TRA.
             await finalizeOrphanedRuns()
+            // SDK-7061: capture the stop result so we only mark the build stopped when it
+            // actually succeeded. A failed stop must leave buildStopped=false so the
+            // process-exit cleanup re-stop path can still close the build.
+            let stopResult: { status?: string } | undefined
             try {
-                await (isCLIEnabled ? BrowserstackCLI.getInstance().stop() : stopBuildUpstream())
+                stopResult = await (isCLIEnabled ? BrowserstackCLI.getInstance().stop() : stopBuildUpstream()) as { status?: string } | undefined
                 PerformanceTester.end(PERFORMANCE_SDK_EVENTS.FRAMEWORK_EVENTS.STOP)
             } catch (err) {
                 BStackLogger.error(`Error while stopping CLI ${err}`)
@@ -639,7 +643,11 @@ export default class BrowserstackLauncherService implements Services.ServiceInst
             if (process.env[BROWSERSTACK_OBSERVABILITY] && process.env[BROWSERSTACK_TESTHUB_UUID]) {
                 console.log(`\nVisit https://automation.browserstack.com/builds/${process.env[BROWSERSTACK_TESTHUB_UUID]} to view build report, insights, and many more debugging information all at one place!\n`)
             }
-            this.browserStackConfig.testObservability.buildStopped = true
+            // CLI path manages its own build lifecycle; for the direct-HTTP path only
+            // mark stopped when stopBuildUpstream returned success (SDK-7061).
+            if (isCLIEnabled || stopResult?.status === 'success') {
+                this.browserStackConfig.testObservability.buildStopped = true
+            }
 
             await PerformanceTester.stopAndGenerate('performance-launcher.html')
             if (process.env[PERF_MEASUREMENT_ENV]) {

packages/browserstack-service/src/util.ts

@@ -743,30 +743,46 @@ export const stopBuildUpstream = PerformanceTester.measureWrapper(PERFORMANCE_SD
         data.finished_metadata = [{ reason: 'user_killed', signal: killSignal }]
     }
 
-    try {
-        const url = `${APIUtils.DATA_ENDPOINT}/api/v1/builds/${process.env[BROWSERSTACK_TESTHUB_UUID]}/stop`
-        const response = await fetch(url, {
-            method: 'PUT',
-            headers: {
-                ...DEFAULT_REQUEST_CONFIG.headers,
-                'Authorization': `Bearer ${process.env[BROWSERSTACK_TESTHUB_JWT]}`
-            },
-            body: JSON.stringify(data)
-        })
-        BStackLogger.debug(`[STOP_BUILD] Success response: ${await response.text()}`)
-        stopBuildUsage.success()
-        return {
-            status: 'success',
-            message: ''
-        }
-    } catch (error: unknown) {
-        stopBuildUsage.failed(error)
-        BStackLogger.debug(`[STOP_BUILD] Failed. Error: ${error}`)
-        return {
-            status: 'error',
-            message: (error as Error).message
+    const url = `${APIUtils.DATA_ENDPOINT}/api/v1/builds/${process.env[BROWSERSTACK_TESTHUB_UUID]}/stop`
+    // SDK-7061: the build-stop PUT is the signal that closes the TRA build. A single
+    // 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
+    let lastError: unknown
+    for (let attempt = 1; attempt <= maxAttempts; attempt++) {
+        try {
+            const response = await fetch(url, {
+                method: 'PUT',
+                headers: {
+                    ...DEFAULT_REQUEST_CONFIG.headers,
+                    'Authorization': `Bearer ${process.env[BROWSERSTACK_TESTHUB_JWT]}`
+                },
+                body: JSON.stringify(data)
+            })
+            if (!response.ok) {
+                throw new Error(`HTTP ${response.status} ${response.statusText}`)
+            }
+            BStackLogger.debug(`[STOP_BUILD] Success response: ${await response.text()}`)
+            stopBuildUsage.success()
+            return {
+                status: 'success',
+                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))
+            }
         }
     }
+    stopBuildUsage.failed(lastError)
+    BStackLogger.debug(`[STOP_BUILD] Failed after ${maxAttempts} attempts. Error: ${lastError}`)
+    return {
+        status: 'error',
+        message: (lastError as Error)?.message ?? 'stop build failed'
+    }
 }))
 
 export function getCiInfo () {

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

@@ -17,6 +17,7 @@ import * as FunnelInstrumentation from '../src/instrumentation/funnelInstrumenta
 import { RERUN_TESTS_ENV, BROWSERSTACK_TESTHUB_UUID, RERUN_ENV } from '../src/constants.js'
 import * as thUtils from '../src/testHub/utils.js'
 import TestOpsConfig from '../src/testOps/testOpsConfig.js'
+import { BrowserstackCLI } from '../src/cli/index.js'
 
 vi.mock('@wdio/logger', () => import(path.join(process.cwd(), '__mocks__', '@wdio/logger')))
 vi.mock('browserstack-local')
@@ -728,6 +729,29 @@ describe('onComplete', () => {
         await service.onComplete()
         expect(stopBuildUpstreamSpy).toHaveBeenCalledTimes(1)
     })
+
+    // SDK-7061: on the direct-HTTP path buildStopped must reflect whether the stop PUT
+    // actually succeeded, so the process-exit cleanup re-stop can still close the build
+    // when the in-hook stop failed.
+    it('marks buildStopped true when the direct-HTTP stop succeeds', async () => {
+        vi.spyOn(BrowserstackCLI.getInstance(), 'isRunning').mockReturnValue(false)
+        vi.spyOn(utils, 'stopBuildUpstream').mockResolvedValue({ status: 'success', message: '' } as any)
+
+        const service = new BrowserstackLauncher({} as any, [{}] as any, {} as any)
+        ;(service as any).browserStackConfig.testObservability.buildStopped = false
+        await service.onComplete()
+        expect((service as any).browserStackConfig.testObservability.buildStopped).toBe(true)
+    })
+
+    it('leaves buildStopped false when the direct-HTTP stop fails', async () => {
+        vi.spyOn(BrowserstackCLI.getInstance(), 'isRunning').mockReturnValue(false)
+        vi.spyOn(utils, 'stopBuildUpstream').mockResolvedValue({ status: 'error', message: 'boom' } as any)
+
+        const service = new BrowserstackLauncher({} as any, [{}] as any, {} as any)
+        ;(service as any).browserStackConfig.testObservability.buildStopped = false
+        await service.onComplete()
+        expect((service as any).browserStackConfig.testObservability.buildStopped).toBe(false)
+    })
 })
 
 describe('constructor', () => {

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

@@ -824,6 +824,17 @@ describe('getScenarioExamples', () => {
 })
 
 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.
+    // performance is intentionally NOT faked (negative perf.now() under the 2020
+    // system clock breaks perf_hooks on Node 18) — mirrors the module-level config.
+    // useRealTimers() first is required: re-calling useFakeTimers over an already-active
+    // fake clock does NOT widen `toFake`, so setTimeout would otherwise stay real.
+    const useBackoffTimers = () => {
+        vi.useRealTimers()
+        vi.useFakeTimers({ toFake: ['Date', 'setTimeout', 'clearTimeout'] }).setSystemTime(new Date('2020-01-01'))
+    }
+
     it('return error if completed but jwt token not present', async () => {
         process.env[TESTOPS_BUILD_COMPLETED_ENV] = 'true'
         delete process.env[BROWSERSTACK_TESTHUB_JWT]
@@ -839,26 +850,85 @@ describe('stopBuildUpstream', () => {
         process.env[TESTOPS_BUILD_COMPLETED_ENV] = 'true'
         process.env[BROWSERSTACK_TESTHUB_JWT] = 'jwt'
 
+        // SDK-7061: the stop path now checks response.ok and reads response.text(),
+        // so the mock must yield an ok (2xx) response with a readable body.
         vi.mocked(fetch).mockReturnValueOnce(Promise.resolve(Response.json({})))
 
         const result: any = await stopBuildUpstream()
         expect(vi.mocked(fetch).mock.calls[0][1]?.method).toEqual('PUT')
+        expect(vi.mocked(fetch).mock.calls.length).toEqual(1)
         expect(result.status).toEqual('success')
     })
 
     it('return error if failed', async () => {
         process.env[TESTOPS_BUILD_COMPLETED_ENV] = 'true'
         process.env[BROWSERSTACK_TESTHUB_JWT] = 'jwt'
 
-        vi.mocked(fetch).mockReturnValueOnce(Promise.reject(Response.json({})))
+        // 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.
+        useBackoffTimers()
+        vi.mocked(fetch)
+            .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)
+        const result: any = await promise
 
-        const result: any = await stopBuildUpstream()
         expect(vi.mocked(fetch).mock.calls[0][1]?.method).toEqual('PUT')
+        expect(vi.mocked(fetch).mock.calls.length).toEqual(3)
+        expect(result.status).toEqual('error')
+    })
+
+    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'
+
+        // SDK-7061: a non-2xx response is a failure and must be retried (previously a
+        // best-effort PUT logged any response as success).
+        useBackoffTimers()
+        vi.mocked(fetch)
+            .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' })))
+
+        const promise = stopBuildUpstream()
+        await vi.advanceTimersByTimeAsync(2000)
+        const result: any = await promise
+
+        expect(vi.mocked(fetch).mock.calls.length).toEqual(3)
         expect(result.status).toEqual('error')
     })
 
+    it('returns success when a later retry attempt succeeds', async () => {
+        process.env[TESTOPS_BUILD_COMPLETED_ENV] = 'true'
+        process.env[BROWSERSTACK_TESTHUB_JWT] = 'jwt'
+
+        // SDK-7061: a transient failure followed by a 2xx must resolve to success without
+        // exhausting all attempts.
+        useBackoffTimers()
+        vi.mocked(fetch)
+            .mockReturnValueOnce(Promise.resolve(Response.json({}, { status: 500, statusText: 'Server Error' })))
+            .mockReturnValueOnce(Promise.resolve(Response.json({})))
+
+        const promise = stopBuildUpstream()
+        await vi.advanceTimersByTimeAsync(2000)
+        const result: any = await promise
+
+        expect(vi.mocked(fetch).mock.calls.length).toEqual(2)
+        expect(result.status).toEqual('success')
+    })
+
     afterEach(() => {
+        // mockClear (not mockReset) so the shared global fetch mock keeps its default
+        // implementation for the rest of the file; the *Once queues above are fully
+        // consumed within each test, so nothing leaks.
         vi.mocked(fetch).mockClear()
+        // Restore the module-level clock config (only Date faked) so later suites in this
+        // file are unaffected by the setTimeout faking these retry tests turn on.
+        vi.useFakeTimers({ toFake: ['Date'] }).setSystemTime(new Date('2020-01-01'))
     })
 })