From 27288d6831140ef6d4afdbe360517977c639abe3 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Thu, 24 Sep 2026 16:52:56 +0530 Subject: [PATCH 1/4] fix(cli): fall back to BROWSERSTACK_BINARY_URL when update_cli fails (SDK-6948) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit update_cli runs during bootstrap — before the binary spawns and before GRR localizes hosts — so it always targets the prod api host. On an internal staging run (BROWSERSTACK_STAGING_ENV) the staging creds are rejected there with 401, leaving the CLI binary path empty and crashing the launcher with spawn('') -> ERR_INVALID_ARG_VALUE ('file' cannot be empty). Wrap the update_cli call: on failure, if BROWSERSTACK_BINARY_URL is set, download the binary from that URL and use it; otherwise re-throw (behaviour unchanged when the var is unset). Adds unit tests for both paths. v9 port of the v8 fix (PR #183), validated end-to-end against devapplca (update_cli 401 -> fallback download from a served zip -> binary spawned -> build-start reached). Co-Authored-By: Claude Opus 4.8 --- .changeset/sdk-6948-binary-url-fallback.md | 7 ++++ .../browserstack-service/src/cli/cliUtils.ts | 32 +++++++++++++++-- .../tests/cli/cliUtils.test.ts | 35 +++++++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 .changeset/sdk-6948-binary-url-fallback.md diff --git a/.changeset/sdk-6948-binary-url-fallback.md b/.changeset/sdk-6948-binary-url-fallback.md new file mode 100644 index 00000000..0700043f --- /dev/null +++ b/.changeset/sdk-6948-binary-url-fallback.md @@ -0,0 +1,7 @@ +--- +"@wdio/browserstack-service": patch +--- + +fix(cli): fall back to `BROWSERSTACK_BINARY_URL` when `update_cli` fails (SDK-6948) + +`update_cli` runs during bootstrap — before the binary is spawned and before GRR localizes the API hosts — so it always targets production `api.browserstack.com`. On an internal staging run (`BROWSERSTACK_STAGING_ENV`) the staging credentials are rejected there (`401`), which left the CLI binary path empty and crashed the launcher with `spawn('') … ERR_INVALID_ARG_VALUE: 'file' cannot be empty`. When `update_cli` fails and `BROWSERSTACK_BINARY_URL` is set, download the binary from that URL instead so the run can proceed. Opt-in only — with the variable unset the original error propagates exactly as before. diff --git a/packages/browserstack-service/src/cli/cliUtils.ts b/packages/browserstack-service/src/cli/cliUtils.ts index 03b335f2..73d11149 100644 --- a/packages/browserstack-service/src/cli/cliUtils.ts +++ b/packages/browserstack-service/src/cli/cliUtils.ts @@ -252,14 +252,40 @@ export class CLIUtils { } queryParams.cli_version = version } - const response = await this.requestToUpdateCLI(queryParams, config) + const browserStackBinaryUrl = + process.env.BROWSERSTACK_BINARY_URL || null + + let response + try { + response = await this.requestToUpdateCLI(queryParams, config) + } catch (err) { + // update_cli runs during bootstrap — before the binary is spawned and before GRR + // localizes the API hosts — so it always targets the production api host. On an internal + // staging run (BROWSERSTACK_STAGING_ENV) the staging creds are rejected there (401). If an + // explicit binary URL was supplied, use it so the run is not blocked on this call. Opt-in + // only: with no BROWSERSTACK_BINARY_URL the error propagates exactly as before, so + // production behaviour is unchanged. + if (!isNullOrEmpty(browserStackBinaryUrl)) { + const status = (err as { response?: { statusCode?: number } })?.response?.statusCode ?? (err as Error)?.message + logger.warn( + `update_cli request failed (${status}); falling back to BROWSERSTACK_BINARY_URL`, + ) + const fallbackBinaryPath = await this.downloadLatestBinary( + browserStackBinaryUrl as string, + cliDir, + ) + PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) + return fallbackBinaryPath + } + PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) + throw err + } + if (nestedKeyValue(response, ['updated_cli_version'])) { logger.debug( `Need to update binary, current binary version: ${queryParams.cli_version}`, ) - const browserStackBinaryUrl = - process.env.BROWSERSTACK_BINARY_URL || null if (!isNullOrEmpty(browserStackBinaryUrl)) { logger.debug( `Using BROWSERSTACK_BINARY_URL: ${browserStackBinaryUrl}`, diff --git a/packages/browserstack-service/tests/cli/cliUtils.test.ts b/packages/browserstack-service/tests/cli/cliUtils.test.ts index aa82167c..f924ba2e 100644 --- a/packages/browserstack-service/tests/cli/cliUtils.test.ts +++ b/packages/browserstack-service/tests/cli/cliUtils.test.ts @@ -459,6 +459,41 @@ describe('CLIUtils', () => { mockConfig ) }) + + it('falls back to BROWSERSTACK_BINARY_URL when update_cli fails (SDK-6948)', async () => { + const saved = process.env.BROWSERSTACK_BINARY_URL + process.env.BROWSERSTACK_BINARY_URL = 'https://example.com/staging-binary.zip' + try { + const fallbackPath = '/mock/cli/dir/binary-fallback' + vi.spyOn(CLIUtils, 'runShellCommand').mockResolvedValue('1.0.0') + // update_cli fails (e.g. staging creds against the prod api host -> 401) + vi.spyOn(CLIUtils, 'requestToUpdateCLI').mockRejectedValue({ response: { statusCode: 401 } }) + vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue(fallbackPath) + + const result = await CLIUtils.checkAndUpdateCli(mockExistingPath, mockCliDir, mockConfig) + + expect(result).toBe(fallbackPath) + expect(CLIUtils.downloadLatestBinary).toHaveBeenCalledWith('https://example.com/staging-binary.zip', mockCliDir) + } finally { + if (saved === undefined) { delete process.env.BROWSERSTACK_BINARY_URL } else { process.env.BROWSERSTACK_BINARY_URL = saved } + } + }) + + it('re-throws when update_cli fails and no BROWSERSTACK_BINARY_URL is set (unchanged behaviour)', async () => { + const saved = process.env.BROWSERSTACK_BINARY_URL + delete process.env.BROWSERSTACK_BINARY_URL + try { + const err = new Error('Unauthorized') + vi.spyOn(CLIUtils, 'runShellCommand').mockResolvedValue('1.0.0') + vi.spyOn(CLIUtils, 'requestToUpdateCLI').mockRejectedValue(err) + const downloadSpy = vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue('/should/not/be/used') + + await expect(CLIUtils.checkAndUpdateCli(mockExistingPath, mockCliDir, mockConfig)).rejects.toBe(err) + expect(downloadSpy).not.toHaveBeenCalled() + } finally { + if (saved !== undefined) { process.env.BROWSERSTACK_BINARY_URL = saved } + } + }) }) describe('setupCliPath', () => { From f3694d8401e0d9d4adbf591300ff6529520c8dd0 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Wed, 7 Oct 2026 21:05:03 +0530 Subject: [PATCH 2/4] fix(cli): treat a non-2xx update_cli reply as a failure for the BINARY_URL fallback (SDK-6948) requestToUpdateCLI resolved a 401 as a plain JSON body ({"message":"Unauthorized"}), so the BROWSERSTACK_BINARY_URL fallback added earlier in this PR never ran: with an empty CLI dir the bootstrap failed with "Failed to get CLI binary path", and with an existing binary the stale one was reused silently. - requestToUpdateCLI now rejects on !response.ok, carrying the status code and body. - checkAndUpdateCli also falls back when a reply has neither url nor updated_cli_version (prod always returns both on success). - Without BROWSERSTACK_BINARY_URL a non-2xx reply still keeps the existing binary, exactly as before; network errors still propagate. Co-Authored-By: Claude Opus 5.5 --- .../browserstack-service/src/cli/cliUtils.ts | 45 ++++++++--- .../tests/cli/cliUtils.test.ts | 80 +++++++++++++++++++ 2 files changed, 113 insertions(+), 12 deletions(-) diff --git a/packages/browserstack-service/src/cli/cliUtils.ts b/packages/browserstack-service/src/cli/cliUtils.ts index 73d11149..e3ff447c 100644 --- a/packages/browserstack-service/src/cli/cliUtils.ts +++ b/packages/browserstack-service/src/cli/cliUtils.ts @@ -255,6 +255,18 @@ export class CLIUtils { const browserStackBinaryUrl = process.env.BROWSERSTACK_BINARY_URL || null + const fallbackToBinaryUrl = async (reason: unknown) => { + logger.warn( + `update_cli request failed (${reason}); falling back to BROWSERSTACK_BINARY_URL`, + ) + const fallbackBinaryPath = await this.downloadLatestBinary( + browserStackBinaryUrl as string, + cliDir, + ) + PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) + return fallbackBinaryPath + } + let response try { response = await this.requestToUpdateCLI(queryParams, config) @@ -263,24 +275,27 @@ export class CLIUtils { // localizes the API hosts — so it always targets the production api host. On an internal // staging run (BROWSERSTACK_STAGING_ENV) the staging creds are rejected there (401). If an // explicit binary URL was supplied, use it so the run is not blocked on this call. Opt-in - // only: with no BROWSERSTACK_BINARY_URL the error propagates exactly as before, so - // production behaviour is unchanged. + // only: with no BROWSERSTACK_BINARY_URL behaviour is exactly as before. + const statusCode = (err as { response?: { statusCode?: number } })?.response?.statusCode if (!isNullOrEmpty(browserStackBinaryUrl)) { - const status = (err as { response?: { statusCode?: number } })?.response?.statusCode ?? (err as Error)?.message - logger.warn( - `update_cli request failed (${status}); falling back to BROWSERSTACK_BINARY_URL`, - ) - const fallbackBinaryPath = await this.downloadLatestBinary( - browserStackBinaryUrl as string, - cliDir, - ) - PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) - return fallbackBinaryPath + return fallbackToBinaryUrl(statusCode ?? (err as Error)?.message) } PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) + if (statusCode !== undefined) { + // A non-2xx reply used to be returned as a body without `updated_cli_version`, + // which kept the existing binary — preserve that. + return existingCliPath + } throw err } + // A successful update_cli reply always carries `updated_cli_version` and `url`; a body + // with neither is an error payload, so honour the explicit binary URL here too. + if (!isNullOrEmpty(browserStackBinaryUrl) && + !nestedKeyValue(response, ['updated_cli_version']) && !nestedKeyValue(response, ['url'])) { + return fallbackToBinaryUrl(JSON.stringify(response)) + } + if (nestedKeyValue(response, ['updated_cli_version'])) { logger.debug( `Need to update binary, current binary version: ${queryParams.cli_version}`, @@ -456,6 +471,12 @@ export class CLIUtils { ) const jsonResponse = await response.json() logger.debug(`response ${JSON.stringify(jsonResponse)}`) + if (!response.ok) { + throw Object.assign( + new Error(`update_cli request failed with status ${response.status}`), + { response: { statusCode: response.status, body: jsonResponse } }, + ) + } return jsonResponse } diff --git a/packages/browserstack-service/tests/cli/cliUtils.test.ts b/packages/browserstack-service/tests/cli/cliUtils.test.ts index f924ba2e..51672b8f 100644 --- a/packages/browserstack-service/tests/cli/cliUtils.test.ts +++ b/packages/browserstack-service/tests/cli/cliUtils.test.ts @@ -496,6 +496,71 @@ describe('CLIUtils', () => { }) }) + describe('checkAndUpdateCli update_cli 401 (SDK-6948)', () => { + const mockCliDir = '/mock/cli/dir' + const mockExistingPath = '/mock/cli/dir/binary-existing' + const config = { user: 'testuser', key: 'testkey' } as Options.Testrunner + const savedBinaryUrl = process.env.BROWSERSTACK_BINARY_URL + const savedFetch = global.fetch + + beforeEach(() => { + vi.spyOn(PerformanceTester, 'start').mockImplementation(() => {}) + vi.spyOn(PerformanceTester, 'end').mockImplementation(() => {}) + vi.spyOn(CLIUtils, 'runShellCommand').mockResolvedValue('1.0.0') + vi.spyOn(CLIUtils, 'isBinaryBusy').mockReturnValue(false) + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 401, + json: vi.fn().mockResolvedValue({ message: 'Unauthorized' }) + }) + }) + + afterEach(() => { + global.fetch = savedFetch + if (savedBinaryUrl === undefined) { + delete process.env.BROWSERSTACK_BINARY_URL + } else { + process.env.BROWSERSTACK_BINARY_URL = savedBinaryUrl + } + vi.restoreAllMocks() + }) + + it('falls back to BROWSERSTACK_BINARY_URL when update_cli answers 401 with a JSON body', async () => { + process.env.BROWSERSTACK_BINARY_URL = 'https://example.com/staging-binary.zip' + const download = vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue('/mock/cli/dir/binary-fallback') + + const result = await CLIUtils.checkAndUpdateCli('', mockCliDir, config) + + expect(result).toBe('/mock/cli/dir/binary-fallback') + expect(download).toHaveBeenCalledWith('https://example.com/staging-binary.zip', mockCliDir) + }) + + it('keeps the existing binary on a 401 when BROWSERSTACK_BINARY_URL is unset (unchanged behaviour)', async () => { + delete process.env.BROWSERSTACK_BINARY_URL + const download = vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue('/should/not/be/used') + + const result = await CLIUtils.checkAndUpdateCli(mockExistingPath, mockCliDir, config) + + expect(result).toBe(mockExistingPath) + expect(download).not.toHaveBeenCalled() + }) + + it('falls back when a 2xx body has neither url nor updated_cli_version', async () => { + process.env.BROWSERSTACK_BINARY_URL = 'https://example.com/staging-binary.zip' + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: vi.fn().mockResolvedValue({ message: 'Unauthorized' }) + }) + const download = vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue('/mock/cli/dir/binary-fallback') + + const result = await CLIUtils.checkAndUpdateCli(mockExistingPath, mockCliDir, config) + + expect(result).toBe('/mock/cli/dir/binary-fallback') + expect(download).toHaveBeenCalledWith('https://example.com/staging-binary.zip', mockCliDir) + }) + }) + describe('setupCliPath', () => { const mockConfig = {} as Options.Testrunner @@ -562,6 +627,7 @@ describe('CLIUtils', () => { // Mock fetch to return a mock response global.fetch = vi.fn().mockResolvedValue({ + ok: true, json: vi.fn().mockResolvedValue({ status: 'success' }) }) }) @@ -578,6 +644,7 @@ describe('CLIUtils', () => { const mockJsonResponse = { updated_cli_version: '2.0.0' } global.fetch = vi.fn().mockResolvedValue({ + ok: true, json: vi.fn().mockResolvedValue(mockJsonResponse) }) @@ -601,6 +668,7 @@ describe('CLIUtils', () => { it('returns response from fetch', async () => { const mockResponse = { updated_cli_version: '2.0.0' } global.fetch = vi.fn().mockResolvedValue({ + ok: true, json: vi.fn().mockResolvedValue(mockResponse) }) @@ -609,6 +677,18 @@ describe('CLIUtils', () => { expect(result).toEqual(mockResponse) }) + it('rejects with the status code on a non-2xx reply (SDK-6948)', async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 401, + json: vi.fn().mockResolvedValue({ message: 'Unauthorized' }) + }) + + await expect(CLIUtils.requestToUpdateCLI({}, mockConfig)) + .rejects + .toMatchObject({ response: { statusCode: 401, body: { message: 'Unauthorized' } } }) + }) + it('handles errors from fetch', async () => { const mockError = new Error('Network error') global.fetch = vi.fn().mockRejectedValue(mockError) From 4eb68d26f735fc4938b9000f3d46d9c345d7bae6 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Thu, 8 Oct 2026 12:53:14 +0530 Subject: [PATCH 3/4] fix(cli): address review on update_cli fallback (SDK-6948) - requestToUpdateCLI checks response.ok before parsing, so an HTML/empty error page keeps its status code instead of surfacing as a SyntaxError. - checkAndUpdateCli closes the SDK_CLI_CHECK_UPDATE span once in a finally, including when the fallback download rejects. - Tests: clear HTTP(S)_PROXY so _fetch cannot bypass the global.fetch mock, assert the mock was hit, and cover no-cached-binary 401, rejected fallback download, empty BROWSERSTACK_BINARY_URL and a non-JSON 5xx. - Changeset rewritten for the v9 behaviour. Co-Authored-By: Claude Opus 5.5 --- .changeset/sdk-6948-binary-url-fallback.md | 2 +- .../browserstack-service/src/cli/cliUtils.ts | 160 +++++++++--------- .../tests/cli/cliUtils.test.ts | 60 +++++++ 3 files changed, 140 insertions(+), 82 deletions(-) diff --git a/.changeset/sdk-6948-binary-url-fallback.md b/.changeset/sdk-6948-binary-url-fallback.md index 0700043f..586f84ac 100644 --- a/.changeset/sdk-6948-binary-url-fallback.md +++ b/.changeset/sdk-6948-binary-url-fallback.md @@ -4,4 +4,4 @@ fix(cli): fall back to `BROWSERSTACK_BINARY_URL` when `update_cli` fails (SDK-6948) -`update_cli` runs during bootstrap — before the binary is spawned and before GRR localizes the API hosts — so it always targets production `api.browserstack.com`. On an internal staging run (`BROWSERSTACK_STAGING_ENV`) the staging credentials are rejected there (`401`), which left the CLI binary path empty and crashed the launcher with `spawn('') … ERR_INVALID_ARG_VALUE: 'file' cannot be empty`. When `update_cli` fails and `BROWSERSTACK_BINARY_URL` is set, download the binary from that URL instead so the run can proceed. Opt-in only — with the variable unset the original error propagates exactly as before. +`update_cli` runs during bootstrap against production `api.browserstack.com`, so on an internal staging run (`BROWSERSTACK_STAGING_ENV`) it answers `401`. The SDK then found no CLI binary and fell back to the non-CLI flow, or silently reused a stale cached binary. When `update_cli` fails (non-2xx, network error, or a reply with neither `url` nor `updated_cli_version`) and `BROWSERSTACK_BINARY_URL` is set, the binary is now downloaded from that URL. `CLIUtils.requestToUpdateCLI` now rejects on a non-2xx reply (with `response.statusCode`) instead of resolving the error body. With `BROWSERSTACK_BINARY_URL` unset, behaviour is unchanged: a non-2xx reply keeps the existing binary, and network errors propagate. diff --git a/packages/browserstack-service/src/cli/cliUtils.ts b/packages/browserstack-service/src/cli/cliUtils.ts index e3ff447c..7ba639b7 100644 --- a/packages/browserstack-service/src/cli/cliUtils.ts +++ b/packages/browserstack-service/src/cli/cliUtils.ts @@ -227,97 +227,91 @@ export class CLIUtils { PerformanceTester.start(PerformanceEvents.SDK_CLI_CHECK_UPDATE) logger.info(`Current CLI Path Found: ${existingCliPath}`) - const queryParams: Record = { - sdk_version: CLIUtils.getSdkVersion(), - os: platform(), - os_arch: arch(), - cli_version: '0', - sdk_language: this.getSdkLanguage(), - } - if (!isNullOrEmpty(existingCliPath)) { - // If binary is busy (being executed by another process), skip version check - // and API call entirely — use existing binary as-is - if (this.isBinaryBusy(existingCliPath)) { - logger.warn(`Existing binary is currently in use, skipping update: ${existingCliPath}`) - PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) - return existingCliPath + // Single close for the span on every path, including a rejected download. + try { + const queryParams: Record = { + sdk_version: CLIUtils.getSdkVersion(), + os: platform(), + os_arch: arch(), + cli_version: '0', + sdk_language: this.getSdkLanguage(), } - const version = await this.runShellCommand( - `${existingCliPath} version`, - ) - if (version.toLowerCase().includes('text file busy')) { - logger.warn(`Binary busy during version check, skipping update: ${existingCliPath}`) - PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) - return existingCliPath + if (!isNullOrEmpty(existingCliPath)) { + // If binary is busy (being executed by another process), skip version check + // and API call entirely — use existing binary as-is + if (this.isBinaryBusy(existingCliPath)) { + logger.warn(`Existing binary is currently in use, skipping update: ${existingCliPath}`) + return existingCliPath + } + const version = await this.runShellCommand( + `${existingCliPath} version`, + ) + if (version.toLowerCase().includes('text file busy')) { + logger.warn(`Binary busy during version check, skipping update: ${existingCliPath}`) + return existingCliPath + } + queryParams.cli_version = version } - queryParams.cli_version = version - } - const browserStackBinaryUrl = - process.env.BROWSERSTACK_BINARY_URL || null + const browserStackBinaryUrl = + process.env.BROWSERSTACK_BINARY_URL || null - const fallbackToBinaryUrl = async (reason: unknown) => { - logger.warn( - `update_cli request failed (${reason}); falling back to BROWSERSTACK_BINARY_URL`, - ) - const fallbackBinaryPath = await this.downloadLatestBinary( - browserStackBinaryUrl as string, - cliDir, - ) - PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) - return fallbackBinaryPath - } + const fallbackToBinaryUrl = async (reason: unknown) => { + logger.warn( + `update_cli request failed (${reason}); falling back to BROWSERSTACK_BINARY_URL`, + ) + return this.downloadLatestBinary(browserStackBinaryUrl as string, cliDir) + } - let response - try { - response = await this.requestToUpdateCLI(queryParams, config) - } catch (err) { - // update_cli runs during bootstrap — before the binary is spawned and before GRR - // localizes the API hosts — so it always targets the production api host. On an internal - // staging run (BROWSERSTACK_STAGING_ENV) the staging creds are rejected there (401). If an - // explicit binary URL was supplied, use it so the run is not blocked on this call. Opt-in - // only: with no BROWSERSTACK_BINARY_URL behaviour is exactly as before. - const statusCode = (err as { response?: { statusCode?: number } })?.response?.statusCode - if (!isNullOrEmpty(browserStackBinaryUrl)) { - return fallbackToBinaryUrl(statusCode ?? (err as Error)?.message) + let response + try { + response = await this.requestToUpdateCLI(queryParams, config) + } catch (err) { + // update_cli runs during bootstrap — before the binary is spawned and before GRR + // localizes the API hosts — so it always targets the production api host. On an internal + // staging run (BROWSERSTACK_STAGING_ENV) the staging creds are rejected there (401). If an + // explicit binary URL was supplied, use it so the run is not blocked on this call. Opt-in + // only: with no BROWSERSTACK_BINARY_URL behaviour is exactly as before. + const statusCode = (err as { response?: { statusCode?: number } })?.response?.statusCode + if (!isNullOrEmpty(browserStackBinaryUrl)) { + return await fallbackToBinaryUrl(statusCode ?? (err as Error)?.message) + } + if (statusCode !== undefined) { + // A non-2xx reply used to be returned as a body without `updated_cli_version`, + // which kept the existing binary — preserve that. + return existingCliPath + } + throw err } - PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) - if (statusCode !== undefined) { - // A non-2xx reply used to be returned as a body without `updated_cli_version`, - // which kept the existing binary — preserve that. - return existingCliPath + + // A successful update_cli reply always carries `updated_cli_version` and `url`; a body + // with neither is an error payload, so honour the explicit binary URL here too. + if (!isNullOrEmpty(browserStackBinaryUrl) && + !nestedKeyValue(response, ['updated_cli_version']) && !nestedKeyValue(response, ['url'])) { + return await fallbackToBinaryUrl(JSON.stringify(response)) } - throw err - } - // A successful update_cli reply always carries `updated_cli_version` and `url`; a body - // with neither is an error payload, so honour the explicit binary URL here too. - if (!isNullOrEmpty(browserStackBinaryUrl) && - !nestedKeyValue(response, ['updated_cli_version']) && !nestedKeyValue(response, ['url'])) { - return fallbackToBinaryUrl(JSON.stringify(response)) - } + if (nestedKeyValue(response, ['updated_cli_version'])) { + logger.debug( + `Need to update binary, current binary version: ${queryParams.cli_version}`, + ) - if (nestedKeyValue(response, ['updated_cli_version'])) { - logger.debug( - `Need to update binary, current binary version: ${queryParams.cli_version}`, - ) + if (!isNullOrEmpty(browserStackBinaryUrl)) { + logger.debug( + `Using BROWSERSTACK_BINARY_URL: ${browserStackBinaryUrl}`, + ) + response.url = browserStackBinaryUrl + } - if (!isNullOrEmpty(browserStackBinaryUrl)) { - logger.debug( - `Using BROWSERSTACK_BINARY_URL: ${browserStackBinaryUrl}`, + return await this.downloadLatestBinary( + nestedKeyValue(response, ['url']), + cliDir, + nestedKeyValue(response, ['updated_cli_version']), ) - response.url = browserStackBinaryUrl } - - const finalBinaryPath = await this.downloadLatestBinary( - nestedKeyValue(response, ['url']), - cliDir, - nestedKeyValue(response, ['updated_cli_version']), - ) + return existingCliPath + } finally { PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) - return finalBinaryPath } - PerformanceTester.end(PerformanceEvents.SDK_CLI_CHECK_UPDATE) - return existingCliPath } static getCliDir() { @@ -469,14 +463,18 @@ export class CLIUtils { `${APIUtils.BROWSERSTACK_AUTOMATE_API_URL}/${UPDATED_CLI_ENDPOINT}?${params.toString()}`, requestInit, ) - const jsonResponse = await response.json() - logger.debug(`response ${JSON.stringify(jsonResponse)}`) if (!response.ok) { + // Check the status before parsing: an LB error page (HTML/empty 5xx) must still carry + // its status code rather than surface as a SyntaxError. + const body = await response.json().catch(() => null) + logger.debug(`response ${response.status} ${JSON.stringify(body)}`) throw Object.assign( new Error(`update_cli request failed with status ${response.status}`), - { response: { statusCode: response.status, body: jsonResponse } }, + { response: { statusCode: response.status, body } }, ) } + const jsonResponse = await response.json() + logger.debug(`response ${JSON.stringify(jsonResponse)}`) return jsonResponse } diff --git a/packages/browserstack-service/tests/cli/cliUtils.test.ts b/packages/browserstack-service/tests/cli/cliUtils.test.ts index 51672b8f..973f92d6 100644 --- a/packages/browserstack-service/tests/cli/cliUtils.test.ts +++ b/packages/browserstack-service/tests/cli/cliUtils.test.ts @@ -502,8 +502,12 @@ describe('CLIUtils', () => { const config = { user: 'testuser', key: 'testkey' } as Options.Testrunner const savedBinaryUrl = process.env.BROWSERSTACK_BINARY_URL const savedFetch = global.fetch + const savedProxy = { http: process.env.HTTP_PROXY, https: process.env.HTTPS_PROXY } beforeEach(() => { + // _fetch routes through undici's ProxyAgent when a proxy var is set, bypassing global.fetch + delete process.env.HTTP_PROXY + delete process.env.HTTPS_PROXY vi.spyOn(PerformanceTester, 'start').mockImplementation(() => {}) vi.spyOn(PerformanceTester, 'end').mockImplementation(() => {}) vi.spyOn(CLIUtils, 'runShellCommand').mockResolvedValue('1.0.0') @@ -517,6 +521,8 @@ describe('CLIUtils', () => { afterEach(() => { global.fetch = savedFetch + if (savedProxy.http !== undefined) { process.env.HTTP_PROXY = savedProxy.http } + if (savedProxy.https !== undefined) { process.env.HTTPS_PROXY = savedProxy.https } if (savedBinaryUrl === undefined) { delete process.env.BROWSERSTACK_BINARY_URL } else { @@ -533,6 +539,7 @@ describe('CLIUtils', () => { expect(result).toBe('/mock/cli/dir/binary-fallback') expect(download).toHaveBeenCalledWith('https://example.com/staging-binary.zip', mockCliDir) + expect(global.fetch).toHaveBeenCalledTimes(1) }) it('keeps the existing binary on a 401 when BROWSERSTACK_BINARY_URL is unset (unchanged behaviour)', async () => { @@ -543,6 +550,7 @@ describe('CLIUtils', () => { expect(result).toBe(mockExistingPath) expect(download).not.toHaveBeenCalled() + expect(global.fetch).toHaveBeenCalledTimes(1) }) it('falls back when a 2xx body has neither url nor updated_cli_version', async () => { @@ -558,6 +566,38 @@ describe('CLIUtils', () => { expect(result).toBe('/mock/cli/dir/binary-fallback') expect(download).toHaveBeenCalledWith('https://example.com/staging-binary.zip', mockCliDir) + expect(global.fetch).toHaveBeenCalledTimes(1) + }) + + it('returns an empty path on a 401 with no cached binary and no BROWSERSTACK_BINARY_URL', async () => { + delete process.env.BROWSERSTACK_BINARY_URL + const download = vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue('/should/not/be/used') + + const result = await CLIUtils.checkAndUpdateCli('', mockCliDir, config) + + expect(result).toBe('') + expect(download).not.toHaveBeenCalled() + expect(global.fetch).toHaveBeenCalledTimes(1) + }) + + it('propagates a rejected fallback download and still closes the span', async () => { + process.env.BROWSERSTACK_BINARY_URL = 'https://example.com/staging-binary.zip' + const downloadErr = new Error('download failed') + vi.spyOn(CLIUtils, 'downloadLatestBinary').mockRejectedValue(downloadErr) + + await expect(CLIUtils.checkAndUpdateCli('', mockCliDir, config)).rejects.toBe(downloadErr) + expect(PerformanceTester.end).toHaveBeenCalledTimes(1) + expect(PerformanceTester.end).toHaveBeenCalledWith(PerformanceEvents.SDK_CLI_CHECK_UPDATE) + }) + + it('treats an empty BROWSERSTACK_BINARY_URL as unset', async () => { + process.env.BROWSERSTACK_BINARY_URL = '' + const download = vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue('/should/not/be/used') + + const result = await CLIUtils.checkAndUpdateCli(mockExistingPath, mockCliDir, config) + + expect(result).toBe(mockExistingPath) + expect(download).not.toHaveBeenCalled() }) }) @@ -621,9 +661,13 @@ describe('CLIUtils', () => { user: 'testuser', key: 'testkey' } as Options.Testrunner + const savedProxy = { http: process.env.HTTP_PROXY, https: process.env.HTTPS_PROXY } beforeEach(() => { vi.resetAllMocks() + // _fetch routes through undici's ProxyAgent when a proxy var is set, bypassing global.fetch + delete process.env.HTTP_PROXY + delete process.env.HTTPS_PROXY // Mock fetch to return a mock response global.fetch = vi.fn().mockResolvedValue({ @@ -633,6 +677,8 @@ describe('CLIUtils', () => { }) afterEach(() => { + if (savedProxy.http !== undefined) { process.env.HTTP_PROXY = savedProxy.http } + if (savedProxy.https !== undefined) { process.env.HTTPS_PROXY = savedProxy.https } vi.clearAllMocks() }) @@ -687,6 +733,20 @@ describe('CLIUtils', () => { await expect(CLIUtils.requestToUpdateCLI({}, mockConfig)) .rejects .toMatchObject({ response: { statusCode: 401, body: { message: 'Unauthorized' } } }) + expect(global.fetch).toHaveBeenCalledTimes(1) + }) + + it('keeps the status code when a non-2xx reply has a non-JSON body (SDK-6948)', async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 502, + json: vi.fn().mockRejectedValue(new SyntaxError('Unexpected token <')) + }) + + await expect(CLIUtils.requestToUpdateCLI({}, mockConfig)) + .rejects + .toMatchObject({ response: { statusCode: 502, body: null } }) + expect(global.fetch).toHaveBeenCalledTimes(1) }) it('handles errors from fetch', async () => { From 5c67d0763aaf361be5becaa335010520bd167deb Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Thu, 8 Oct 2026 13:01:39 +0530 Subject: [PATCH 4/4] fix(cli): keep the cached binary on an up-to-date reply or failed fallback download (SDK-6948) - A 2xx update_cli reply with neither url nor updated_cli_version keeps the cached binary; BROWSERSTACK_BINARY_URL is only fetched when none is cached (python #1282 parity). - If the BROWSERSTACK_BINARY_URL download fails and a cached binary exists, use it instead of failing the CLI setup. - Tests for both, plus the no-cached-binary 2xx fallback. Co-Authored-By: Claude Opus 5.5 --- .changeset/sdk-6948-binary-url-fallback.md | 2 +- .../browserstack-service/src/cli/cliUtils.ts | 18 ++++++++--- .../tests/cli/cliUtils.test.ts | 30 +++++++++++++++++-- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/.changeset/sdk-6948-binary-url-fallback.md b/.changeset/sdk-6948-binary-url-fallback.md index 586f84ac..442acf26 100644 --- a/.changeset/sdk-6948-binary-url-fallback.md +++ b/.changeset/sdk-6948-binary-url-fallback.md @@ -4,4 +4,4 @@ fix(cli): fall back to `BROWSERSTACK_BINARY_URL` when `update_cli` fails (SDK-6948) -`update_cli` runs during bootstrap against production `api.browserstack.com`, so on an internal staging run (`BROWSERSTACK_STAGING_ENV`) it answers `401`. The SDK then found no CLI binary and fell back to the non-CLI flow, or silently reused a stale cached binary. When `update_cli` fails (non-2xx, network error, or a reply with neither `url` nor `updated_cli_version`) and `BROWSERSTACK_BINARY_URL` is set, the binary is now downloaded from that URL. `CLIUtils.requestToUpdateCLI` now rejects on a non-2xx reply (with `response.statusCode`) instead of resolving the error body. With `BROWSERSTACK_BINARY_URL` unset, behaviour is unchanged: a non-2xx reply keeps the existing binary, and network errors propagate. +`update_cli` runs during bootstrap against production `api.browserstack.com`, so on an internal staging run (`BROWSERSTACK_STAGING_ENV`) it answers `401`. The SDK then found no CLI binary and fell back to the non-CLI flow, or silently reused a stale cached binary. When `BROWSERSTACK_BINARY_URL` is set and `update_cli` fails (non-2xx or network error), the binary is now downloaded from that URL. If that download fails, a cached binary is used when there is one. A 2xx reply with neither `url` nor `updated_cli_version` keeps the cached binary, and only downloads from the URL when none is cached. `CLIUtils.requestToUpdateCLI` now rejects on a non-2xx reply (with `response.statusCode`) instead of resolving the error body. With `BROWSERSTACK_BINARY_URL` unset, behaviour is unchanged: a non-2xx reply keeps the existing binary, and network errors propagate. diff --git a/packages/browserstack-service/src/cli/cliUtils.ts b/packages/browserstack-service/src/cli/cliUtils.ts index 7ba639b7..a5608bba 100644 --- a/packages/browserstack-service/src/cli/cliUtils.ts +++ b/packages/browserstack-service/src/cli/cliUtils.ts @@ -259,7 +259,17 @@ export class CLIUtils { logger.warn( `update_cli request failed (${reason}); falling back to BROWSERSTACK_BINARY_URL`, ) - return this.downloadLatestBinary(browserStackBinaryUrl as string, cliDir) + try { + return await this.downloadLatestBinary(browserStackBinaryUrl as string, cliDir) + } catch (downloadErr) { + if (isNullOrEmpty(existingCliPath)) { + throw downloadErr + } + logger.warn( + `BROWSERSTACK_BINARY_URL download failed (${util.format(downloadErr)}); using cached binary ${existingCliPath}`, + ) + return existingCliPath + } } let response @@ -283,9 +293,9 @@ export class CLIUtils { throw err } - // A successful update_cli reply always carries `updated_cli_version` and `url`; a body - // with neither is an error payload, so honour the explicit binary URL here too. - if (!isNullOrEmpty(browserStackBinaryUrl) && + // A 2xx reply with neither `updated_cli_version` nor `url` means there is nothing to update: + // keep a cached binary, and only fetch from the explicit binary URL when there is none. + if (!isNullOrEmpty(browserStackBinaryUrl) && isNullOrEmpty(existingCliPath) && !nestedKeyValue(response, ['updated_cli_version']) && !nestedKeyValue(response, ['url'])) { return await fallbackToBinaryUrl(JSON.stringify(response)) } diff --git a/packages/browserstack-service/tests/cli/cliUtils.test.ts b/packages/browserstack-service/tests/cli/cliUtils.test.ts index 973f92d6..99f52393 100644 --- a/packages/browserstack-service/tests/cli/cliUtils.test.ts +++ b/packages/browserstack-service/tests/cli/cliUtils.test.ts @@ -553,7 +553,23 @@ describe('CLIUtils', () => { expect(global.fetch).toHaveBeenCalledTimes(1) }) - it('falls back when a 2xx body has neither url nor updated_cli_version', async () => { + it('keeps the cached binary when a 2xx body has neither url nor updated_cli_version', async () => { + process.env.BROWSERSTACK_BINARY_URL = 'https://example.com/staging-binary.zip' + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: vi.fn().mockResolvedValue({ success: true }) + }) + const download = vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue('/should/not/be/used') + + const result = await CLIUtils.checkAndUpdateCli(mockExistingPath, mockCliDir, config) + + expect(result).toBe(mockExistingPath) + expect(download).not.toHaveBeenCalled() + expect(global.fetch).toHaveBeenCalledTimes(1) + }) + + it('falls back when a 2xx body has neither url nor updated_cli_version and no binary is cached', async () => { process.env.BROWSERSTACK_BINARY_URL = 'https://example.com/staging-binary.zip' global.fetch = vi.fn().mockResolvedValue({ ok: true, @@ -562,13 +578,23 @@ describe('CLIUtils', () => { }) const download = vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue('/mock/cli/dir/binary-fallback') - const result = await CLIUtils.checkAndUpdateCli(mockExistingPath, mockCliDir, config) + const result = await CLIUtils.checkAndUpdateCli('', mockCliDir, config) expect(result).toBe('/mock/cli/dir/binary-fallback') expect(download).toHaveBeenCalledWith('https://example.com/staging-binary.zip', mockCliDir) expect(global.fetch).toHaveBeenCalledTimes(1) }) + it('uses the cached binary when the fallback download fails', async () => { + process.env.BROWSERSTACK_BINARY_URL = 'https://example.com/staging-binary.zip' + vi.spyOn(CLIUtils, 'downloadLatestBinary').mockRejectedValue(new Error('download failed')) + + const result = await CLIUtils.checkAndUpdateCli(mockExistingPath, mockCliDir, config) + + expect(result).toBe(mockExistingPath) + expect(PerformanceTester.end).toHaveBeenCalledTimes(1) + }) + it('returns an empty path on a 401 with no cached binary and no BROWSERSTACK_BINARY_URL', async () => { delete process.env.BROWSERSTACK_BINARY_URL const download = vi.spyOn(CLIUtils, 'downloadLatestBinary').mockResolvedValue('/should/not/be/used')