From 736180216853fbb38562dc859ebe7104efb3de1e Mon Sep 17 00:00:00 2001 From: Oscar Hong Date: Sat, 18 Jul 2026 09:05:07 -0700 Subject: [PATCH 1/2] fix(cli): swallow posthog shutdown-timeout rejection @posthog/core rejects _shutdown when flush exceeds the timeout; the unhandled rejection was re-captured by exception autocapture (daily error-tracking noise) and skipped the final process.exit. Co-Authored-By: Claude Fable 5 --- docs/CHANGELOG.md | 2 ++ packages/cli/__tests__/telemetry.test.ts | 13 +++++++++++++ packages/cli/src/lib/telemetry.ts | 5 ++++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 641b6eaa..5191dc69 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,6 +4,8 @@ ### Fixed +- **CLI telemetry shutdown no longer leaks a "Timeout while shutting down PostHog" exception.** `@posthog/core` rejects `_shutdown` when the flush exceeds the timeout; the rejection could win the race in `shutdownTelemetryWithTimeout`, becoming an unhandled rejection that exception autocapture re-reported to PostHog daily and that skipped the CLI's final `process.exit`. The rejection is now swallowed — a slow telemetry flush is expected and silent. + - **The CLI now waits for and renders the scorecard after a successful sync.** A healthy dashboard response taking longer than 1.5 seconds is no longer discarded with a suggestion to run `straude status` separately. ### Added diff --git a/packages/cli/__tests__/telemetry.test.ts b/packages/cli/__tests__/telemetry.test.ts index 5dbba0e9..c9a35952 100644 --- a/packages/cli/__tests__/telemetry.test.ts +++ b/packages/cli/__tests__/telemetry.test.ts @@ -4,6 +4,7 @@ vi.mock("../src/lib/posthog.js", () => ({ posthog: { capture: vi.fn(), captureException: vi.fn(), + _shutdown: vi.fn(() => Promise.resolve()), }, })); @@ -17,6 +18,7 @@ import { isPushInvocation, reportCliException, reportUsagePushFailed, + shutdownTelemetryWithTimeout, } from "../src/lib/telemetry.js"; const mockCapture = vi.mocked(posthog.capture); @@ -73,4 +75,15 @@ describe("telemetry", () => { { command: "login" }, ); }); + + it("swallows the posthog shutdown-timeout rejection instead of propagating it", async () => { + // @posthog/core rejects _shutdown with this string when flush exceeds the + // timeout. If it propagates, it becomes an unhandled rejection that + // exception autocapture re-reports and that skips the final process.exit. + vi.mocked(posthog._shutdown).mockRejectedValueOnce( + "Timeout while shutting down PostHog. Some events may not have been sent.", + ); + + await expect(shutdownTelemetryWithTimeout(10)).resolves.toBeTypeOf("number"); + }); }); diff --git a/packages/cli/src/lib/telemetry.ts b/packages/cli/src/lib/telemetry.ts index b6806fa1..3acc4a3b 100644 --- a/packages/cli/src/lib/telemetry.ts +++ b/packages/cli/src/lib/telemetry.ts @@ -57,7 +57,10 @@ export async function shutdownTelemetryWithTimeout( let timer: NodeJS.Timeout | undefined; try { await Promise.race([ - posthog._shutdown(timeoutMs), + // @posthog/core rejects _shutdown when flush exceeds the timeout. A + // slow/failed telemetry flush must never surface: unhandled, it gets + // re-captured by exception autocapture and skips the final process.exit. + posthog._shutdown(timeoutMs).catch(() => {}), new Promise((resolve) => { timer = setTimeout(resolve, timeoutMs); timer.unref?.(); From 9b2533fee43218f977d64d5460733415dc0f23e2 Mon Sep 17 00:00:00 2001 From: Oscar Hong Date: Sat, 8 Aug 2026 15:48:39 -0700 Subject: [PATCH 2/2] fix(cli): guarantee the exit path survives a telemetry shutdown failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR's claim is that the CLI always reaches process.exit. The .catch on _shutdown removes the one rejection source we know about, but the call site still made the exit conditional on shutdownTelemetryWithTimeout resolving — any future throw inside it skips process.exit and hangs the CLI on the event loop. Make the exit unconditional. Also pin the late-rejection semantics: Promise.race subscribes to both inputs, so a rejection arriving after the local timer wins is already handled. The test documents that so a refactor away from Promise.race can't quietly reintroduce an unhandled rejection. Co-Authored-By: Claude Opus 5 --- packages/cli/__tests__/telemetry.test.ts | 30 ++++++++++++++++++++++++ packages/cli/src/index.ts | 9 ++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/cli/__tests__/telemetry.test.ts b/packages/cli/__tests__/telemetry.test.ts index c9a35952..4995e6f2 100644 --- a/packages/cli/__tests__/telemetry.test.ts +++ b/packages/cli/__tests__/telemetry.test.ts @@ -86,4 +86,34 @@ describe("telemetry", () => { await expect(shutdownTelemetryWithTimeout(10)).resolves.toBeTypeOf("number"); }); + + it("stays quiet when the local timer wins and the rejection lands later", async () => { + // posthog's own timeout and ours are both 150 ms, so the rejection can land + // after the local timer already resolved. Promise.race subscribes to both + // inputs, so that late rejection is already handled — this pins that down + // so a refactor away from Promise.race can't silently reintroduce a + // late unhandled rejection. + const unhandled = vi.fn(); + process.on("unhandledRejection", unhandled); + try { + vi.mocked(posthog._shutdown).mockReturnValueOnce( + new Promise((_resolve, reject) => + setTimeout( + () => + reject( + "Timeout while shutting down PostHog. Some events may not have been sent.", + ), + 20, + ), + ), + ); + + await expect(shutdownTelemetryWithTimeout(1)).resolves.toBeTypeOf("number"); + await new Promise((resolve) => setTimeout(resolve, 60)); + + expect(unhandled).not.toHaveBeenCalled(); + } finally { + process.off("unhandledRejection", unhandled); + } + }); }); diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index bb8edbbc..22cb3b18 100755 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -246,4 +246,11 @@ main() } console.error(`Error: ${errorMessage(err)}`); }) - .finally(() => shutdownTelemetryWithTimeout().then(() => process.exit(exitCode))); + .finally(() => + // Telemetry shutdown must never decide whether the process exits: if it + // rejects, `.then` is skipped and the CLI hangs on the event loop with an + // unhandled rejection instead of returning its exit code. + shutdownTelemetryWithTimeout() + .catch(() => {}) + .then(() => process.exit(exitCode)), + );