feat(telemetry): record why a run ended and where a deploy stopped

.changeset/grow-1233-cli-deploy-telemetry.md

@@ -0,0 +1,9 @@
+---
+"clerk": patch
+---
+
+Record `clerk deploy status` on an unfinished deploy as incomplete rather than an error in usage telemetry, and give the ways a `clerk deploy` run can end their own error codes — a skipped step, an interrupted prompt and a wait on Clerk's provisioning were previously indistinguishable. Every `clerk deploy` and `clerk deploy status` event now also records the state the deploy was in when the run ended, so a run that stopped short says where, and which of DNS, SSL, email DNS and OAuth had been verified at that point — recorded only from a read that actually succeeded, so a failed status call is never reported as a failed check. Output and exit codes are unchanged.
+
+`clerk doctor` now names the check that crashed instead of printing an anonymous "Check crashed" line (which `--json` labelled "Unknown check"), says the crash is a bug in the CLI instead of reporting issues with your integration, and reports a crashed check as `doctor_check_crashed` rather than `doctor_failed`, so a bug in the CLI is distinguishable from a real problem with your integration. Its `--json` results carry `crashed: true` on that check. The exit code is unchanged.
+
+`clerk api`, `clerk users create` and `clerk mcp install --json` failures now carry an error code in usage telemetry — each prints the failure itself instead of throwing, which used to leave the event with a bare error. An API response with a Clerk error code records that code; one without is recorded by its HTTP status as `api_rate_limited` (429), `api_not_found` (a 404 on a path the person typed), `cli_endpoint_not_found` (a 404 on a path the CLI built, from its endpoint catalog or a hardcoded route), `api_client_error` (other 4xx) or `api_error` (5xx). `mcp install --json` records the same code human mode already did. Nothing printed changes and exit codes are unchanged.

packages/cli-core/src/cli-program.ts

@@ -51,6 +51,7 @@ import {
   finalizeAndSendTelemetry,
   startCommandTelemetry,
   telemetryResultForError,
+  telemetryResultForSoftExit,
 } from "./lib/telemetry.ts";
 
 /**
@@ -263,12 +264,11 @@ export async function runProgram(
     // the exit for that case; racing it here would report the wrong outcome.
     if (interruptedExitCode() !== null) return;
     // Some commands report failure via process.exitCode instead of throwing —
-    // read it back so telemetry doesn't record them as successes.
+    // read it back so telemetry doesn't record them as successes. What a
+    // nonzero code there *meant* is the command's to say, via
+    // `declareSoftExitOutcome`; absent a declaration this is still an error.
     const softExitCode = Number(process.exitCode ?? EXIT_CODE.SUCCESS);
-    await finalizeAndSendTelemetry({
-      outcome: softExitCode === EXIT_CODE.SUCCESS ? "success" : "error",
-      exitCode: softExitCode,
-    });
+    await finalizeAndSendTelemetry(telemetryResultForSoftExit(softExitCode));
   } catch (error) {
     if (interruptedExitCode() !== null) return;
     // Started before rendering so the message is printed before we block on the

packages/cli-core/src/commands/api/index.test.ts

@@ -10,6 +10,7 @@ import {
   configStubs,
   libPromptsStubs,
   stubFetch,
+  captureTelemetryPayload,
 } from "../../test/lib/stubs.ts";
 
 let mockStoredToken: string | null = null;
@@ -878,4 +879,125 @@ describe("api command", () => {
       Object.defineProperty(process, "platform", { value: originalPlatform, writable: true });
     }
   });
+
+  // The API error is caught here to put the raw body on stdout, so the throw
+  // path never classifies it. What the event carries instead — per status,
+  // since telemetry has neither the status nor the endpoint.
+  describe("what telemetry records as the error code", () => {
+    const clerkBody = (code: string) => JSON.stringify({ errors: [{ code, message: "" }] });
+
+    function recordedFor(
+      endpoint: string,
+      options: Record<string, unknown> = {},
+      caller: { userSuppliedPath?: boolean } = {},
+    ) {
+      return captureTelemetryPayload("api", async () => {
+        const { api } = await import("./index.ts");
+        await api(endpoint, undefined, options, caller);
+      });
+    }
+
+    test("a Clerk error code in the response body", async () => {
+      stubFetch(async () => new Response(clerkBody("resource_not_found"), { status: 404 }));
+      const { payload } = await recordedFor("/users/bad_id");
+      expect(payload.outcome).toBe("error");
+      expect(payload.exit_code).toBe(1);
+      expect(payload.error_code).toBe("resource_not_found");
+    });
+
+    test.each([
+      [429, "api_rate_limited"],
+      [404, "api_not_found"],
+      [400, "api_client_error"],
+      [500, "api_error"],
+    ])("an uncoded %i is api-prefixed by status", async (status, expected) => {
+      stubFetch(async () => new Response("<html>nope</html>", { status }));
+      const { payload } = await recordedFor("/organization_role");
+      expect(payload.outcome).toBe("error");
+      expect(payload.exit_code).toBe(1);
+      expect(payload.error_code).toBe(expected);
+    });
+
+    // `--fapi` only decides whether there is a catalog to suggest searching;
+    // the status is the status.
+    test("--fapi makes no difference to an uncoded 404", async () => {
+      process.env.CLERK_PLATFORM_API_KEY = "ak_test_platform";
+      const pk = `pk_test_${btoa("clerk.example.com$")}`;
+      stubFetch(async (input) => {
+        if (input.toString().includes("/v1/platform/applications/app_1")) {
+          return new Response(
+            JSON.stringify({
+              application_id: "app_1",
+              instances: [
+                { instance_id: "ins_dev", environment_type: "development", publishable_key: pk },
+              ],
+            }),
+            { status: 200 },
+          );
+        }
+        return new Response("404 page not found", { status: 404 });
+      });
+      const { payload } = await recordedFor("/bogus", {
+        fapi: true,
+        app: "app_1",
+        instance: "dev",
+      });
+      expect(payload.error_code).toBe("api_not_found");
+      expect(captured.err).not.toContain("clerk api ls");
+    });
+
+    // The same bare 404 on a path the interactive builder chose from the
+    // CLI's own catalog is the CLI's failure, not a typo, and must not be
+    // filed with the typed ones.
+    test("an uncoded 404 on a catalog endpoint is cli_endpoint_not_found", async () => {
+      stubFetch(async () => new Response("404 page not found", { status: 404 }));
+      const { payload } = await recordedFor("/organization_role", {}, { userSuppliedPath: false });
+      expect(payload.outcome).toBe("error");
+      expect(payload.exit_code).toBe(1);
+      expect(payload.error_code).toBe("cli_endpoint_not_found");
+      // A coded 404 names the resource, whoever wrote the path.
+      stubFetch(async () => new Response(clerkBody("resource_not_found"), { status: 404 }));
+      const coded = await recordedFor("/users/bad_id", {}, { userSuppliedPath: false });
+      expect(coded.payload.error_code).toBe("resource_not_found");
+    });
+
+    // Not an ApiError, so the local catch rethrows and the throw path
+    // classifies it — unchanged, and pinned so the split cannot widen into it.
+    test("a rejected fetch is still a thrown unexpected_error", async () => {
+      stubFetch(async () => {
+        throw new Error("socket hang up");
+      });
+      const { payload, error } = await captureTelemetryPayload("api", () => runApi("/users"), {
+        captureError: true,
+      });
+      expect(error).toBeInstanceOf(Error);
+      expect(payload.outcome).toBe("error");
+      expect(payload.error_code).toBe("unexpected_error");
+    });
+
+    test("a successful request is a success with no code", async () => {
+      const { payload } = await recordedFor("/users");
+      expect(payload.outcome).toBe("success");
+      expect(payload.exit_code).toBe(0);
+      expect(payload.error_code).toBeNull();
+    });
+
+    // The printed output is the reason the error is caught locally; recording
+    // it must not change a byte of it.
+    test("--include on an error still prints the headers and body, with the code on the event", async () => {
+      stubFetch(
+        async () =>
+          new Response('{"error":"bad"}', {
+            status: 400,
+            headers: { "x-request-id": "req_err" },
+          }),
+      );
+      const { payload } = await recordedFor("/users", { include: true });
+      expect(captured.err).toContain("HTTP 400");
+      expect(captured.err).toContain("x-request-id: req_err");
+      expect(captured.out).toContain('"error": "bad"');
+      expect(payload.error_code).toBe("api_client_error");
+      expect(payload.exit_code).toBe(1);
+    });
+  });
 });

packages/cli-core/src/commands/api/index.ts

@@ -7,6 +7,7 @@ import { bapiRequest } from "../../lib/bapi.ts";
 import { fapiRequest } from "../../lib/fapi.ts";
 import { resolveFapiHost } from "./fapi.ts";
 import { ApiError, ERROR_CODE, throwUsageError, throwUserAbort } from "../../lib/errors.ts";
+import { declareSoftExitError } from "../../lib/telemetry.ts";
 import { validateJsonBody } from "../../lib/json-body.ts";
 import { isHuman } from "../../mode.ts";
 import { confirm } from "../../lib/prompts.ts";
@@ -63,10 +64,16 @@ async function resolveApiTarget(
   return { baseUrl, runRequest: async (req) => bapiRequest({ ...req, secretKey, baseUrl }) };
 }
 
+/**
+ * `userSuppliedPath` says who wrote the request path, for telemetry's 404
+ * classification. The default is the command line, so the person; a caller
+ * that builds its own path passes false.
+ */
 export async function api(
   endpoint: string | undefined,
   filter: string | undefined,
   options: ApiOptions,
+  { userSuppliedPath = true }: { userSuppliedPath?: boolean } = {},
 ): Promise<void> {
   const nested = isInsideGutter();
   if (!nested) intro("Calling Clerk API");
@@ -154,6 +161,8 @@ export async function api(
           const scope = options.platform ? " --platform" : "";
           log.info(`If the endpoint path was a guess, search with: clerk api ls <keyword>${scope}`);
         }
+        // Handled here, so telemetry never sees the throw it would classify.
+        declareSoftExitError(error, { userSuppliedPath });
         process.exitCode = 1;
         closeStatus = "failed";
         return;
@@ -310,5 +319,7 @@ export function registerApi(program: Program): void {
         description: "GET the public FAPI environment payload",
       },
     ])
-    .action(api);
+    // Wrapped because Commander passes the Command itself as a fourth argument,
+    // which must not land in `api`'s caller options.
+    .action(async (endpoint, filter, options) => api(endpoint, filter, options));
 }

packages/cli-core/src/commands/api/interactive.test.ts

@@ -2,7 +2,12 @@ import { test, expect, describe, beforeEach, afterEach, spyOn, mock } from "bun:
 import { mkdtemp, rm } from "node:fs/promises";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
-import { useCaptureLog, listageStubs, stubFetch } from "../../test/lib/stubs.ts";
+import {
+  captureTelemetryPayload,
+  useCaptureLog,
+  listageStubs,
+  stubFetch,
+} from "../../test/lib/stubs.ts";
 
 let _mode = "human";
 mock.module("../../mode.ts", () => ({
@@ -15,6 +20,7 @@ mock.module("../../mode.ts", () => ({
 }));
 
 const { parseSpec, _setCacheDir } = (await import("./catalog.ts")) as any;
+const { _setConfigDir } = await import("../../lib/config.ts");
 const { setMode } = (await import("../../mode.ts")) as any;
 
 const MINIMAL_SPEC = `
@@ -81,6 +87,7 @@ describe("apiInteractive", () => {
   beforeEach(async () => {
     tempDir = await mkdtemp(join(tmpdir(), "clerk-interactive-test-"));
     _setCacheDir(tempDir);
+    _setConfigDir(tempDir);
 
     // Pre-populate fresh cache
     const cached = parseSpec(MINIMAL_SPEC);
@@ -115,6 +122,7 @@ describe("apiInteractive", () => {
 
   afterEach(async () => {
     _setCacheDir(undefined);
+    _setConfigDir(undefined);
     process.env = { ...originalEnv };
     globalThis.fetch = originalFetch;
     Object.defineProperty(process.stdin, "isTTY", {
@@ -164,6 +172,28 @@ describe("apiInteractive", () => {
     expect(fetchCalls[0]!.method).toBe("GET");
   });
 
+  // The builder hands the catalog's path to the real handler; what telemetry
+  // records for a bare 404 on it is the seam this pins, end to end.
+  test("a bare 404 on a catalog endpoint is recorded as the CLI's failure", async () => {
+    setMode("human");
+    selectResponses.push("Users");
+    selectResponses.push({
+      method: "GET",
+      path: "/users",
+      summary: "List all users",
+      tag: "Users",
+      operationId: "GetUserList",
+      pathParams: [],
+      hasRequestBody: false,
+    });
+    confirmResponses.push(true);
+    stubFetch(async () => new Response("404 page not found", { status: 404 }));
+    const { payload } = await captureTelemetryPayload("api", () => runApiInteractive({}));
+    expect(payload.outcome).toBe("error");
+    expect(payload.exit_code).toBe(1);
+    expect(payload.error_code).toBe("cli_endpoint_not_found");
+  });
+
   test("prompts for path parameters", async () => {
     setMode("human");
     selectResponses.push("Users");

packages/cli-core/src/commands/api/interactive.ts

@@ -100,10 +100,18 @@ export async function apiInteractive(options: ApiOptions): Promise<void> {
 
   // 7. Delegate to the main api handler
   const { api } = await import("./index.ts");
-  await api(resolvedPath, undefined, {
-    ...options,
-    method: endpoint.method,
-    data: body,
-    yes: true, // skip double-confirmation
-  });
+  await api(
+    resolvedPath,
+    undefined,
+    {
+      ...options,
+      method: endpoint.method,
+      data: body,
+      yes: true, // skip double-confirmation
+    },
+    // The endpoint is the catalog's, so a bare 404 is recorded as the CLI's.
+    // A typed parameter goes into the route as typed, so a malformed value
+    // (a slash, a dot segment) can be misfiled the same way; accepted as rare.
+    { userSuppliedPath: false },
+  );
 }