Merge pull request #72 from golf-mcp/asch/telemetry-error-differenciation

Asch/telemetry error differentiation

Author: aschlean

.github/SECURITY.md

@@ -4,7 +4,7 @@
 
 | Version | Supported          |
 | ------- | ------------------ |
-| 0.1.12  | :white_check_mark: |
+| 0.1.13  | :white_check_mark: |
 
 
 ## Reporting a Vulnerability

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "golf-mcp"
-version = "0.1.12"
+version = "0.1.13"
 description = "Framework for building MCP servers"
 authors = [
     {name = "Antoni Gmitruk", email = "antoni@golf.dev"}
@@ -64,7 +64,7 @@ golf = ["examples/**/*"]
 
 [tool.poetry]
 name = "golf-mcp"
-version = "0.1.12"
+version = "0.1.13"
 description = "Framework for building MCP servers with zero boilerplate"
 authors = ["Antoni Gmitruk <antoni@golf.dev>"]
 license = "Apache-2.0"

src/golf/__init__.py

@@ -1 +1 @@
-__version__ = "0.1.12"
+__version__ = "0.1.13"

src/golf/cli/main.py

@@ -15,6 +15,8 @@
     set_telemetry_enabled,
     shutdown,
     track_event,
+    track_detailed_error,
+    _detect_execution_environment,
 )
 
 # Create console for rich output
@@ -156,16 +158,12 @@ def build_dev(
         # Track successful build with environment
         track_event("cli_build_success", {"success": True, "environment": "dev"})
     except Exception as e:
-        error_type = type(e).__name__
-        error_message = str(e)
-        track_event(
+        track_detailed_error(
             "cli_build_failed",
-            {
-                "success": False,
-                "environment": "dev",
-                "error_type": error_type,
-                "error_message": error_message,
-            },
+            e,
+            context="Development build with environment variables",
+            operation="build_dev",
+            additional_props={"environment": "dev", "copy_env": True}
         )
         raise
 
@@ -212,16 +210,12 @@ def build_prod(
         # Track successful build with environment
         track_event("cli_build_success", {"success": True, "environment": "prod"})
     except Exception as e:
-        error_type = type(e).__name__
-        error_message = str(e)
-        track_event(
+        track_detailed_error(
             "cli_build_failed",
-            {
-                "success": False,
-                "environment": "prod",
-                "error_type": error_type,
-                "error_message": error_message,
-            },
+            e,
+            context="Production build without environment variables",
+            operation="build_prod",
+            additional_props={"environment": "prod", "copy_env": False}
         )
         raise
 
@@ -282,18 +276,15 @@ def run(
 
                 build_project(project_root, settings, dist_dir)
             except Exception as e:
-                error_type = type(e).__name__
-                error_message = str(e)
                 console.print(
-                    f"[bold red]Error building project:[/bold red] {error_message}"
+                    f"[bold red]Error building project:[/bold red] {str(e)}"
                 )
-                track_event(
+                track_detailed_error(
                     "cli_run_failed",
-                    {
-                        "success": False,
-                        "error_type": f"BuildError.{error_type}",
-                        "error_message": error_message,
-                    },
+                    e,
+                    context="Auto-build before running server",
+                    operation="auto_build_before_run",
+                    additional_props={"auto_build": True}
                 )
                 raise
         else:
@@ -325,32 +316,56 @@ def run(
             port=port,
         )
 
-        # Track based on return code
+        # Track based on return code with better categorization
         if return_code == 0:
             track_event("cli_run_success", {"success": True})
+        elif return_code in [130, 143, 137, 2]:
+            # Intentional shutdowns (not errors):
+            # 130: Ctrl+C (SIGINT)
+            # 143: SIGTERM (graceful shutdown, e.g., Kubernetes, Docker)
+            # 137: SIGKILL (forced shutdown)
+            # 2: General interrupt/graceful shutdown
+            shutdown_type = {
+                130: "UserInterrupt",
+                143: "GracefulShutdown", 
+                137: "ForcedShutdown",
+                2: "Interrupt"
+            }.get(return_code, "GracefulShutdown")
+            
+            track_event(
+                "cli_run_shutdown",
+                {
+                    "success": True,  # Not an error
+                    "shutdown_type": shutdown_type,
+                    "exit_code": return_code,
+                },
+            )
         else:
+            # Actual errors (unexpected exit codes)
             track_event(
                 "cli_run_failed",
                 {
                     "success": False,
-                    "error_type": "NonZeroExit",
-                    "error_message": f"Server exited with code {return_code}",
+                    "error_type": "UnexpectedExit",
+                    "error_message": f"Server process exited unexpectedly with code {return_code}",
+                    "exit_code": return_code,
+                    "operation": "server_process_execution",
+                    "context": "Server process terminated with unexpected exit code",
+                    # Add execution environment context
+                    "execution_env": _detect_execution_environment(),
                 },
             )
 
         # Exit with the same code as the server
         if return_code != 0:
             raise typer.Exit(code=return_code)
     except Exception as e:
-        error_type = type(e).__name__
-        error_message = str(e)
-        track_event(
+        track_detailed_error(
             "cli_run_failed",
-            {
-                "success": False,
-                "error_type": error_type,
-                "error_message": error_message,
-            },
+            e,
+            context="Server execution or startup failure",
+            operation="run_server_execution",
+            additional_props={"has_dist_dir": dist_dir.exists() if dist_dir else False}
         )
         raise
 

src/golf/commands/run.py

@@ -64,10 +64,24 @@ def run_server(
             env=env,
         )
 
+        # Provide more context about the exit
+        if process.returncode == 0:
+            console.print("[green]Server stopped successfully[/green]")
+        elif process.returncode == 130:
+            console.print("[yellow]Server stopped by user interrupt (Ctrl+C)[/yellow]")
+        elif process.returncode == 143:
+            console.print("[yellow]Server stopped by SIGTERM (graceful shutdown)[/yellow]")
+        elif process.returncode == 137:
+            console.print("[yellow]Server stopped by SIGKILL (forced shutdown)[/yellow]")
+        elif process.returncode in [1, 2]:
+            console.print(f"[red]Server exited with error code {process.returncode}[/red]")
+        else:
+            console.print(f"[orange]Server exited with code {process.returncode}[/orange]")
+
         return process.returncode
     except KeyboardInterrupt:
-        console.print("\n[yellow]Server stopped by user[/yellow]")
-        return 0
+        console.print("\n[yellow]Server stopped by user (Ctrl+C)[/yellow]")
+        return 130  # Standard exit code for SIGINT
     except Exception as e:
         console.print(f"\n[bold red]Error running server:[/bold red] {e}")
         return 1

src/golf/core/builder.py

@@ -950,6 +950,23 @@ def build_project(
             import traceback
 
             console.print(f"[red]{traceback.format_exc()}[/red]")
+            
+            # Track detailed error for pre_build.py execution failures
+            try:
+                from golf.core.telemetry import track_detailed_error
+                track_detailed_error(
+                    "build_pre_build_failed",
+                    e,
+                    context="Executing pre_build.py configuration script",
+                    operation="pre_build_execution",
+                    additional_props={
+                        "file_path": str(pre_build_path.relative_to(project_path)),
+                        "build_env": build_env,
+                    }
+                )
+            except Exception:
+                # Don't let telemetry errors break the build
+                pass
 
     # Clear the output directory if it exists
     if output_dir.exists():

src/golf/core/telemetry.py

@@ -281,6 +281,8 @@ def track_event(event_name: str, properties: dict[str, Any] | None = None) -> No
                 "command_type",
                 "error_type",
                 "error_message",
+                "shutdown_type",
+                "exit_code",
             }
             for key in safe_keys:
                 if key in properties:
@@ -326,26 +328,98 @@ def track_command(
     track_event(f"cli_{command}", properties)
 
 
-def _sanitize_error_message(message: str) -> str:
-    """Sanitize error message to remove sensitive information.
+def track_detailed_error(
+    event_name: str,
+    error: Exception,
+    context: str | None = None,
+    operation: str | None = None,
+    additional_props: dict[str, Any] | None = None,
+) -> None:
+    """Track a detailed error with enhanced debugging information.
 
     Args:
-        message: Raw error message
-
-    Returns:
-        Sanitized error message
+        event_name: Name of the error event (e.g., "cli_run_failed", "cli_build_failed")
+        error: The exception that occurred
+        context: Additional context about where the error occurred
+        operation: The specific operation that failed
+        additional_props: Additional properties to include
     """
+    import traceback
+    import time
+
+    properties = {
+        "success": False,
+        "error_type": type(error).__name__,
+        "error_message": _sanitize_error_message(str(error)),
+        "timestamp": int(time.time()),
+    }
+
+    # Add operation context
+    if operation:
+        properties["operation"] = operation
+    if context:
+        properties["context"] = context
+
+    # Add sanitized stack trace for debugging
+    try:
+        tb_lines = traceback.format_exception(type(error), error, error.__traceback__)
+        # Get the last few frames (most relevant) and sanitize them
+        relevant_frames = tb_lines[-3:] if len(tb_lines) > 3 else tb_lines
+        sanitized_trace = []
+        
+        for frame in relevant_frames:
+            # Sanitize file paths in stack trace
+            sanitized_frame = _sanitize_error_message(frame.strip())
+            # Further sanitize common traceback patterns
+            sanitized_frame = sanitized_frame.replace('File "[PATH]', 'File "[PATH]')
+            sanitized_trace.append(sanitized_frame)
+        
+        properties["stack_trace"] = " | ".join(sanitized_trace)
+        
+        # Add the specific line that caused the error if available
+        if hasattr(error, '__traceback__') and error.__traceback__:
+            tb = error.__traceback__
+            while tb.tb_next:
+                tb = tb.tb_next
+            properties["error_line"] = tb.tb_lineno
+            
+    except Exception:
+        # Don't fail if we can't capture stack trace
+        pass
+
+    # Add system context for debugging
+    try:
+        properties["python_executable"] = _sanitize_error_message(platform.python_implementation())
+        properties["platform_detail"] = platform.platform()[:50]  # Limit length
+    except Exception:
+        pass
+
+    # Merge additional properties
+    if additional_props:
+        # Only include safe additional properties
+        safe_additional_keys = {
+            "exit_code", "shutdown_type", "environment", "template",
+            "build_env", "transport", "component_count", "file_path",
+            "component_type", "validation_error", "config_error"
+        }
+        for key, value in additional_props.items():
+            if key in safe_additional_keys:
+                properties[key] = value
+
+    track_event(event_name, properties)
+
+def _sanitize_error_message(message: str) -> str:
+    """Sanitize error messages to remove sensitive information."""
     import re
 
-    # Remove absolute file paths but keep the filename
-    # Unix-style paths
-    message = re.sub(
-        r'/(?:Users|home|var|tmp|opt|usr|etc)/[^\s"\']+/([^/\s"\']+)', r"\1", message
-    )
-    # Windows-style paths
-    message = re.sub(r'[A-Za-z]:\\[^\s"\']+\\([^\\s"\']+)', r"\1", message)
-    # Generic path pattern (catches remaining paths)
-    message = re.sub(r'(?:^|[\s"])(/[^\s"\']+/)+([^/\s"\']+)', r"\2", message)
+    # Remove file paths but preserve filenames
+    # Match paths with directories and capture the filename
+    # Unix style: /path/to/file.py -> file.py
+    message = re.sub(r"(/[^/\s]+)+/([^/\s]+)", r"\2", message)
+    # Windows style: C:\path\to\file.py -> file.py
+    message = re.sub(r"([A-Za-z]:\\[^\\]+\\)+([^\\]+)", r"\2", message)
+    # Remaining absolute paths without filename
+    message = re.sub(r"[/\\][^\s]*[/\\]", "[PATH]/", message)
 
     # Remove potential API keys or tokens (common patterns)
     # Generic API keys (20+ alphanumeric with underscores/hyphens)