From 60690b86f1e0bd87807c328c404d8f5765f67f0a Mon Sep 17 00:00:00 2001 From: Bhargavi-BS Date: Thu, 13 Aug 2026 09:24:37 +0530 Subject: [PATCH 1/2] fix(observability): make TRA build-stop survive corporate-network blips (SDK-7229) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build-stop PUT is the only signal that closes a TRA build. SDK-7061 added a 3-attempt retry, but the whole window was ~1.5s (500ms + 1000ms) — shorter than a typical corporate DNS/proxy blip — and the request carried no timeout at all, so a connection that never settled could stall onComplete indefinitely. On top of that, every failure was logged as a bare `TypeError: fetch failed`. Node's fetch keeps the actionable detail (ENOTFOUND, ECONNRESET, proxy refusal) on `error.cause`, which plain interpolation drops — so a failed build stop was indistinguishable from any other network fault in a customer log. - Widen the retry to STOP_BUILD_MAX_ATTEMPTS (4) with exponential backoff (1s/2s/4s), capped by a STOP_BUILD_TOTAL_BUDGET_MS (30s) wall-clock deadline. The deadline bounds the added shutdown cost regardless of attempt count. - Bound each attempt with an AbortController (STOP_BUILD_ATTEMPT_TIMEOUT_MS, 10s, clamped to the remaining budget), and report an aborted attempt as a timeout rather than a generic AbortError. - Add describeErrorWithCause() and use it on every build-stop failure log so the underlying transport reason is recorded. Observed against the customer's exact failure mode (fetch rejecting with an ENOTFOUND cause): 1 attempt before SDK-7061, 3 attempts / 1.5s on 9.33.1, and 4 attempts / 7.0s here — with the DNS cause now present in the log line. --- .../browserstack-service/src/constants.ts | 9 ++ packages/browserstack-service/src/util.ts | 63 +++++++++++-- .../browserstack-service/tests/util.test.ts | 93 +++++++++++++++++-- 3 files changed, 147 insertions(+), 18 deletions(-) diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index 71e2f49..b5a308d 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/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' diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 972ce45..f4fda77 100644 --- a/packages/browserstack-service/src/util.ts +++ b/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,9 +790,21 @@ 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', @@ -776,7 +812,8 @@ export const stopBuildUpstream = PerformanceTester.measureWrapper(PERFORMANCE_SD ...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' diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index af292e2..0907079 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/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) From 941b6779c66490bed7325e8b6a44f0f2dfdf8760 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 13 Aug 2026 03:56:23 +0000 Subject: [PATCH 2/2] chore(changeset): auto-generate from PR template (patch) --- .changeset/pr-134.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/pr-134.md diff --git a/.changeset/pr-134.md b/.changeset/pr-134.md new file mode 100644 index 0000000..1291e69 --- /dev/null +++ b/.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`.