Skip to content

feat: track CLI errors and crashes via PostHog - #3

Open
0xjgv wants to merge 1 commit into
mainfrom
feat/cli-issue-tracking
Open

feat: track CLI errors and crashes via PostHog#3
0xjgv wants to merge 1 commit into
mainfrom
feat/cli-issue-tracking

Conversation

@0xjgv

@0xjgv 0xjgv commented Jun 23, 2026

Copy link
Copy Markdown
Owner

Extends the existing PostHog telemetry so the CLI tracks command failures, not just invocations — previously a crash, thrown error, or process.exit(1) was invisible.

Adds trackError (via PostHog captureException) with privacy-safe classification: error class, error code, and a path-redacted stack capped to 10 frames — never raw messages, arguments, or absolute paths, honoring the module's existing privacy promise. Programmer-defect types and non-Error throws are tagged crash, deliberately thrown errors user_error.

Wires it in by registering global uncaughtException/unhandledRejection handlers, funneling the ~11 duplicated command catch blocks through one failCommand, and flushing telemetry before process.exit so failure events aren't dropped mid-request on the error path. Covered by new errors unit tests plus trackError cases; full suite (946 tests) green.


Summary by cubic

Adds privacy-safe error and crash tracking for the CLI, so we capture failures (not just runs) and reliably flush telemetry before exit.

  • New Features

    • Added trackError to send sanitized exceptions (class, code, redacted stack capped to 10 frames) to PostHog; never raw messages, args, or absolute paths.
    • Classified failures as user_error vs crash (programmer defects and non-Error throws).
    • Hooked global uncaughtException and unhandledRejection to report crashes.
  • Refactors

    • Replaced duplicated catch blocks with a single failCommand that reports, flushes, and exits with code 1.
    • Set currentCommandPath in preAction to attribute errors to the right command.
    • Added unit tests for error classification, sanitization, and tracking.

Written for commit f69b4eb. Summary will update on new commits.

Review in cubic

Extend telemetry to capture command failures, not just invocations.

- Add trackError using PostHog captureException with privacy-safe
  classification: error class, error code, and a path-redacted stack
  capped to 10 frames. No raw messages, arguments, or absolute paths.
- Classify programmer-defect types and non-Error throws as crashes;
  deliberately thrown errors as user errors.
- Register global uncaughtException/unhandledRejection handlers.
- Funnel the repeated command catch blocks through one failCommand.
- Flush telemetry before process.exit so failure events are not
  dropped mid-request on the error path.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces privacy-safe error classification and sanitization for telemetry, tracking command failures and crashes in PostHog while redacting absolute user paths and raw error messages from stack traces. Feedback on these changes highlights several critical improvement opportunities: first, failCommand should safely extract error messages using an instanceof check to avoid secondary crashes from unsafe casting; second, reportCrash should temporarily disable global exception handlers to prevent infinite loops during reporting and log the stack trace for debugging; and third, process.cwd() and os.homedir() should be cached in redactPaths to improve performance and prevent crashes if process.cwd() throws.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/cli/index.ts
Comment on lines +45 to +51
async function failCommand(error: unknown): Promise<never> {
console.log();
ora().fail(`Error: ${(error as Error).message}`);
await trackError(currentCommandPath, version, error);
await shutdown();
process.exit(1);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In failCommand, casting error directly to Error and accessing .message is unsafe. If error is not an instance of Error (e.g., if it is null, undefined, or a plain string/object), this will throw a TypeError (such as "Cannot read properties of null (reading 'message')"), causing a secondary crash during error handling. We should safely extract the error message using an instanceof check, similar to how it is done in reportCrash.

Suggested change
async function failCommand(error: unknown): Promise<never> {
console.log();
ora().fail(`Error: ${(error as Error).message}`);
await trackError(currentCommandPath, version, error);
await shutdown();
process.exit(1);
}
async function failCommand(error: unknown): Promise<never> {
console.log();
const message = error instanceof Error ? error.message : String(error);
ora().fail(`Error: ${message}`);
await trackError(currentCommandPath, version, error);
await shutdown();
process.exit(1);
}
References
  1. Enforce defensive programming: ensure appropriate null/nil/None checks or other language-idiomatic guards exist before object property accesses.

Comment thread src/cli/index.ts
Comment on lines +56 to +64
function reportCrash(error: unknown): void {
void (async () => {
console.log();
ora().fail(`Error: ${error instanceof Error ? error.message : String(error)}`);
await trackError(currentCommandPath, version, error, 'crash');
await shutdown();
process.exit(1);
})();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In reportCrash, registering global handlers for uncaughtException and unhandledRejection without removing them or using a guard variable inside the handler can lead to infinite loops if any error occurs during the asynchronous reporting phase (e.g., inside trackError or shutdown). Additionally, swallowing the stack trace of uncaught exceptions makes it extremely difficult for users and developers to debug crashes. We should print the stack trace to console.error and remove the event listeners at the beginning of reportCrash to prevent re-entry.

function reportCrash(error: unknown): void {
  // Prevent infinite loops if reporting itself crashes
  process.off('uncaughtException', reportCrash);
  process.off('unhandledRejection', reportCrash);

  void (async () => {
    console.log();
    ora().fail(`Error: ${error instanceof Error ? error.message : String(error)}`);
    if (error instanceof Error && error.stack) {
      console.error(error.stack);
    }
    await trackError(currentCommandPath, version, error, 'crash');
    await shutdown();
    process.exit(1);
  })();
}

Comment thread src/telemetry/errors.ts
Comment on lines +54 to +61
function redactPaths(line: string): string {
let out = line;
const cwd = process.cwd();
const home = os.homedir();
if (cwd) out = out.split(cwd).join('.');
if (home) out = out.split(home).join('~');
return out;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In redactPaths, calling process.cwd() and os.homedir() on every single line of the stack trace is inefficient (as it performs multiple system calls/bindings per frame). Furthermore, process.cwd() can throw an error (e.g., ENOENT if the directory from which the CLI was started has been deleted), which would crash the error sanitization process. We can lazily retrieve and cache these paths on the first call to redactPaths to ensure both safety and high performance.

let cachedCwd: string | undefined;
let cachedHome: string | undefined;

function redactPaths(line: string): string {
  if (cachedCwd === undefined) {
    try {
      cachedCwd = process.cwd();
    } catch {
      cachedCwd = '';
    }
  }
  if (cachedHome === undefined) {
    try {
      cachedHome = os.homedir();
    } catch {
      cachedHome = '';
    }
  }

  let out = line;
  if (cachedCwd) out = out.split(cachedCwd).join('.');
  if (cachedHome) out = out.split(cachedHome).join('~');
  return out;
}

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 5 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/cli/index.ts">

<violation number="1" location="src/cli/index.ts:47">
P1: Unsafe cast of `error` to `Error` — if the thrown value is `null`, `undefined`, or a non-Error object, accessing `.message` will throw a secondary `TypeError` during error handling. Use `error instanceof Error ? error.message : String(error)` as `reportCrash` already does.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/cli/index.ts
*/
async function failCommand(error: unknown): Promise<never> {
console.log();
ora().fail(`Error: ${(error as Error).message}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Unsafe cast of error to Error — if the thrown value is null, undefined, or a non-Error object, accessing .message will throw a secondary TypeError during error handling. Use error instanceof Error ? error.message : String(error) as reportCrash already does.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/index.ts, line 47:

<comment>Unsafe cast of `error` to `Error` — if the thrown value is `null`, `undefined`, or a non-Error object, accessing `.message` will throw a secondary `TypeError` during error handling. Use `error instanceof Error ? error.message : String(error)` as `reportCrash` already does.</comment>

<file context>
@@ -25,12 +25,44 @@ import {
+ */
+async function failCommand(error: unknown): Promise<never> {
+  console.log();
+  ora().fail(`Error: ${(error as Error).message}`);
+  await trackError(currentCommandPath, version, error);
+  await shutdown();
</file context>
Suggested change
ora().fail(`Error: ${(error as Error).message}`);
ora().fail(`Error: ${error instanceof Error ? error.message : String(error)}`);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant