Fix/sdk 7061 buildstop retry - #115
Merged
hamza-browserstack merged 9 commits intoJul 29, 2026
Merged
Conversation
…lag + detached watchdog) The build-stop PUT was a single best-effort request with no retry and no HTTP-status check; a transient failure, or a launcher terminated (CI cancel / kill -9 / OOM) before it was sent, left the O11Y build hanging 'running' until the ~60-min inactivity timeout. - stopBuildUpstream: 3x retry + backoff + response.ok check - buildStopped gated on stop success (recoverable by exit cleanup) - detached shutdown-watchdog survives launcher kill -9 and sends the stop; JWT from inherited env (not disk), marker 0o600 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…atchdog Replace the detached-watchdog approach with the sibling-idiomatic in-process shutdown hook, made SYNCHRONOUS/blocking (spawnSync) so the stop PUT lands before WDIO force-exits (the async variant did not). Matches browserstack-javaagent (addShutdownHook) + browserstack-node-agent (process.on signal suite); no separate process, no JWT on disk. Net vs main: util.ts (stop retry + response.ok), launcher.ts (buildStopped gated on success), exitHandler.ts (sync shutdown stop on SIGTERM/SIGINT/SIGHUP/uncaughtException/exit). Removes buildWatchdog.ts + buildWatchdogMarker.ts. True kill -9/OOM remains a server-side concern. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…op (review) stopBuildSyncBlocking is armed by setupExitHandlers() before BrowserStackConfig is initialized in the launcher constructor; an early uncaughtException could hit a bare getInstance() returning undefined and throw a TypeError from inside the uncaughtException listener (fatal, masks the original error). Add an if(!config) return guard + optional chaining so the handler never throws. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…topBuildSyncBlocking for tests The stopBuildUpstream retry (3x) broke tests/util.test.ts 'return error if failed' (stubbed only one rejection → attempt 2 hit the default 200 mock → returned success). Fix the existing tests to the new behavior and add coverage. - util.test.ts: single-call success (response.ok path); reject all 3 attempts → error; non-2xx (500/502/503) retried → error; transient 500-then-200 → success. Fake timers for the 500ms*attempt backoff (real-timers-first then re-fake with setTimeout), mockClear (not mockReset) to preserve the shared global fetch mock. - launcher.test.ts: onComplete direct-HTTP stop success → buildStopped=true; stop error → buildStopped=false. - exitHandler.test.ts (new): stopBuildSyncBlocking — config-undefined early-return (no throw/spawn), spawnSync invoked once + marks buildStopped on status 0, no mark on non-zero, skips when CLI running / UUID+JWT absent / already stopped. Hermetic (spawnSync mocked). - src/exitHandler.ts: add `export` to stopBuildSyncBlocking (testability only; zero runtime behavior change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y-aware; no exit hijack) Addresses review of the sync-shutdown approach. Replaces the raw signal handlers + spawnSync + process.exit(0) with a single async hook registered through async-exit-hook (the same package @wdio/cli uses), awaiting the proxy-aware stopBuildUpstream(). C1: no longer calls process.exit — participates in the shared async-exit-hook ordered exit, so WDIO's runner.shutdown() completes and the conventional signal exit code (SIGTERM 143 / SIGINT 130) is preserved instead of masked to 0; stops intercepting SIGABRT/SIGQUIT. C2: the stop now flows through stopBuildUpstream -> fetchWrapper._fetch -> undici ProxyAgent (honors HTTP(S)_PROXY/NO_PROXY); removes the raw http/https sync child that bypassed proxy support. Adds async-exit-hook dependency (matches @wdio/cli) + ambient type decl; rewrites exitHandler.test.ts for the new registration. No version bump / no changelog. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…p-retry # Conflicts: # packages/browserstack-service/src/exitHandler.ts # packages/browserstack-service/tests/exitHandler.test.ts
hamza-browserstack
requested review from
anish353 and
kamal-kaur04
and removed request for
a team
July 29, 2026 18:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this about?
Related Jira task/s
Release (mandatory for every PR — required for the
ready-for-reviewlabel)Version bump: (required — tick exactly one)
Release notes type: (optional)
Release notes (customer-facing): (optional but encouraged)
Release notes (internal): (required — engineer-facing; what actually changed / why)
Checklist
PR Validations
Run Tests: Comment RUN_TESTS to trigger sanity tests.