diff --git a/.changeset/pr-201.md b/.changeset/pr-201.md new file mode 100644 index 00000000..43cd3f4e --- /dev/null +++ b/.changeset/pr-201.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, highlighted in yellow for a warning and red for an error, and written to the SDK log without colour so they stay searchable. diff --git a/packages/browserstack-service/src/cli/grpcClient.ts b/packages/browserstack-service/src/cli/grpcClient.ts index bb189e20..7e00ebdc 100644 --- a/packages/browserstack-service/src/cli/grpcClient.ts +++ b/packages/browserstack-service/src/cli/grpcClient.ts @@ -48,6 +48,14 @@ import { BStackLogger } from './cliLogger.js' // Increased from default 4 MB to accommodate large extension payloads 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 * @@ -278,6 +286,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) { @@ -292,6 +301,101 @@ 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) { + // 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)}`) + } + } + } 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/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 6d55f1e1..12f5e22c 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 * as bstackLogger from '../../src/bstackLogger.js' +import { BStackLogger as CliBStackLogger } from '../../src/cli/cliLogger.js' import type { SDKClient } from '../../src/grpc/index.js' import { CLIUtils } from '../../src/cli/cliUtils.js' import type grpc from '@grpc/grpc-js' @@ -154,6 +155,158 @@ describe('GrpcClient', () => { expect(request.exitSignal).toBe('') expect(request.exitReason).toBe('') }) + + describe('customer-visible summary entries', () => { + let stdoutSpy: ReturnType + let stderrSpy: ReturnType + + const respondWith = (response: unknown) => { + grpcClient.client = { + stopBinSession: vi.fn().mockImplementation((req, cb) => cb(null, response)) + } as any + } + + beforeEach(() => { + stdoutSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true) + stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) + }) + + afterEach(() => { + stdoutSpy.mockRestore() + stderrSpy.mockRestore() + }) + + 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 grpcClient.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 grpcClient.stopBinSession() + 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 grpcClient.stopBinSession() + 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 grpcClient.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 () => { + respondWith({ entries: [{ entryType: 'version_nudge', severity: 'bogus', body: 'body' }] }) + await grpcClient.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 grpcClient.stopBinSession() + } + expect(stdoutSpy).not.toHaveBeenCalled() + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it('archives via logToFile only, so the block is never printed twice', async () => { + // 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. + const toFile = vi.spyOn(CliBStackLogger, 'logToFile').mockImplementation(() => {}) + const infoSpy = vi.spyOn(CliBStackLogger, 'info').mockImplementation(() => {}) + const warnSpy = vi.spyOn(CliBStackLogger, 'warn').mockImplementation(() => {}) + const errorSpy = vi.spyOn(CliBStackLogger, 'error').mockImplementation(() => {}) + + respondWith({ entries: [ + { entryType: 'version_nudge', severity: 'warning', body: 'outdated' }, + { entryType: 'version_nudge', severity: 'error', body: 'deprecated' }, + { entryType: 'version_nudge', severity: 'info', body: 'notice' } + ] }) + await grpcClient.stopBinSession() + + expect(toFile).toHaveBeenCalledWith('outdated', 'warn') + expect(toFile).toHaveBeenCalledWith('deprecated', 'error') + expect(toFile).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(warnSpy).not.toHaveBeenCalledWith(body) + expect(errorSpy).not.toHaveBeenCalledWith(body) + expect(infoSpy).not.toHaveBeenCalledWith(body) + } + + toFile.mockRestore() + infoSpy.mockRestore() + warnSpy.mockRestore() + errorSpy.mockRestore() + }) + + 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. + const toFile = vi.spyOn(CliBStackLogger, 'logToFile').mockImplementation(() => {}) + 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 grpcClient.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(toFile).toHaveBeenCalledWith('first', 'warn') + expect(toFile).toHaveBeenCalledWith('second', 'warn') + expect(toFile).toHaveBeenCalledWith('third', 'info') + + toFile.mockRestore() + }) + + 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(grpcClient.stopBinSession()).resolves.toMatchObject({ done: true }) + }) + }) }) describe('connectBinSession', () => {