From 2510870133b792344add63f0ebaea70ef11c6297 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 11 Apr 2026 21:25:26 +0900 Subject: [PATCH] fix(run): isolate CLI telemetry failures Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/cli/run/runner.telemetry.test.ts | 91 +++++++++++++++++++++ src/cli/run/runner.ts | 114 +++++++++++++++++---------- 2 files changed, 162 insertions(+), 43 deletions(-) create mode 100644 src/cli/run/runner.telemetry.test.ts diff --git a/src/cli/run/runner.telemetry.test.ts b/src/cli/run/runner.telemetry.test.ts new file mode 100644 index 000000000..0117f74a8 --- /dev/null +++ b/src/cli/run/runner.telemetry.test.ts @@ -0,0 +1,91 @@ +import { afterEach, describe, expect, it, mock } from "bun:test" + +describe("run telemetry isolation", () => { + afterEach(() => { + mock.restore() + }) + + it("does not crash CLI run when telemetry throws", async () => { + // given + mock.module("../../plugin-config", () => ({ + loadPluginConfig: mock(() => ({})), + })) + mock.module("./agent-resolver", () => ({ + resolveRunAgent: mock(() => "Sisyphus - Ultraworker"), + })) + mock.module("./events", () => ({ + createEventState: mock(() => ({ + messageCount: 0, + lastPartText: "Run completed", + agentColorsByName: {}, + })), + processEvents: mock(async () => {}), + serializeError: (error: unknown) => (error instanceof Error ? error.message : String(error)), + })) + mock.module("./server-connection", () => ({ + createServerConnection: mock(async () => ({ + client: { + event: { + subscribe: mock(async () => ({ stream: {} })), + }, + session: { + promptAsync: mock(async () => undefined), + }, + }, + cleanup: mock(() => {}), + })), + })) + mock.module("./session-resolver", () => ({ + resolveSession: mock(async () => "ses_test"), + })) + mock.module("./json-output", () => ({ + createJsonOutputManager: mock(() => ({ + redirectToStderr: mock(() => {}), + restore: mock(() => {}), + emitResult: mock(() => {}), + })), + })) + mock.module("./on-complete-hook", () => ({ + executeOnCompleteHook: mock(async () => {}), + })) + mock.module("./model-resolver", () => ({ + resolveRunModel: mock(() => null), + })) + mock.module("./poll-for-completion", () => ({ + pollForCompletion: mock(async () => 0), + })) + mock.module("./agent-profile-colors", () => ({ + loadAgentProfileColors: mock(async () => ({})), + })) + mock.module("./stdin-suppression", () => ({ + suppressRunInput: mock(() => mock(() => {})), + })) + mock.module("./timestamp-output", () => ({ + createTimestampedStdoutController: mock(() => ({ + enable: mock(() => {}), + restore: mock(() => {}), + })), + })) + mock.module("../../shared/posthog", () => ({ + createCliPostHog: mock(() => ({ + trackActive: () => { + throw new Error("telemetry failed") + }, + capture: mock(() => {}), + captureException: mock(() => {}), + shutdown: mock(async () => { + throw new Error("shutdown failed") + }), + })), + getPostHogDistinctId: mock(() => "run-distinct-id"), + })) + + const { run } = await import(`./runner?telemetry=${Date.now()}-${Math.random()}`) + + // when + const result = await run({ message: "test" }) + + // then + expect(result).toBe(0) + }) +}) diff --git a/src/cli/run/runner.ts b/src/cli/run/runner.ts index 5f407d7b2..d6b52a299 100644 --- a/src/cli/run/runner.ts +++ b/src/cli/run/runner.ts @@ -53,17 +53,25 @@ export async function run(options: RunOptions): Promise { const posthog = createCliPostHog() const distinctId = getPostHogDistinctId() - posthog.trackActive(distinctId, "run_started") - posthog.capture({ - distinctId, - event: "run_started", - properties: { - command: "run", - agent: resolvedAgent, - has_model: !!options.model, - has_session_id: !!options.sessionId, - }, - }) + try { + posthog.trackActive(distinctId, "run_started") + } catch { + // telemetry failure is non-fatal, silently ignore + } + try { + posthog.capture({ + distinctId, + event: "run_started", + properties: { + command: "run", + agent: resolvedAgent, + has_model: !!options.model, + has_session_id: !!options.sessionId, + }, + }) + } catch { + // telemetry failure is non-fatal, silently ignore + } try { const resolvedModel = resolveRunModel(options.model) @@ -157,27 +165,35 @@ export async function run(options: RunOptions): Promise { } if (exitCode === 0) { - posthog.capture({ - distinctId, - event: "run_completed", - properties: { - command: "run", - agent: resolvedAgent, - duration_ms: durationMs, - message_count: eventState.messageCount, - }, - }) + try { + posthog.capture({ + distinctId, + event: "run_completed", + properties: { + command: "run", + agent: resolvedAgent, + duration_ms: durationMs, + message_count: eventState.messageCount, + }, + }) + } catch { + // telemetry failure is non-fatal, silently ignore + } } else if (exitCode === 1) { - posthog.capture({ - distinctId, - event: "run_failed", - properties: { - command: "run", - agent: resolvedAgent, - exit_code: exitCode, - duration_ms: durationMs, - }, - }) + try { + posthog.capture({ + distinctId, + event: "run_failed", + properties: { + command: "run", + agent: resolvedAgent, + exit_code: exitCode, + duration_ms: durationMs, + }, + }) + } catch { + // telemetry failure is non-fatal, silently ignore + } } return exitCode @@ -194,21 +210,33 @@ export async function run(options: RunOptions): Promise { if (err instanceof Error && err.name === "AbortError") { return 130 } - posthog.captureException(err, distinctId) - posthog.capture({ - distinctId, - event: "run_failed", - properties: { - command: "run", - agent: resolvedAgent, - error: serializeError(err), - duration_ms: Date.now() - startTime, - }, - }) + try { + posthog.captureException(err, distinctId) + } catch { + // telemetry failure is non-fatal, silently ignore + } + try { + posthog.capture({ + distinctId, + event: "run_failed", + properties: { + command: "run", + agent: resolvedAgent, + error: serializeError(err), + duration_ms: Date.now() - startTime, + }, + }) + } catch { + // telemetry failure is non-fatal, silently ignore + } console.error(pc.red(`Error: ${serializeError(err)}`)) return 1 } finally { - await posthog.shutdown() + try { + await posthog.shutdown() + } catch { + // telemetry failure is non-fatal, silently ignore + } timestampOutput?.restore() } }