From 96d624f77579fbe8dc02972a11b493393e20f550 Mon Sep 17 00:00:00 2001 From: Timo Huovinen Date: Sat, 18 Jul 2026 23:14:49 +0300 Subject: [PATCH] fix(cli): add safe diagnostics for empty MQTT runner errors --- .../__tests__/abstract-check-runner.spec.ts | 78 +++++++++++++++++++ .../cli/src/services/abstract-check-runner.ts | 27 ++++++- 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/services/__tests__/abstract-check-runner.spec.ts b/packages/cli/src/services/__tests__/abstract-check-runner.spec.ts index f36e7bbde..163d6394d 100644 --- a/packages/cli/src/services/__tests__/abstract-check-runner.spec.ts +++ b/packages/cli/src/services/__tests__/abstract-check-runner.spec.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import AbstractCheckRunner, { Events, SequenceId } from '../abstract-check-runner.js' +import { NoMatchingChecksError } from '../../rest/test-sessions.js' // --------------------------------------------------------------------------- // Module mocks — must be hoisted before any imports that pull these in @@ -239,4 +240,81 @@ describe('AbstractCheckRunner — SocketClient lifecycle', () => { expect(mockClient.endAsync).toHaveBeenCalledTimes(1) }) + + it('reports safe diagnostics when MQTT connection fails without a message', async () => { + const connectionError = Object.assign(new Error(''), { code: 'ECONNRESET' }) + vi.mocked(SocketClient.connect).mockRejectedValueOnce(connectionError) + const runner = makeRunner() + const errors: Error[] = [] + runner.on(Events.ERROR, error => errors.push(error)) + + await runner.run() + + expect(errors).toHaveLength(1) + expect(errors[0].message).toBe('MQTT connection failed: no error message was provided (code: ECONNRESET)') + }) + + it('omits unsafe MQTT connection error codes from fallback diagnostics', async () => { + const connectionError = Object.assign(new Error(''), { code: 'api-key=secret-value' }) + vi.mocked(SocketClient.connect).mockRejectedValueOnce(connectionError) + const runner = makeRunner() + const errors: Error[] = [] + runner.on(Events.ERROR, error => errors.push(error)) + + await runner.run() + + expect(errors).toHaveLength(1) + expect(errors[0].message).toBe('MQTT connection failed: no error message was provided') + expect(errors[0].message).not.toContain('secret-value') + }) + + it('preserves useful MQTT connection error messages', async () => { + const connectionError = new Error('Connection refused') + vi.mocked(SocketClient.connect).mockRejectedValueOnce(connectionError) + const runner = makeRunner() + const errors: Error[] = [] + runner.on(Events.ERROR, error => errors.push(error)) + + await runner.run() + + expect(errors).toHaveLength(1) + expect(errors[0].message).toBe('Connection refused') + }) + + it('reports safe diagnostics when MQTT subscription fails without a message', async () => { + const subscriptionError = Object.assign(new Error(''), { code: 135 }) + const mockClient = { + on: vi.fn(), + subscribeAsync: vi.fn().mockRejectedValue(subscriptionError), + endAsync: vi.fn().mockResolvedValue(undefined), + } + vi.mocked(SocketClient.connect).mockResolvedValueOnce(mockClient as any) + const runner = makeRunner() + const errors: Error[] = [] + runner.on(Events.ERROR, error => errors.push(error)) + + await runner.run() + + expect(errors).toHaveLength(1) + expect(errors[0].message).toBe('MQTT subscription failed: no error message was provided (code: 135)') + }) + + it('preserves typed scheduling errors', async () => { + const mockClient = { + on: vi.fn(), + subscribeAsync: vi.fn().mockResolvedValue(undefined), + endAsync: vi.fn().mockResolvedValue(undefined), + } + vi.mocked(SocketClient.connect).mockResolvedValueOnce(mockClient as any) + const schedulingError = new NoMatchingChecksError() + const runner = makeRunner() + runner.scheduleChecks = vi.fn().mockRejectedValue(schedulingError) + const errors: Error[] = [] + runner.on(Events.ERROR, error => errors.push(error)) + + await runner.run() + + expect(errors).toHaveLength(1) + expect(errors[0]).toBeInstanceOf(NoMatchingChecksError) + }) }) diff --git a/packages/cli/src/services/abstract-check-runner.ts b/packages/cli/src/services/abstract-check-runner.ts index 30a777581..e32cead62 100644 --- a/packages/cli/src/services/abstract-check-runner.ts +++ b/packages/cli/src/services/abstract-check-runner.ts @@ -44,6 +44,21 @@ export const DEFAULT_PLAYWRIGHT_CHECK_RUN_TIMEOUT_SECONDS = 1200 const DEFAULT_SCHEDULING_DELAY_EXCEEDED_MS = 20000 +function ensureErrorMessage (err: unknown, fallback: string): Error { + if (!(err instanceof Error)) { + return new Error(fallback) + } + if (err.message.trim()) { + return err + } + + const code = (err as Error & { code?: unknown }).code + const isSafeNumericCode = typeof code === 'number' && Number.isFinite(code) + const isSafeStringCode = typeof code === 'string' && /^[A-Z][A-Z0-9_]{0,63}$/.test(code) + err.message = isSafeNumericCode || isSafeStringCode ? `${fallback} (code: ${code})` : fallback + return err +} + export default abstract class AbstractCheckRunner extends EventEmitter { checks: Map testSessionId?: string @@ -96,10 +111,18 @@ export default abstract class AbstractCheckRunner extends EventEmitter { return } - socketClient = await SocketClient.connect() + try { + socketClient = await SocketClient.connect() + } catch (err) { + throw ensureErrorMessage(err, 'MQTT connection failed: no error message was provided') + } // Configure the socket listener and allChecksFinished listener before starting checks to avoid race conditions - await this.configureResultListener(checkRunSuiteId, socketClient) + try { + await this.configureResultListener(checkRunSuiteId, socketClient) + } catch (err) { + throw ensureErrorMessage(err, 'MQTT subscription failed: no error message was provided') + } const { testSessionId, checks } = await this.scheduleChecks(checkRunSuiteId) this.testSessionId = testSessionId