feat(cr): cr

billxinli · billxinli · commit 36ee7fa743d4 · 2025-12-15T18:32:04.000Z
diff --git a/src/cli.mts b/src/cli.mts
@@ -20,6 +20,7 @@ import { meowWithSubcommands } from './utils/meow-with-subcommands.mts'
 import { serializeResultJson } from './utils/serialize-result-json.mts'
 import {
   finalizeTelemetry,
+  setupTelemetryExitHandlers,
   trackCliComplete,
   trackCliError,
   trackCliStart,
@@ -28,9 +29,15 @@ import { socketPackageLink } from './utils/terminal-link.mts'
 
 const __filename = fileURLToPath(import.meta.url)
 
+// Capture CLI start time at module level for global error handlers.
+const cliStartTime = Date.now()
+
+// Set up telemetry exit handlers early to catch all exit scenarios.
+setupTelemetryExitHandlers()
+
 void (async () => {
   // Track CLI start for telemetry.
-  const cliStartTime = await trackCliStart(process.argv)
+  await trackCliStart(process.argv)
 
   const registryUrl = lookupRegistryUrl()
   await updateNotifier({
@@ -119,17 +126,13 @@ void (async () => {
     }
 
     await captureException(e)
-  } finally {
-    // Finalize telemetry to ensure all events are sent.
-    // This runs on both success and error paths.
-    await finalizeTelemetry()
   }
 })().catch(async err => {
   // Fatal error in main async function.
   console.error('Fatal error:', err)
 
   // Track CLI error for fatal exceptions.
-  await trackCliError(process.argv, Date.now(), err, 1)
+  await trackCliError(process.argv, cliStartTime, err, 1)
 
   // Finalize telemetry before fatal exit.
   await finalizeTelemetry()
@@ -143,7 +146,7 @@ process.on('uncaughtException', async err => {
   console.error('Uncaught exception:', err)
 
   // Track CLI error for uncaught exception.
-  await trackCliError(process.argv, Date.now(), err, 1)
+  await trackCliError(process.argv, cliStartTime, err, 1)
 
   // Finalize telemetry before exit.
   await finalizeTelemetry()
@@ -158,7 +161,7 @@ process.on('unhandledRejection', async (reason, promise) => {
 
   // Track CLI error for unhandled rejection.
   const error = reason instanceof Error ? reason : new Error(String(reason))
-  await trackCliError(process.argv, Date.now(), error, 1)
+  await trackCliError(process.argv, cliStartTime, error, 1)
 
   // Finalize telemetry before exit.
   await finalizeTelemetry()
diff --git a/src/commands/npm/cmd-npm.mts b/src/commands/npm/cmd-npm.mts
@@ -102,16 +102,17 @@ async function run(
   // See https://nodejs.org/api/child_process.html#event-exit.
   spawnPromise.process.on(
     'exit',
-    async (code: number | null, signalName: NodeJS.Signals | null) => {
-      // Track subprocess exit and flush telemetry.
-      await trackSubprocessExit(NPM, subprocessStartTime, code)
-
-      if (signalName) {
-        process.kill(process.pid, signalName)
-      } else if (typeof code === 'number') {
-        // eslint-disable-next-line n/no-process-exit
-        process.exit(code)
-      }
+    (code: number | null, signalName: NodeJS.Signals | null) => {
+      // Track subprocess exit and flush telemetry before exiting.
+      // Use .then() to ensure telemetry completes before process.exit().
+      void trackSubprocessExit(NPM, subprocessStartTime, code).then(() => {
+        if (signalName) {
+          process.kill(process.pid, signalName)
+        } else if (typeof code === 'number') {
+          // eslint-disable-next-line n/no-process-exit
+          process.exit(code)
+        }
+      })
     },
   )
 
diff --git a/src/commands/npx/cmd-npx.mts b/src/commands/npx/cmd-npx.mts
@@ -87,16 +87,17 @@ async function run(
   // See https://nodejs.org/api/child_process.html#event-exit.
   spawnPromise.process.on(
     'exit',
-    async (code: number | null, signalName: NodeJS.Signals | null) => {
-      // Track subprocess exit and flush telemetry.
-      await trackSubprocessExit(NPX, subprocessStartTime, code)
-
-      if (signalName) {
-        process.kill(process.pid, signalName)
-      } else if (typeof code === 'number') {
-        // eslint-disable-next-line n/no-process-exit
-        process.exit(code)
-      }
+    (code: number | null, signalName: NodeJS.Signals | null) => {
+      // Track subprocess exit and flush telemetry before exiting.
+      // Use .then() to ensure telemetry completes before process.exit().
+      void trackSubprocessExit(NPX, subprocessStartTime, code).then(() => {
+        if (signalName) {
+          process.kill(process.pid, signalName)
+        } else if (typeof code === 'number') {
+          // eslint-disable-next-line n/no-process-exit
+          process.exit(code)
+        }
+      })
     },
   )
 
diff --git a/src/commands/pnpm/cmd-pnpm.mts b/src/commands/pnpm/cmd-pnpm.mts
@@ -96,16 +96,17 @@ async function run(
   // See https://nodejs.org/api/child_process.html#event-exit.
   spawnPromise.process.on(
     'exit',
-    async (code: number | null, signalName: NodeJS.Signals | null) => {
-      // Track subprocess exit and flush telemetry.
-      await trackSubprocessExit(PNPM, subprocessStartTime, code)
-
-      if (signalName) {
-        process.kill(process.pid, signalName)
-      } else if (typeof code === 'number') {
-        // eslint-disable-next-line n/no-process-exit
-        process.exit(code)
-      }
+    (code: number | null, signalName: NodeJS.Signals | null) => {
+      // Track subprocess exit and flush telemetry before exiting.
+      // Use .then() to ensure telemetry completes before process.exit().
+      void trackSubprocessExit(PNPM, subprocessStartTime, code).then(() => {
+        if (signalName) {
+          process.kill(process.pid, signalName)
+        } else if (typeof code === 'number') {
+          // eslint-disable-next-line n/no-process-exit
+          process.exit(code)
+        }
+      })
     },
   )
 
diff --git a/src/commands/yarn/cmd-yarn.mts b/src/commands/yarn/cmd-yarn.mts
@@ -96,16 +96,17 @@ async function run(
   // See https://nodejs.org/api/child_process.html#event-exit.
   spawnPromise.process.on(
     'exit',
-    async (code: number | null, signalName: NodeJS.Signals | null) => {
-      // Track subprocess exit and flush telemetry.
-      await trackSubprocessExit(YARN, subprocessStartTime, code)
-
-      if (signalName) {
-        process.kill(process.pid, signalName)
-      } else if (typeof code === 'number') {
-        // eslint-disable-next-line n/no-process-exit
-        process.exit(code)
-      }
+    (code: number | null, signalName: NodeJS.Signals | null) => {
+      // Track subprocess exit and flush telemetry before exiting.
+      // Use .then() to ensure telemetry completes before process.exit().
+      void trackSubprocessExit(YARN, subprocessStartTime, code).then(() => {
+        if (signalName) {
+          process.kill(process.pid, signalName)
+        } else if (typeof code === 'number') {
+          // eslint-disable-next-line n/no-process-exit
+          process.exit(code)
+        }
+      })
     },
   )
 
diff --git a/src/utils/telemetry/integration.mts b/src/utils/telemetry/integration.mts
@@ -5,6 +5,9 @@
  * Usage:
  * ```typescript
  * import {
+ *   setupTelemetryExitHandlers,
+ *   finalizeTelemetry,
+ *   finalizeTelemetrySync,
  *   trackCliStart,
  *   trackCliEvent,
  *   trackCliComplete,
@@ -14,6 +17,9 @@
  *   trackSubprocessError
  * } from './utils/telemetry/integration.mts'
  *
+ * // Set up exit handlers once during CLI initialization.
+ * setupTelemetryExitHandlers()
+ *
  * // Track main CLI execution.
  * const startTime = await trackCliStart(process.argv)
  * await trackCliComplete(process.argv, startTime, 0)
@@ -27,12 +33,17 @@
  *
  * // On subprocess error.
  * await trackSubprocessError('npm', subStart, error, 1)
+ *
+ * // Manual finalization (usually not needed if exit handlers are set up).
+ * await finalizeTelemetry() // Async version.
+ * finalizeTelemetrySync()    // Sync version (best-effort).
  * ```
  */
 import { homedir } from 'node:os'
 import process from 'node:process'
 
 import { debugFn } from '@socketsecurity/registry/lib/debug'
+import { escapeRegExp } from '@socketsecurity/registry/lib/regexps'
 
 import { TelemetryService } from './service.mts'
 import constants, { CONFIG_KEY_DEFAULT_ORG } from '../../constants.mts'
@@ -48,8 +59,9 @@ const debug = (message: string): void => {
 }
 
 /**
- * Finalize telemetry and clean up resources.
+ * Finalize telemetry and clean up resources (async version).
  * This should be called before process.exit to ensure telemetry is sent and resources are cleaned up.
+ * Use this in async contexts like beforeExit handlers.
  *
  * @returns Promise that resolves when finalization completes.
  */
@@ -61,6 +73,76 @@ export async function finalizeTelemetry(): Promise<void> {
   }
 }
 
+/**
+ * Finalize telemetry synchronously (best-effort).
+ * This triggers a flush without awaiting it.
+ * Use this in synchronous contexts like signal handlers where async operations are not possible.
+ *
+ * Note: This is best-effort only. Events may be lost if the process exits before flush completes.
+ * Prefer finalizeTelemetry() (async version) when possible.
+ */
+export function finalizeTelemetrySync(): void {
+  const instance = TelemetryService.getCurrentInstance()
+  if (instance) {
+    debug('Triggering sync flush (best-effort)')
+    void instance.flush()
+  }
+}
+
+/**
+ * Set up exit handlers for telemetry finalization.
+ * This registers handlers for both normal exits (beforeExit) and common fatal signals.
+ *
+ * Flushing strategy:
+ * - Batch-based: Auto-flush when queue reaches 10 events.
+ * - beforeExit: Async handler for clean shutdowns (when event loop empties).
+ * - Fatal signals (SIGINT, SIGTERM, SIGHUP): Best-effort sync flush.
+ * - Accepts that forced exits (SIGKILL, process.exit()) may lose final events.
+ *
+ * Call this once during CLI initialization to ensure telemetry is flushed on exit.
+ *
+ * @example
+ * ```typescript
+ * // In src/cli.mts
+ * setupTelemetryExitHandlers()
+ * ```
+ */
+export function setupTelemetryExitHandlers(): void {
+  let handlersRegistered = false
+
+  // Use beforeExit for async finalization during clean shutdowns.
+  // This fires when the event loop empties but before process actually exits.
+  process.on('beforeExit', () => {
+    if (!handlersRegistered) {
+      return
+    }
+    debug('beforeExit handler triggered')
+    void finalizeTelemetry()
+  })
+
+  // Register handlers for common fatal signals as best-effort fallback.
+  // These are synchronous contexts, so we can only trigger flush without awaiting.
+  const fatalSignals: NodeJS.Signals[] = ['SIGINT', 'SIGTERM', 'SIGHUP']
+
+  for (const signal of fatalSignals) {
+    try {
+      process.on(signal, () => {
+        if (!handlersRegistered) {
+          return
+        }
+        debug(`Signal ${signal} received, attempting sync flush`)
+        finalizeTelemetrySync()
+      })
+    } catch (e) {
+      // Some signals may not be available on all platforms.
+      debug(`Failed to register handler for signal ${signal}: ${e}`)
+    }
+  }
+
+  handlersRegistered = true
+  debug('Telemetry exit handlers registered (beforeExit + common signals)')
+}
+
 /**
  * Track subprocess exit and finalize telemetry.
  * This is a convenience function that tracks completion/error based on exit code
@@ -200,7 +282,7 @@ function sanitizeArgv(argv: string[]): string[] {
     // Remove user home directory from file paths.
     const homeDir = homedir()
     if (homeDir) {
-      return arg.replace(new RegExp(homeDir, 'g'), '~')
+      return arg.replace(new RegExp(escapeRegExp(homeDir), 'g'), '~')
     }
 
     return arg
@@ -222,7 +304,7 @@ function sanitizeErrorAttribute(input: string | undefined): string | undefined {
   // Remove user home directory.
   const homeDir = homedir()
   if (homeDir) {
-    return input.replace(new RegExp(homeDir, 'g'), '~')
+    return input.replace(new RegExp(escapeRegExp(homeDir), 'g'), '~')
   }
 
   return input
@@ -324,6 +406,7 @@ export async function trackCliEvent(
 /**
  * Track CLI completion event.
  * Should be called on successful CLI exit.
+ * Events are flushed automatically by exit handlers.
  *
  * @param argv
  * @param startTime Start timestamp from trackCliStart.
@@ -336,22 +419,16 @@ export async function trackCliComplete(
 ): Promise<void> {
   debug('Capture end of command')
 
-  await trackEvent(
-    'cli_complete',
-    buildContext(argv),
-    {
-      duration: calculateDuration(startTime),
-      exit_code: normalizeExitCode(exitCode, 0),
-    },
-    {
-      flush: true,
-    },
-  )
+  await trackEvent('cli_complete', buildContext(argv), {
+    duration: calculateDuration(startTime),
+    exit_code: normalizeExitCode(exitCode, 0),
+  })
 }
 
 /**
  * Track CLI error event.
  * Should be called when CLI exits with an error.
+ * Events are flushed automatically by exit handlers.
  *
  * @param argv
  * @param startTime Start timestamp from trackCliStart.
@@ -375,7 +452,6 @@ export async function trackCliError(
     },
     {
       error: normalizeError(error),
-      flush: true,
     },
   )
 }
diff --git a/src/utils/telemetry/integration.test.mts b/src/utils/telemetry/integration.test.mts
@@ -242,7 +242,7 @@ describe('telemetry integration', () => {
           }),
         }),
       )
-      expect(mockFlush).toHaveBeenCalled()
+      // Note: Flush now happens automatically via exit handlers, not inline.
     })
 
     it('normalizes exit code when string', async () => {
@@ -292,7 +292,7 @@ describe('telemetry integration', () => {
           }),
         }),
       )
-      expect(mockFlush).toHaveBeenCalled()
+      // Note: Flush now happens automatically via exit handlers, not inline.
     })
 
     it('normalizes non-Error objects', async () => {
diff --git a/src/utils/telemetry/service.mts b/src/utils/telemetry/service.mts
