fix(SDK-7061): resilient TRA build-stop — retry + success-gated flag + synchronous shutdown stop

[pri-gadhiya]

What is this about?

Fixes SDK-7061: WebdriverIO runs where the Test Observability / TRA build appears "stuck" and only closes via the ~60-min server-side inactivity timeout, even though the run finished.

Root cause: the build-stop (PUT /api/v1/builds/<uuid>/stop) was sent once, best-effort — no retry, and it did not check the HTTP status. If that request failed/timed out, or the WDIO launcher was interrupted (CI cancel/timeout) before it was sent, the build was never marked complete.

This PR makes the build-stop resilient (verified on WebdriverIO-cucumber, before/after):

1. util.ts stopBuildUpstream — retries the stop 3× with backoff and treats a non-2xx response as a failure (previously a non-2xx was logged as success).
2. launcher.ts onComplete — buildStopped is set only when the stop actually succeeds, so a failed stop stays recoverable.
3. exitHandler.ts — synchronous/blocking shutdown stop. On SIGTERM/SIGINT/SIGHUP/uncaughtException/exit, a blocking spawnSync child issues the stop PUT and the process waits for it before exiting — the Node analog of Java's blocking Runtime.getRuntime().addShutdownHook. This closes the build on a graceful CI cancel/timeout. (An earlier async handler was verified not to work here: WDIO's launcher calls process.exit before an async PUT can land; the synchronous approach fixes that. The JWT is read from the inherited environment, never written to disk.)

Consistency: this mirrors how browserstack-javaagent (JVM shutdown hook) and browserstack-node-agent (process.on signal suite) handle build-stop on termination — an in-process shutdown hook, no separate watchdog process.

Known boundary (out of scope here, same as java/node): a true uncatchable kill -9/OOM of the launcher runs no handler, so that build still relies on a server-side inactivity/heartbeat auto-close — tracked separately (the shared browserstack-binary stopBuild path has the same retry gap and is the highest-leverage place to harden across all binary-flow SDKs).

Related Jira task/s

• https://browserstack.atlassian.net/browse/SDK-7061
• Related (server-side appearance analysis): https://browserstack.atlassian.net/browse/TRAP-4046

Release (mandatory for every PR — required for the ready-for-review label)

Version bump: (required — tick exactly one)

• minor (backwards-compatible feature)
• patch (bug fix or other small change)

Release notes type: (optional)

• New Feature
• Bug Fix
• Other Improvement

Release notes (customer-facing): (optional but encouraged)

• Fixed WebdriverIO test results sometimes not appearing (builds staying "in progress") when the build-completion signal failed or the test runner was interrupted.

Release notes (internal): (required — engineer-facing; what actually changed / why)

• stopBuildUpstream hardened: 3× retry + backoff + response.ok check (was a single best-effort PUT; non-2xx was logged as success).
• buildStopped gated on stop success (was set unconditionally), so the shutdown path can re-attempt a failed stop.
• Added a synchronous/blocking shutdown stop in exitHandler.ts (SIGTERM/SIGINT/SIGHUP/uncaughtException/exit via spawnSync) — Java addShutdownHook analog; closes the build on graceful interruption. JWT read from env, not persisted. Bounded: blocks exit ≤15s if the collector is unreachable (teardown-only).

Checklist

• Ready to review
• Has it been tested locally?

PR Validations

Run Tests: Comment RUN_TESTS to trigger sanity tests.

[pri-gadhiya]

RUN_TESTS

[pri-gadhiya]

🔴 SDK PR Review — SDK-7061 (automated stack:sdk-pr-review)

Verdict: 🔴 Blocking — the util.ts retry/response.ok hardening and the launcher.ts buildStopped gating are correct and match sibling-SDK precedent (javaagent addShutdownHook, node-agent process.on suite); the new synchronous shutdown-stop in exitHandler.ts has one grounded correctness bug in its guard clauses, plus no test coverage for the three changed behaviors.

🔴 Blocking (must-fix before merge)

packages/browserstack-service/src/exitHandler.ts:81-90 — stopBuildSyncBlocking's early-return guards run outside its own try, and can throw before BrowserStackConfig is initialized, defeating the "never throws" contract.

• BrowserStackConfig.getInstance() called bare returns the singleton, which is undefined until the launcher constructor reaches getInstance(_options, _config, capabilities) at launcher.ts:122.
• But setupExitHandlers() arms the uncaughtException listener at launcher.ts:91 — 31 lines earlier. Anything that throws synchronously in that window fires the new uncaughtException handler → stopBuildSyncBlocking('uncaughtException') → config is undefined → config.testObservability throws a second, unguarded TypeError.
• A throw from inside an uncaughtException listener is fatal in Node — it crashes outside the intended process.exit(1) path and masks the original error.
• Suggested resolution: add if (!config) { return } right after getInstance() (or move the three early-return checks inside the existing try) so the function is unconditionally throw-safe regardless of constructor init order.

🟠 Non-blocking (strongly recommended)

• No tests for any of the three changed behaviors (gh pr diff touches only the 3 source files + the changeset). util.test.ts doesn't assert the non-2xx→retry behavior (and the existing failure test now sleeps ~1.5s of real backoff with no new assertion); launcher.test.ts doesn't assert the buildStopped gating; exitHandler.ts has no test file. Given the PR's value is these behaviors, worth closing before merge.
• Narrow double-send window: if SIGTERM/SIGINT arrives while onComplete's await stopBuildUpstream() is still in flight (before buildStopped is set), the sync path fires a second concurrent stop. Likely harmless (idempotent, bounded, best-effort) but a real race this PR introduces — worth an acknowledging comment or an extra "stop in flight" guard.
• CLI-path scoping: stopBuildSyncBlocking early-returns when the CLI is running (intentional), and handleCLICleanup() only kill()s the child without awaiting its stop. Scope the release note to the direct-HTTP path so it doesn't imply CLI-mode users get the same guarantee.

💡 Nits

• launcher.ts — unchecked cast on the stopResult shape (both stop functions pass through wrappers that erase the return type; correct today, no compiler safety net).
• Inline sync-stop script exits 0 on missing UUID/JWT/endpoint (reads as success). Unreachable today (parent pre-checks) but consider exiting non-zero for defense-in-depth.
• SIGHUP registered on win32 where it has limited semantics — harmless, worth a comment since the platform branch below shows platform-awareness.

✅ Confirmed clean (traced, not just diff-read)

Interaction with merged #77 (observability-disable-on-blocked-buildstart) — no conflict (stop path gates on UUID/JWT env, not .enabled); single handler registration (launcher only, no per-worker duplication); no watchdog remnants; JWT never in argv (env-read inside the child); spawnSync retry budget (13.5s) fits the 15s timeout; framework-agnostic (cucumber/mocha/jasmine); not the isRunning() bootstrap anti-pattern; tsc --noEmit + eslint pass.

Bottom line: one real must-fix (the unguarded throw), then this is good to go; the test coverage is a strong recommendation.

[pri-gadhiya]

✅ SDK PR Review (re-review) — SDK-7061 · cec4d73

Verdict: ✅ Green — the prior 🔴 blocking finding is resolved; good to merge from a correctness standpoint.

The earlier 🔴 — stopBuildSyncBlocking dereferencing BrowserStackConfig.getInstance() while it's still undefined during the early uncaughtException window (handler armed at launcher.ts:91, config initialized at launcher.ts:122) — is fixed in cec4d73: the function now guards with if (!config) { return } as its first statement, before any other access. Traced all three entry paths (signal handler, uncaughtException handler, exit handler) — all route through the guarded function and are safe. Optional chaining on config.testObservability?.buildStopped closes the related soft spot; the one remaining write (config.testObservability.buildStopped = true) is reached only after the guard and is inside a try/catch, so it can't escape as an unhandled throw either.

tsc --noEmit, npm run build, and eslint on exitHandler.ts are all clean on Node 20. No new issues introduced by cec4d73 (it's an isolated 4-line guard).

Non-blocking, still open (deferred — follow-up hardening, not blockers):

• No test coverage yet for the three shutdown behaviors (signal / exit-hook / uncaughtException sync stop).
• Narrow signal-vs-onComplete double-send window — guarded by buildStopSentOnSignal / buildStopped, not structurally eliminated.
• Release note doesn't explicitly scope out the CLI-managed build-stop path (stopBuildSyncBlocking early-returns when the CLI is running).
• Minor naming / log-level nits.