From 53fed741cbaa76a26bf7b79bfaff10b64b6a1e50 Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Tue, 15 Sep 2026 22:17:25 +0530 Subject: [PATCH 1/5] feat(browserstack-service): render end-of-build summary entries (SDK-7358) The binary returns CustomerVisibleSummaryEntry items on StopBinSessionResponse for end-of-build messages such as the SDK version nudge. The service had neither the proto field nor a renderer, so those messages were dropped for every wdio customer running through the CLI path. Adds the proto field and a renderer that writes `body` verbatim, picks the stream from `severity` (warn/warning/error -> stderr, everything else including unknown -> stdout so a malformed severity cannot trip CI stderr watchers), and archives a copy to the log file for runners that keep only the log directory. Deliberately does not branch on `entry_type`, so future entry types need no further service change. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/cli/grpcClient.ts | 52 +++++++++++++ .../browserstack/sdk/v1/sdk-messages.proto | 18 +++++ .../tests/cli/grpcClient.test.ts | 73 +++++++++++++++++++ 3 files changed, 143 insertions(+) diff --git a/packages/browserstack-service/src/cli/grpcClient.ts b/packages/browserstack-service/src/cli/grpcClient.ts index 51243188..2ad167d2 100644 --- a/packages/browserstack-service/src/cli/grpcClient.ts +++ b/packages/browserstack-service/src/cli/grpcClient.ts @@ -277,6 +277,7 @@ export class GrpcClient { try { const response = await stopBinSessionPromise(request) this.logger.info('StopBinSession successful') + this.renderCustomerVisibleSummary(response) PerformanceTester.end(PERFORMANCE_SDK_EVENTS.EVENTS.SDK_CLI_ON_STOP) return response } catch (error: unknown) { @@ -291,6 +292,52 @@ export class GrpcClient { } } + /** + * Render end-of-build customer-visible summary entries. + * + * Per the binary proto contract (CustomerVisibleSummaryEntry in + * sdk-messages.proto): iterate by `severity` + `body`, write `body` verbatim, + * and pick the stream from `severity`. Never branches on `entryType`, so new + * entry types need no SDK change. + * @private + */ + private renderCustomerVisibleSummary(response: unknown) { + try { + const entries = (response as { entries?: Array<{ severity?: string, body?: string }> })?.entries + if (!entries?.length) { + return + } + + for (const entry of entries) { + const body = entry?.body || '' + if (!body) { + continue + } + + const severity = (entry?.severity || 'info').toLowerCase() + // warn/warning/error -> stderr, everything else (info AND unknown) -> + // stdout, so a malformed severity cannot false-alarm CI tooling + // watching stderr. + const isErrorStream = severity === 'warn' || severity === 'warning' || severity === 'error' + // Written directly rather than through the logger, whose per-line + // prefix would break the binary's box-border alignment. + ;(isErrorStream ? process.stderr : process.stdout).write(`${body}\n`) + + // Archived copy — terminal scrollback is lost on CI runners that + // keep only the log directory. + if (severity === 'error') { + this.logger.error(body) + } else if (isErrorStream) { + this.logger.warn(body) + } else { + this.logger.info(body) + } + } + } catch (error: unknown) { + this.logger.debug(`StopBinSession entries forwarding failed: ${util.format(error)}`) + } + } + async testSessionEvent(data: Omit) { PerformanceTester.start(PERFORMANCE_SDK_EVENTS.DISPATCHER_EVENTS.TEST_SESSION) const workerId = this.getClientWorkerIdFromContext(data.executionContext) @@ -496,6 +543,11 @@ export class GrpcClient { message: log.message, timestamp: log.timestamp, level: log.level, + // Attachment entries carry no message — the binary streams the file + // from filePath when it drains its upload queue. + fileName: log.fileName, + fileSize: log.fileSize, + filePath: log.filePath, }) logEntries.push(logEntry) } diff --git a/packages/browserstack-service/src/proto/browserstack/sdk/v1/sdk-messages.proto b/packages/browserstack-service/src/proto/browserstack/sdk/v1/sdk-messages.proto index f6d840e5..b9dc77bd 100644 --- a/packages/browserstack-service/src/proto/browserstack/sdk/v1/sdk-messages.proto +++ b/packages/browserstack-service/src/proto/browserstack/sdk/v1/sdk-messages.proto @@ -157,6 +157,24 @@ message StopBinSessionResponse { optional string error = 2; optional string automate_buildlink = 3; optional string hashed_id = 4; + // End-of-build customer-visible summary entries. Populated on EVERY response + // shape — success, error, and clean-build alike (empty when nothing to + // surface). Iterate by `severity` + `body`; do not branch on `entry_type`, + // so new entry types need no SDK change. + repeated CustomerVisibleSummaryEntry entries = 5; +} + +// A single customer-visible summary entry surfaced at end of build via +// StopBinSessionResponse.entries. The binary owns the prose so all SDKs render +// consistent text; SDKs choose the output stream from `severity`. +message CustomerVisibleSummaryEntry { + // Stable machine-readable identifier (e.g. "network_restrictions"). + string entry_type = 1; + // One of: "info" | "warn" | "error". + string severity = 2; + // Pre-formatted block to display verbatim. May contain embedded newlines. + string body = 3; + optional string doc_link = 4; } message ConnectBinSessionRequest { diff --git a/packages/browserstack-service/tests/cli/grpcClient.test.ts b/packages/browserstack-service/tests/cli/grpcClient.test.ts index e63ab79f..af6cc056 100644 --- a/packages/browserstack-service/tests/cli/grpcClient.test.ts +++ b/packages/browserstack-service/tests/cli/grpcClient.test.ts @@ -51,3 +51,76 @@ describe('GrpcClient.stopBinSession', () => { expect(request.exitReason).toBeUndefined() }) }) + +describe('GrpcClient.stopBinSession customer-visible summary entries', () => { + let client: GrpcClient + let stdoutSpy: ReturnType + let stderrSpy: ReturnType + + const respondWith = (response: unknown) => { + client.client = { + stopBinSession: vi.fn((_req: unknown, cb: (err: unknown, res: unknown) => void) => cb(null, response)) + } as any + } + + beforeEach(() => { + client = new GrpcClient() + client.binSessionId = 'bin-1' + stdoutSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true) + stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) + }) + + afterEach(() => { + stdoutSpy.mockRestore() + stderrSpy.mockRestore() + vi.clearAllMocks() + }) + + it('writes the body verbatim to stdout for an info entry', async () => { + respondWith({ entries: [{ entryType: 'version_nudge', severity: 'info', body: 'line one\nline two' }] }) + await client.stopBinSession() + expect(stdoutSpy).toHaveBeenCalledWith('line one\nline two\n') + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it('routes warn and error entries to stderr', async () => { + respondWith({ entries: [ + { entryType: 'version_nudge', severity: 'warn', body: 'outdated' }, + { entryType: 'version_nudge', severity: 'error', body: 'deprecated' } + ] }) + await client.stopBinSession() + expect(stderrSpy).toHaveBeenCalledWith('outdated\n') + expect(stderrSpy).toHaveBeenCalledWith('deprecated\n') + expect(stdoutSpy).not.toHaveBeenCalled() + }) + + it('treats the server\'s "warning" spelling as an error stream', async () => { + respondWith({ entries: [{ entryType: 'version_nudge', severity: 'warning', body: 'outdated' }] }) + await client.stopBinSession() + expect(stderrSpy).toHaveBeenCalledWith('outdated\n') + }) + + it('sends an unknown severity to stdout so CI stderr watchers are not tripped', async () => { + respondWith({ entries: [{ entryType: 'version_nudge', severity: 'bogus', body: 'body' }] }) + await client.stopBinSession() + expect(stdoutSpy).toHaveBeenCalledWith('body\n') + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it('writes nothing when entries are absent, empty, or bodiless', async () => { + for (const response of [{ done: true }, { entries: [] }, { entries: [{ severity: 'warn', body: '' }] }]) { + respondWith(response) + await client.stopBinSession() + } + expect(stdoutSpy).not.toHaveBeenCalled() + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it('still returns the response when rendering throws', async () => { + stdoutSpy.mockImplementation(() => { + throw new Error('stream closed') + }) + respondWith({ entries: [{ severity: 'info', body: 'body' }], done: true }) + await expect(client.stopBinSession()).resolves.toMatchObject({ done: true }) + }) +}) From 4d39f2a26ca7d92bb707fe24a7701093fa462cf3 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 16 Sep 2026 08:06:55 +0000 Subject: [PATCH 2/5] chore(changeset): auto-generate from PR template (minor) --- .changeset/pr-200.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/pr-200.md diff --git a/.changeset/pr-200.md b/.changeset/pr-200.md new file mode 100644 index 00000000..b167daeb --- /dev/null +++ b/.changeset/pr-200.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": minor +--- + +- End-of-build messages from BrowserStack — such as a notice that your SDK version is outdated or has a known issue — are now shown at the end of your test run and written to the SDK log. From acf0d35a0ce8cade6a4d36dc646cae98c544b1d5 Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Thu, 24 Sep 2026 13:10:51 +0530 Subject: [PATCH 3/5] feat(browserstack-service): tint summary entries and archive them once (SDK-7358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to the end-of-build summary rendering added in this PR. Colour: a warn entry (an outdated SDK) is tinted yellow and an error entry (a deprecated one) red, with the block's first line of text emphasised. Lines are wrapped and reset individually rather than the block as a whole, so a truncated or interleaved write cannot leave the customer's terminal stuck in colour. Applied to the stream copy only — the archived copy stays plain, because escape codes reach a log file as literal bytes and break anchored searches over it. Archiving: switch from the info/warn/error helpers to logToFile. Those helpers also call @wdio/logger, which writes to the console, so the customer saw the block twice — once raw from the stream write, once prefixed by the logger. logToFile writes to the log file only. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/cli/grpcClient.ts | 66 ++++++++++++++++--- .../tests/cli/grpcClient.test.ts | 53 +++++++++++++-- 2 files changed, 106 insertions(+), 13 deletions(-) diff --git a/packages/browserstack-service/src/cli/grpcClient.ts b/packages/browserstack-service/src/cli/grpcClient.ts index 2ad167d2..3ed671ff 100644 --- a/packages/browserstack-service/src/cli/grpcClient.ts +++ b/packages/browserstack-service/src/cli/grpcClient.ts @@ -47,6 +47,14 @@ import { BStackLogger } from './cliLogger.js' const GRPC_MESSAGE_LIMIT = 20 * 1024 * 1024 // 20 MB in bytes +// Explicit \x1b escapes so the ESC byte stays visible in source. Applied to the +// terminal copy of a summary entry only; the archived copy stays plain. +const SUMMARY_ANSI = { + reset: '\x1b[0m', + error: { base: '\x1b[31m', emphasis: '\x1b[1;31m' }, + warn: { base: '\x1b[33m', emphasis: '\x1b[1;33m' } +} + /** * GrpcClient - Singleton class for managing gRPC client connections * @@ -320,24 +328,64 @@ export class GrpcClient { // watching stderr. const isErrorStream = severity === 'warn' || severity === 'warning' || severity === 'error' // Written directly rather than through the logger, whose per-line - // prefix would break the binary's box-border alignment. - ;(isErrorStream ? process.stderr : process.stdout).write(`${body}\n`) + // prefix would break the binary's box-border alignment. Colour is + // applied here only — the archived copy below stays plain. + ;(isErrorStream ? process.stderr : process.stdout) + .write(`${this.colouriseSummaryBody(body, severity)}\n`) // Archived copy — terminal scrollback is lost on CI runners that // keep only the log directory. - if (severity === 'error') { - this.logger.error(body) - } else if (isErrorStream) { - this.logger.warn(body) - } else { - this.logger.info(body) - } + // + // logToFile, NOT the info/warn/error helpers: those also call + // @wdio/logger, which writes to the console, so the customer + // would see the block twice (once raw above, once prefixed). + this.logger.logToFile(body, severity === 'error' ? 'error' : (isErrorStream ? 'warn' : 'info')) } } catch (error: unknown) { this.logger.debug(`StopBinSession entries forwarding failed: ${util.format(error)}`) } } + /** + * Tint a summary block by severity — yellow for warn (an outdated SDK), red for + * error (a deprecated one), untouched otherwise. + * + * Each line is wrapped and reset on its own rather than the block as a whole, so + * a truncated or interleaved write cannot leave the customer's terminal stuck in + * colour. The first non-blank, non-border line is emphasised. + * @private + */ + private colouriseSummaryBody(body: string, severity: string): string { + try { + const palette = severity === 'error' + ? SUMMARY_ANSI.error + : ((severity === 'warn' || severity === 'warning') ? SUMMARY_ANSI.warn : null) + if (!palette) { + return body + } + + let emphasised = false + return body.split('\n').map((line) => { + const trimmed = line.trim() + if (!trimmed) { + return line + } + // A border is any line carrying no letters or digits, rather than a + // check for the binary's current U+2500 divider — so a change to the + // glyph cannot silently start emphasising the wrong line. + const isBorder = !/[A-Za-z0-9]/.test(trimmed) + if (!isBorder && !emphasised) { + emphasised = true + return `${palette.emphasis}${line}${SUMMARY_ANSI.reset}` + } + return `${palette.base}${line}${SUMMARY_ANSI.reset}` + }).join('\n') + } catch { + // Colour is cosmetic — never let it cost the customer the message. + return body + } + } + async testSessionEvent(data: Omit) { PerformanceTester.start(PERFORMANCE_SDK_EVENTS.DISPATCHER_EVENTS.TEST_SESSION) const workerId = this.getClientWorkerIdFromContext(data.executionContext) diff --git a/packages/browserstack-service/tests/cli/grpcClient.test.ts b/packages/browserstack-service/tests/cli/grpcClient.test.ts index af6cc056..cf91be0d 100644 --- a/packages/browserstack-service/tests/cli/grpcClient.test.ts +++ b/packages/browserstack-service/tests/cli/grpcClient.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' import { GrpcClient } from '../../src/cli/grpcClient.js' +import { BStackLogger } from '../../src/cli/cliLogger.js' vi.mock('../../src/grpc/index.js', () => ({ StopBinSessionRequestConstructor: { create: (fields: Record) => ({ ...fields }) } @@ -11,7 +12,7 @@ vi.mock('../../src/cli/cliUtils.js', () => ({ })) vi.mock('../../src/cli/cliLogger.js', () => ({ - BStackLogger: { debug: vi.fn(), info: vi.fn(), error: vi.fn(), warn: vi.fn() } + BStackLogger: { debug: vi.fn(), info: vi.fn(), error: vi.fn(), warn: vi.fn(), logToFile: vi.fn() } })) vi.mock('../../src/instrumentation/performance/performance-tester.js', () => ({ @@ -89,15 +90,36 @@ describe('GrpcClient.stopBinSession customer-visible summary entries', () => { { entryType: 'version_nudge', severity: 'error', body: 'deprecated' } ] }) await client.stopBinSession() - expect(stderrSpy).toHaveBeenCalledWith('outdated\n') - expect(stderrSpy).toHaveBeenCalledWith('deprecated\n') + expect(stderrSpy).toHaveBeenCalledWith('\x1b[1;33moutdated\x1b[0m\n') + expect(stderrSpy).toHaveBeenCalledWith('\x1b[1;31mdeprecated\x1b[0m\n') expect(stdoutSpy).not.toHaveBeenCalled() }) it('treats the server\'s "warning" spelling as an error stream', async () => { respondWith({ entries: [{ entryType: 'version_nudge', severity: 'warning', body: 'outdated' }] }) await client.stopBinSession() - expect(stderrSpy).toHaveBeenCalledWith('outdated\n') + expect(stderrSpy).toHaveBeenCalledWith('\x1b[1;33moutdated\x1b[0m\n') + }) + + it('tints a warn block yellow and an error block red, line by line', async () => { + const body = '────\n Title\n\n Detail\n────' + respondWith({ entries: [ + { entryType: 'version_nudge', severity: 'warn', body }, + { entryType: 'version_nudge', severity: 'error', body } + ] }) + await client.stopBinSession() + + // Borders take the base tint and the first line carrying text is emphasised. + // Blank lines are left alone, and every tinted line closes its own reset, so a + // truncated write cannot leave the terminal stuck in colour. + expect(stderrSpy).toHaveBeenCalledWith( + '\x1b[33m────\x1b[0m\n\x1b[1;33m Title\x1b[0m\n\n' + + '\x1b[33m Detail\x1b[0m\n\x1b[33m────\x1b[0m\n' + ) + expect(stderrSpy).toHaveBeenCalledWith( + '\x1b[31m────\x1b[0m\n\x1b[1;31m Title\x1b[0m\n\n' + + '\x1b[31m Detail\x1b[0m\n\x1b[31m────\x1b[0m\n' + ) }) it('sends an unknown severity to stdout so CI stderr watchers are not tripped', async () => { @@ -116,6 +138,29 @@ describe('GrpcClient.stopBinSession customer-visible summary entries', () => { expect(stderrSpy).not.toHaveBeenCalled() }) + it('archives via logToFile only, so the block is never printed twice', async () => { + respondWith({ entries: [ + { entryType: 'version_nudge', severity: 'warning', body: 'outdated' }, + { entryType: 'version_nudge', severity: 'error', body: 'deprecated' }, + { entryType: 'version_nudge', severity: 'info', body: 'notice' } + ] }) + await client.stopBinSession() + + // logToFile writes to the log file only. info/warn/error additionally + // call @wdio/logger, which writes to the console — using them here + // would duplicate the block the stream writes above already emitted. + expect(BStackLogger.logToFile).toHaveBeenCalledWith('outdated', 'warn') + expect(BStackLogger.logToFile).toHaveBeenCalledWith('deprecated', 'error') + expect(BStackLogger.logToFile).toHaveBeenCalledWith('notice', 'info') + // The console-writing helpers must never receive a body. (They are still + // used for unrelated lines such as "StopBinSession successful".) + for (const body of ['outdated', 'deprecated', 'notice']) { + expect(BStackLogger.warn).not.toHaveBeenCalledWith(body) + expect(BStackLogger.error).not.toHaveBeenCalledWith(body) + expect(BStackLogger.info).not.toHaveBeenCalledWith(body) + } + }) + it('still returns the response when rendering throws', async () => { stdoutSpy.mockImplementation(() => { throw new Error('stream closed') From 4a94e0d86e81402108beeae241f9cfdf80fb9646 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 07:44:04 +0000 Subject: [PATCH 4/5] chore(changeset): auto-generate from PR template (minor) --- .changeset/pr-200.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/pr-200.md b/.changeset/pr-200.md index b167daeb..43cd3f4e 100644 --- a/.changeset/pr-200.md +++ b/.changeset/pr-200.md @@ -2,4 +2,4 @@ "@wdio/browserstack-service": minor --- -- End-of-build messages from BrowserStack — such as a notice that your SDK version is outdated or has a known issue — are now shown at the end of your test run and written to the SDK log. +- End-of-build messages from BrowserStack — such as a notice that your SDK version is outdated or has a known issue — are now shown at the end of your test run, highlighted in yellow for a warning and red for an error, and written to the SDK log without colour so they stay searchable. From 2b9e5f28e8e942d2f07ab446b62f3a07ea696b45 Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Thu, 24 Sep 2026 17:04:42 +0530 Subject: [PATCH 5/5] fix(browserstack-service): scope the summary-entry catch per entry; cover attachment fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on SDK-7358. 1. renderCustomerVisibleSummary wrapped the whole `for (const entry of entries)` loop in one try/catch, so a stream that rejected entry N aborted the loop: entries N+1.. were neither written nor archived. Only one entry exists today (the version nudge), but the proto is explicitly built for more entry types, so the gap widens as they are added. The catch now sits inside the loop body, and the archived copy is written before the stream write so archival no longer depends on the write succeeding. The outer catch stays — stopBinSession rethrows, so a throw escaping this method would cost the caller its response. This is the same defect and fix as the v8 port (#201). 2. Adds the missing test for the fileName/fileSize/filePath passthrough in logCreatedEvent. Those fields are unrelated to the summary-entry feature and arrived in this PR undocumented and uncovered; this locks their behaviour rather than leaving an untested drive-by. Whether the change belongs in its own PR is still worth a call. Both new tests fail without their respective fix. Package suite on Node 25: 56 files, 1259 tests, 0 failures. tsc --noEmit exit 0, eslint exit 0. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/cli/grpcClient.ts | 53 +++++++------ .../tests/cli/grpcClient.test.ts | 74 ++++++++++++++++++- 2 files changed, 104 insertions(+), 23 deletions(-) diff --git a/packages/browserstack-service/src/cli/grpcClient.ts b/packages/browserstack-service/src/cli/grpcClient.ts index 3ed671ff..2aabc1cd 100644 --- a/packages/browserstack-service/src/cli/grpcClient.ts +++ b/packages/browserstack-service/src/cli/grpcClient.ts @@ -317,29 +317,38 @@ export class GrpcClient { } for (const entry of entries) { - const body = entry?.body || '' - if (!body) { - continue + // Scoped per entry, not around the loop: a stream that rejects one + // entry must not drop the entries after it. The proto allows many + // entry types, so this widens as more are added. + try { + const body = entry?.body || '' + if (!body) { + continue + } + + const severity = (entry?.severity || 'info').toLowerCase() + // warn/warning/error -> stderr, everything else (info AND unknown) -> + // stdout, so a malformed severity cannot false-alarm CI tooling + // watching stderr. + const isErrorStream = severity === 'warn' || severity === 'warning' || severity === 'error' + + // Archived copy is written FIRST — terminal scrollback is lost on + // CI runners that keep only the log directory, so the durable copy + // must not depend on the stream write succeeding. + // + // logToFile, NOT the info/warn/error helpers: those also call + // @wdio/logger, which writes to the console, so the customer + // would see the block twice (once raw below, once prefixed). + this.logger.logToFile(body, severity === 'error' ? 'error' : (isErrorStream ? 'warn' : 'info')) + + // Written directly rather than through the logger, whose per-line + // prefix would break the binary's box-border alignment. Colour is + // applied here only — the archived copy above stays plain. + ;(isErrorStream ? process.stderr : process.stdout) + .write(`${this.colouriseSummaryBody(body, severity)}\n`) + } catch (error: unknown) { + this.logger.debug(`StopBinSession entry forwarding failed: ${util.format(error)}`) } - - const severity = (entry?.severity || 'info').toLowerCase() - // warn/warning/error -> stderr, everything else (info AND unknown) -> - // stdout, so a malformed severity cannot false-alarm CI tooling - // watching stderr. - const isErrorStream = severity === 'warn' || severity === 'warning' || severity === 'error' - // Written directly rather than through the logger, whose per-line - // prefix would break the binary's box-border alignment. Colour is - // applied here only — the archived copy below stays plain. - ;(isErrorStream ? process.stderr : process.stdout) - .write(`${this.colouriseSummaryBody(body, severity)}\n`) - - // Archived copy — terminal scrollback is lost on CI runners that - // keep only the log directory. - // - // logToFile, NOT the info/warn/error helpers: those also call - // @wdio/logger, which writes to the console, so the customer - // would see the block twice (once raw above, once prefixed). - this.logger.logToFile(body, severity === 'error' ? 'error' : (isErrorStream ? 'warn' : 'info')) } } catch (error: unknown) { this.logger.debug(`StopBinSession entries forwarding failed: ${util.format(error)}`) diff --git a/packages/browserstack-service/tests/cli/grpcClient.test.ts b/packages/browserstack-service/tests/cli/grpcClient.test.ts index cf91be0d..94312c1c 100644 --- a/packages/browserstack-service/tests/cli/grpcClient.test.ts +++ b/packages/browserstack-service/tests/cli/grpcClient.test.ts @@ -4,7 +4,11 @@ import { GrpcClient } from '../../src/cli/grpcClient.js' import { BStackLogger } from '../../src/cli/cliLogger.js' vi.mock('../../src/grpc/index.js', () => ({ - StopBinSessionRequestConstructor: { create: (fields: Record) => ({ ...fields }) } + StopBinSessionRequestConstructor: { create: (fields: Record) => ({ ...fields }) }, + ExecutionContextConstructor: { create: (fields: Record) => ({ ...fields }) }, + LogCreatedEventRequestConstructor: { create: (fields: Record) => ({ ...fields }) }, + // eslint-disable-next-line camelcase + LogCreatedEventRequest_LogEntryConstructor: { create: (fields: Record) => ({ ...fields }) } })) vi.mock('../../src/cli/cliUtils.js', () => ({ @@ -161,6 +165,32 @@ describe('GrpcClient.stopBinSession customer-visible summary entries', () => { } }) + it('keeps rendering and archiving the entries after one whose write throws', async () => { + // The catch is scoped per entry, not around the loop: a stream that + // rejects entry one must not silently drop entries two and three. + stderrSpy.mockImplementation((chunk: any) => { + if (String(chunk).includes('first')) { + throw new Error('stream closed') + } + return true + }) + + respondWith({ entries: [ + { entryType: 'version_nudge', severity: 'warn', body: 'first' }, + { entryType: 'version_nudge', severity: 'warn', body: 'second' }, + { entryType: 'version_nudge', severity: 'info', body: 'third' } + ] }) + await client.stopBinSession() + + expect(stderrSpy).toHaveBeenCalledWith('\x1b[1;33msecond\x1b[0m\n') + expect(stdoutSpy).toHaveBeenCalledWith('third\n') + // Archival runs before the stream write, so even the entry whose write + // threw is still kept in the log directory. + expect(BStackLogger.logToFile).toHaveBeenCalledWith('first', 'warn') + expect(BStackLogger.logToFile).toHaveBeenCalledWith('second', 'warn') + expect(BStackLogger.logToFile).toHaveBeenCalledWith('third', 'info') + }) + it('still returns the response when rendering throws', async () => { stdoutSpy.mockImplementation(() => { throw new Error('stream closed') @@ -169,3 +199,45 @@ describe('GrpcClient.stopBinSession customer-visible summary entries', () => { await expect(client.stopBinSession()).resolves.toMatchObject({ done: true }) }) }) + +describe('GrpcClient.logCreatedEvent', () => { + let client: GrpcClient + let logCreatedEvent: ReturnType + + beforeEach(() => { + logCreatedEvent = vi.fn((_req: unknown, cb: (err: unknown, res: unknown) => void) => cb(null, {})) + client = new GrpcClient() + client.binSessionId = 'bin-1' + client.client = { logCreatedEvent } as any + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + it('forwards the attachment fields on a log entry to the binary', async () => { + // Attachment entries carry no message — the binary streams the file from + // filePath, so dropping these three silently breaks attachment upload. + await client.logCreatedEvent({ + platformIndex: 0, + logs: [{ + uuid: 'log-1', + kind: 'TEST_ATTACHMENT', + timestamp: '2026-01-01T00:00:00Z', + level: 'info', + fileName: 'screenshot.png', + fileSize: 2048, + filePath: '/tmp/screenshot.png' + }], + executionContext: { processId: 1, threadId: 2, hash: 'h' } + } as any) + + expect(logCreatedEvent).toHaveBeenCalledTimes(1) + const sent = logCreatedEvent.mock.calls[0][0] as any + expect(sent.logs[0]).toMatchObject({ + fileName: 'screenshot.png', + fileSize: 2048, + filePath: '/tmp/screenshot.png' + }) + }) +})