Merge pull request #2283 from gtardif/track_command_errors

dgageot · web-flow · commit a70ca272d2d9 · 2026-04-01T09:26:56.000+02:00
Track command errors with success=false and error details
diff --git a/cmd/root/a2a.go b/cmd/root/a2a.go
@@ -35,9 +35,12 @@ func newA2ACmd() *cobra.Command {
 	return cmd
 }
 
-func (f *a2aFlags) runA2ACommand(cmd *cobra.Command, args []string) error {
+func (f *a2aFlags) runA2ACommand(cmd *cobra.Command, args []string) (commandErr error) {
 	ctx := cmd.Context()
 	telemetry.TrackCommand(ctx, "serve", append([]string{"a2a"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(ctx, "serve", append([]string{"a2a"}, args...), commandErr)
+	}()
 
 	out := cli.NewPrinter(cmd.OutOrStdout())
 	agentFilename := args[0]
diff --git a/cmd/root/acp.go b/cmd/root/acp.go
@@ -36,9 +36,12 @@ func newACPCmd() *cobra.Command {
 	return cmd
 }
 
-func (f *acpFlags) runACPCommand(cmd *cobra.Command, args []string) error {
+func (f *acpFlags) runACPCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	ctx := cmd.Context()
 	telemetry.TrackCommand(ctx, "serve", append([]string{"acp"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(ctx, "serve", append([]string{"acp"}, args...), commandErr)
+	}()
 
 	agentFilename := args[0]
 
diff --git a/cmd/root/alias.go b/cmd/root/alias.go
@@ -111,8 +111,11 @@ func newAliasRemoveCmd() *cobra.Command {
 	}
 }
 
-func runAliasAddCommand(cmd *cobra.Command, args []string, flags *aliasAddFlags) error {
+func runAliasAddCommand(cmd *cobra.Command, args []string, flags *aliasAddFlags) (commandErr error) {
 	telemetry.TrackCommand(cmd.Context(), "alias", append([]string{"add"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(cmd.Context(), "alias", append([]string{"add"}, args...), commandErr)
+	}()
 
 	out := cli.NewPrinter(cmd.OutOrStdout())
 	name := args[0]
@@ -178,8 +181,11 @@ func runAliasAddCommand(cmd *cobra.Command, args []string, flags *aliasAddFlags)
 	return nil
 }
 
-func runAliasListCommand(cmd *cobra.Command, args []string) error {
+func runAliasListCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	telemetry.TrackCommand(cmd.Context(), "alias", append([]string{"list"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(cmd.Context(), "alias", append([]string{"list"}, args...), commandErr)
+	}()
 
 	out := cli.NewPrinter(cmd.OutOrStdout())
 
@@ -234,8 +240,11 @@ func runAliasListCommand(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
-func runAliasRemoveCommand(cmd *cobra.Command, args []string) error {
+func runAliasRemoveCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	telemetry.TrackCommand(cmd.Context(), "alias", append([]string{"remove"}, args...))
+	defer func() {
+		telemetry.TrackCommandError(cmd.Context(), "alias", append([]string{"remove"}, args...), commandErr)
+	}()
 
 	out := cli.NewPrinter(cmd.OutOrStdout())
 	name := args[0]
diff --git a/cmd/root/api.go b/cmd/root/api.go
@@ -46,9 +46,12 @@ func newAPICmd() *cobra.Command {
 	return cmd
 }
 
-func (f *apiFlags) runAPICommand(cmd *cobra.Command, args []string) error {
+func (f *apiFlags) runAPICommand(cmd *cobra.Command, args []string) (commandErr error) {
 	ctx := cmd.Context()
 	telemetry.TrackCommand(ctx, "serve", append([]string{"api"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(ctx, "serve", append([]string{"api"}, args...), commandErr)
+	}()
 
 	out := cli.NewPrinter(cmd.OutOrStdout())
 	agentsPath := args[0]
diff --git a/cmd/root/debug.go b/cmd/root/debug.go
@@ -75,8 +75,11 @@ func (f *debugFlags) loadTeam(ctx context.Context, agentFilename string, opts ..
 	return t, nil
 }
 
-func (f *debugFlags) runDebugConfigCommand(cmd *cobra.Command, args []string) error {
+func (f *debugFlags) runDebugConfigCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	telemetry.TrackCommand(cmd.Context(), "debug", append([]string{"config"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(cmd.Context(), "debug", append([]string{"config"}, args...), commandErr)
+	}()
 
 	agentSource, err := config.Resolve(args[0], f.runConfig.EnvProvider())
 	if err != nil {
@@ -91,8 +94,11 @@ func (f *debugFlags) runDebugConfigCommand(cmd *cobra.Command, args []string) er
 	return yaml.NewEncoder(cmd.OutOrStdout()).Encode(cfg)
 }
 
-func (f *debugFlags) runDebugToolsetsCommand(cmd *cobra.Command, args []string) error {
+func (f *debugFlags) runDebugToolsetsCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	telemetry.TrackCommand(cmd.Context(), "debug", append([]string{"toolsets"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(cmd.Context(), "debug", append([]string{"toolsets"}, args...), commandErr)
+	}()
 
 	ctx := cmd.Context()
 
@@ -131,8 +137,11 @@ func (f *debugFlags) runDebugToolsetsCommand(cmd *cobra.Command, args []string)
 	return nil
 }
 
-func (f *debugFlags) runDebugTitleCommand(cmd *cobra.Command, args []string) error {
+func (f *debugFlags) runDebugTitleCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	telemetry.TrackCommand(cmd.Context(), "debug", append([]string{"title"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(cmd.Context(), "debug", append([]string{"title"}, args...), commandErr)
+	}()
 
 	ctx := cmd.Context()
 
diff --git a/cmd/root/debug_auth.go b/cmd/root/debug_auth.go
@@ -32,9 +32,12 @@ func newDebugAuthCmd() *cobra.Command {
 		Use:   "auth",
 		Short: "Print Docker Desktop authentication information",
 		Args:  cobra.NoArgs,
-		RunE: func(cmd *cobra.Command, _ []string) error {
+		RunE: func(cmd *cobra.Command, _ []string) (commandErr error) {
 			ctx := cmd.Context()
 			telemetry.TrackCommand(ctx, "debug", []string{"auth"})
+			defer func() { // do not inline this defer so that commandErr is not resolved early
+				telemetry.TrackCommandError(ctx, "debug", []string{"auth"}, commandErr)
+			}()
 
 			w := cmd.OutOrStdout()
 
diff --git a/cmd/root/eval.go b/cmd/root/eval.go
@@ -49,8 +49,11 @@ func newEvalCmd() *cobra.Command {
 	return cmd
 }
 
-func (f *evalFlags) runEvalCommand(cmd *cobra.Command, args []string) error {
+func (f *evalFlags) runEvalCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	telemetry.TrackCommand(cmd.Context(), "eval", args)
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(cmd.Context(), "eval", args, commandErr)
+	}()
 
 	ctx := cmd.Context()
 	agentFilename := args[0]
diff --git a/cmd/root/mcp.go b/cmd/root/mcp.go
@@ -38,9 +38,12 @@ func newMCPCmd() *cobra.Command {
 	return cmd
 }
 
-func (f *mcpFlags) runMCPCommand(cmd *cobra.Command, args []string) error {
+func (f *mcpFlags) runMCPCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	ctx := cmd.Context()
 	telemetry.TrackCommand(ctx, "serve", append([]string{"mcp"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(ctx, "serve", append([]string{"mcp"}, args...), commandErr)
+	}()
 
 	agentFilename := args[0]
 
diff --git a/cmd/root/new.go b/cmd/root/new.go
@@ -50,9 +50,12 @@ Optionally provide a description as an argument to skip the initial prompt.`,
 	return cmd
 }
 
-func (f *newFlags) runNewCommand(cmd *cobra.Command, args []string) error {
+func (f *newFlags) runNewCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	ctx := cmd.Context()
 	telemetry.TrackCommand(ctx, "new", args)
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(ctx, "new", args, commandErr)
+	}()
 
 	t, err := creator.Agent(ctx, &f.runConfig, f.modelParam)
 	if err != nil {
diff --git a/cmd/root/pull.go b/cmd/root/pull.go
@@ -34,9 +34,12 @@ func newPullCmd() *cobra.Command {
 	return cmd
 }
 
-func (f *pullFlags) runPullCommand(cmd *cobra.Command, args []string) error {
+func (f *pullFlags) runPullCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	ctx := cmd.Context()
 	telemetry.TrackCommand(ctx, "share", append([]string{"pull"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(ctx, "share", append([]string{"pull"}, args...), commandErr)
+	}()
 
 	out := cli.NewPrinter(cmd.OutOrStdout())
 	registryRef := args[0]
diff --git a/cmd/root/push.go b/cmd/root/push.go
@@ -24,9 +24,12 @@ func newPushCmd() *cobra.Command {
 	}
 }
 
-func runPushCommand(cmd *cobra.Command, args []string) error {
+func runPushCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	ctx := cmd.Context()
 	telemetry.TrackCommand(ctx, "share", append([]string{"push"}, args...))
+	defer func() { // do not inline this defer so that commandErr is not resolved early
+		telemetry.TrackCommandError(ctx, "share", append([]string{"push"}, args...), commandErr)
+	}()
 
 	agentFilename := args[0]
 	tag := args[1]
diff --git a/cmd/root/run.go b/cmd/root/run.go
@@ -129,18 +129,24 @@ func addRunOrExecFlags(cmd *cobra.Command, flags *runExecFlags) {
 	cmd.PersistentFlags().BoolVar(&flags.outputJSON, "json", false, "Output results in JSON format")
 }
 
-func (f *runExecFlags) runRunCommand(cmd *cobra.Command, args []string) error {
-	// If --sandbox is set, delegate everything to docker sandbox.
-	if f.sandbox {
-		return runInSandbox(cmd, &f.runConfig, f.sandboxTemplate)
-	}
-
+func (f *runExecFlags) runRunCommand(cmd *cobra.Command, args []string) (commandErr error) {
 	ctx := cmd.Context()
 
 	if f.exec {
 		telemetry.TrackCommand(ctx, "exec", args)
+		defer func() { // do not inline this defer so that commandErr is not resolved early
+			telemetry.TrackCommandError(ctx, "exec", args, commandErr)
+		}()
 	} else {
 		telemetry.TrackCommand(ctx, "run", args)
+		defer func() { // do not inline this defer so that commandErr is not resolved early
+			telemetry.TrackCommandError(ctx, "run", args, commandErr)
+		}()
+	}
+
+	// If --sandbox is set, delegate everything to docker sandbox.
+	if f.sandbox {
+		return runInSandbox(cmd, &f.runConfig, f.sandboxTemplate)
 	}
 
 	out := cli.NewPrinter(cmd.OutOrStdout())
diff --git a/pkg/telemetry/client.go b/pkg/telemetry/client.go
@@ -49,9 +49,10 @@ func newClient(ctx context.Context, logger *slog.Logger, enabled, debugMode bool
 
 	if !enabled {
 		return &Client{
-			logger:  telemetryLogger,
-			enabled: false,
-			version: version,
+			logger:    telemetryLogger,
+			enabled:   false,
+			debugMode: debugMode,
+			version:   version,
 		}
 	}
 
diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go
@@ -5,9 +5,17 @@ import (
 	"time"
 )
 
+func (tc *Client) TrackSynchronous(_ context.Context, structuredEvent StructuredEvent) {
+	tc.track(context.Background(), structuredEvent, false)
+}
+
 // Track records a structured telemetry event with type-safe properties (synchronous)
 // This is the only method for telemetry tracking, all event-specific methods are wrappers around this one
-func (tc *Client) Track(_ context.Context, structuredEvent StructuredEvent) {
+func (tc *Client) Track(ctx context.Context, structuredEvent StructuredEvent) {
+	tc.track(ctx, structuredEvent, true)
+}
+
+func (tc *Client) track(_ context.Context, structuredEvent StructuredEvent, async bool) {
 	eventType := structuredEvent.GetEventType()
 	structuredProps := structuredEvent.ToStructuredProperties()
 
@@ -19,10 +27,6 @@ func (tc *Client) Track(_ context.Context, structuredEvent StructuredEvent) {
 		return
 	}
 
-	if !tc.enabled {
-		return
-	}
-
 	// Debug logging to track event flow
 	if tc.debugMode {
 		tc.logger.Debug("Processing telemetry event synchronously", "event_type", eventType)
@@ -34,9 +38,15 @@ func (tc *Client) Track(_ context.Context, structuredEvent StructuredEvent) {
 		tc.printEvent(&event)
 	}
 
-	go func() {
+	if !tc.enabled {
+		return
+	}
+
+	if async {
+		go tc.sendEvent(&event)
+	} else {
 		tc.sendEvent(&event)
-	}()
+	}
 }
 
 // RecordSessionStart initializes session tracking
diff --git a/pkg/telemetry/global.go b/pkg/telemetry/global.go
@@ -21,6 +21,25 @@ func TrackCommand(ctx context.Context, action string, args []string) {
 	}
 }
 
+// TrackCommandError records a command eventerror, send telemetry synchronously to ensure it is sent before the process exits after error
+// do nothing if err == nil
+func TrackCommandError(ctx context.Context, action string, args []string, err error) {
+	if err == nil {
+		return
+	}
+	EnsureGlobalTelemetryInitialized(ctx)
+
+	if globalToolTelemetryClient != nil {
+		commandEvent := CommandEvent{
+			Action:  action,
+			Args:    args,
+			Error:   err.Error(),
+			Success: false,
+		}
+		globalToolTelemetryClient.TrackSynchronous(ctx, &commandEvent)
+	}
+}
+
 // Global variables for simple tool telemetry
 var (
 	globalToolTelemetryClient *Client
