Przeglądaj źródła

feat(telemetry): add PostHog exception tracking for consecutive mistake errors (#10193)

Daniel 1 tydzień temu
rodzic
commit
3a2ad6b0e1

+ 13 - 4
packages/telemetry/src/PostHogTelemetryClient.ts

@@ -10,6 +10,8 @@ import {
 	shouldReportApiErrorToTelemetry,
 	isApiProviderError,
 	extractApiProviderErrorProperties,
+	isConsecutiveMistakeError,
+	extractConsecutiveMistakeErrorProperties,
 } from "@roo-code/types"
 
 import { BaseTelemetryClient } from "./BaseTelemetryClient"
@@ -63,10 +65,12 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
 			console.info(`[PostHogTelemetryClient#capture] ${event.event}`)
 		}
 
+		const properties = await this.getEventProperties(event)
+
 		this.client.capture({
 			distinctId: this.distinctId,
 			event: event.event,
-			properties: await this.getEventProperties(event),
+			properties,
 		})
 	}
 
@@ -100,13 +104,16 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
 			console.info(`[PostHogTelemetryClient#captureException] ${error.message}`)
 		}
 
-		// Auto-extract properties from ApiProviderError and merge with additionalProperties.
+		// Auto-extract properties from known error types and merge with additionalProperties.
 		// Explicit additionalProperties take precedence over auto-extracted properties.
 		let mergedProperties = additionalProperties
 
 		if (isApiProviderError(error)) {
 			const extractedProperties = extractApiProviderErrorProperties(error)
 			mergedProperties = { ...extractedProperties, ...additionalProperties }
+		} else if (isConsecutiveMistakeError(error)) {
+			const extractedProperties = extractConsecutiveMistakeErrorProperties(error)
+			mergedProperties = { ...extractedProperties, ...additionalProperties }
 		}
 
 		// Override the error message with the extracted error message.
@@ -123,10 +130,12 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
 			}
 		}
 
-		this.client.captureException(error, this.distinctId, {
+		const exceptionProperties = {
 			...mergedProperties,
 			$app_version: telemetryProperties?.appVersion,
-		})
+		}
+
+		this.client.captureException(error, this.distinctId, exceptionProperties)
 	}
 
 	/**

+ 184 - 0
packages/types/src/__tests__/telemetry.test.ts

@@ -9,6 +9,9 @@ import {
 	ApiProviderError,
 	isApiProviderError,
 	extractApiProviderErrorProperties,
+	ConsecutiveMistakeError,
+	isConsecutiveMistakeError,
+	extractConsecutiveMistakeErrorProperties,
 } from "../telemetry.js"
 
 describe("telemetry error utilities", () => {
@@ -389,4 +392,185 @@ describe("telemetry error utilities", () => {
 			expect(properties).toHaveProperty("errorCode", 0)
 		})
 	})
+
+	describe("ConsecutiveMistakeError", () => {
+		it("should create an error with correct properties", () => {
+			const error = new ConsecutiveMistakeError("Test error", "task-123", 5, 3, "no_tools_used")
+
+			expect(error.message).toBe("Test error")
+			expect(error.name).toBe("ConsecutiveMistakeError")
+			expect(error.taskId).toBe("task-123")
+			expect(error.consecutiveMistakeCount).toBe(5)
+			expect(error.consecutiveMistakeLimit).toBe(3)
+			expect(error.reason).toBe("no_tools_used")
+		})
+
+		it("should create an error with provider and modelId", () => {
+			const error = new ConsecutiveMistakeError(
+				"Test error",
+				"task-123",
+				5,
+				3,
+				"no_tools_used",
+				"anthropic",
+				"claude-3-sonnet-20240229",
+			)
+
+			expect(error.message).toBe("Test error")
+			expect(error.name).toBe("ConsecutiveMistakeError")
+			expect(error.taskId).toBe("task-123")
+			expect(error.consecutiveMistakeCount).toBe(5)
+			expect(error.consecutiveMistakeLimit).toBe(3)
+			expect(error.reason).toBe("no_tools_used")
+			expect(error.provider).toBe("anthropic")
+			expect(error.modelId).toBe("claude-3-sonnet-20240229")
+		})
+
+		it("should be an instance of Error", () => {
+			const error = new ConsecutiveMistakeError("Test error", "task-123", 3, 3)
+			expect(error).toBeInstanceOf(Error)
+		})
+
+		it("should handle zero values", () => {
+			const error = new ConsecutiveMistakeError("Zero test", "task-000", 0, 0)
+
+			expect(error.taskId).toBe("task-000")
+			expect(error.consecutiveMistakeCount).toBe(0)
+			expect(error.consecutiveMistakeLimit).toBe(0)
+		})
+
+		it("should default reason to unknown when not provided", () => {
+			const error = new ConsecutiveMistakeError("Test error", "task-123", 3, 3)
+			expect(error.reason).toBe("unknown")
+		})
+
+		it("should accept tool_repetition reason", () => {
+			const error = new ConsecutiveMistakeError("Test error", "task-123", 3, 3, "tool_repetition")
+			expect(error.reason).toBe("tool_repetition")
+		})
+
+		it("should accept no_tools_used reason", () => {
+			const error = new ConsecutiveMistakeError("Test error", "task-123", 3, 3, "no_tools_used")
+			expect(error.reason).toBe("no_tools_used")
+		})
+
+		it("should have undefined provider and modelId when not provided", () => {
+			const error = new ConsecutiveMistakeError("Test error", "task-123", 3, 3, "no_tools_used")
+			expect(error.provider).toBeUndefined()
+			expect(error.modelId).toBeUndefined()
+		})
+	})
+
+	describe("isConsecutiveMistakeError", () => {
+		it("should return true for ConsecutiveMistakeError instances", () => {
+			const error = new ConsecutiveMistakeError("Test error", "task-123", 3, 3)
+			expect(isConsecutiveMistakeError(error)).toBe(true)
+		})
+
+		it("should return false for regular Error instances", () => {
+			const error = new Error("Test error")
+			expect(isConsecutiveMistakeError(error)).toBe(false)
+		})
+
+		it("should return false for ApiProviderError instances", () => {
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
+			expect(isConsecutiveMistakeError(error)).toBe(false)
+		})
+
+		it("should return false for null and undefined", () => {
+			expect(isConsecutiveMistakeError(null)).toBe(false)
+			expect(isConsecutiveMistakeError(undefined)).toBe(false)
+		})
+
+		it("should return false for non-error objects", () => {
+			expect(isConsecutiveMistakeError({})).toBe(false)
+			expect(
+				isConsecutiveMistakeError({
+					taskId: "task-123",
+					consecutiveMistakeCount: 3,
+					consecutiveMistakeLimit: 3,
+				}),
+			).toBe(false)
+		})
+
+		it("should return false for Error with wrong name", () => {
+			const error = new Error("Test error")
+			error.name = "CustomError"
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any
+			;(error as any).taskId = "task-123"
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any
+			;(error as any).consecutiveMistakeCount = 3
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any
+			;(error as any).consecutiveMistakeLimit = 3
+			expect(isConsecutiveMistakeError(error)).toBe(false)
+		})
+	})
+
+	describe("extractConsecutiveMistakeErrorProperties", () => {
+		it("should extract all properties from ConsecutiveMistakeError", () => {
+			const error = new ConsecutiveMistakeError("Test error", "task-123", 5, 3, "no_tools_used")
+			const properties = extractConsecutiveMistakeErrorProperties(error)
+
+			expect(properties).toEqual({
+				taskId: "task-123",
+				consecutiveMistakeCount: 5,
+				consecutiveMistakeLimit: 3,
+				reason: "no_tools_used",
+			})
+		})
+
+		it("should extract all properties including provider and modelId", () => {
+			const error = new ConsecutiveMistakeError(
+				"Test error",
+				"task-123",
+				5,
+				3,
+				"no_tools_used",
+				"anthropic",
+				"claude-3-sonnet-20240229",
+			)
+			const properties = extractConsecutiveMistakeErrorProperties(error)
+
+			expect(properties).toEqual({
+				taskId: "task-123",
+				consecutiveMistakeCount: 5,
+				consecutiveMistakeLimit: 3,
+				reason: "no_tools_used",
+				provider: "anthropic",
+				modelId: "claude-3-sonnet-20240229",
+			})
+		})
+
+		it("should not include provider and modelId when undefined", () => {
+			const error = new ConsecutiveMistakeError("Test error", "task-123", 5, 3, "no_tools_used")
+			const properties = extractConsecutiveMistakeErrorProperties(error)
+
+			expect(properties).not.toHaveProperty("provider")
+			expect(properties).not.toHaveProperty("modelId")
+		})
+
+		it("should handle zero values correctly", () => {
+			const error = new ConsecutiveMistakeError("Zero test", "task-000", 0, 0)
+			const properties = extractConsecutiveMistakeErrorProperties(error)
+
+			expect(properties).toEqual({
+				taskId: "task-000",
+				consecutiveMistakeCount: 0,
+				consecutiveMistakeLimit: 0,
+				reason: "unknown",
+			})
+		})
+
+		it("should handle large numbers", () => {
+			const error = new ConsecutiveMistakeError("Large test", "task-large", 1000, 500, "tool_repetition")
+			const properties = extractConsecutiveMistakeErrorProperties(error)
+
+			expect(properties).toEqual({
+				taskId: "task-large",
+				consecutiveMistakeCount: 1000,
+				consecutiveMistakeLimit: 500,
+				reason: "tool_repetition",
+			})
+		})
+	})
 })

+ 54 - 0
packages/types/src/telemetry.ts

@@ -477,3 +477,57 @@ export function extractApiProviderErrorProperties(error: ApiProviderError): Reco
 		...(error.errorCode !== undefined && { errorCode: error.errorCode }),
 	}
 }
+
+/**
+ * Reason why the consecutive mistake limit was reached.
+ */
+export type ConsecutiveMistakeReason = "no_tools_used" | "tool_repetition" | "unknown"
+
+/**
+ * Error class for "Roo is having trouble" consecutive mistake scenarios.
+ * Triggered when the task reaches the configured consecutive mistake limit.
+ * Used for structured exception tracking via PostHog.
+ */
+export class ConsecutiveMistakeError extends Error {
+	constructor(
+		message: string,
+		public readonly taskId: string,
+		public readonly consecutiveMistakeCount: number,
+		public readonly consecutiveMistakeLimit: number,
+		public readonly reason: ConsecutiveMistakeReason = "unknown",
+		public readonly provider?: string,
+		public readonly modelId?: string,
+	) {
+		super(message)
+		this.name = "ConsecutiveMistakeError"
+	}
+}
+
+/**
+ * Type guard to check if an error is a ConsecutiveMistakeError.
+ * Used by telemetry to automatically extract structured properties.
+ */
+export function isConsecutiveMistakeError(error: unknown): error is ConsecutiveMistakeError {
+	return (
+		error instanceof Error &&
+		error.name === "ConsecutiveMistakeError" &&
+		"taskId" in error &&
+		"consecutiveMistakeCount" in error &&
+		"consecutiveMistakeLimit" in error
+	)
+}
+
+/**
+ * Extracts properties from a ConsecutiveMistakeError for telemetry.
+ * Returns the structured properties that can be merged with additionalProperties.
+ */
+export function extractConsecutiveMistakeErrorProperties(error: ConsecutiveMistakeError): Record<string, unknown> {
+	return {
+		taskId: error.taskId,
+		consecutiveMistakeCount: error.consecutiveMistakeCount,
+		consecutiveMistakeLimit: error.consecutiveMistakeLimit,
+		reason: error.reason,
+		...(error.provider !== undefined && { provider: error.provider }),
+		...(error.modelId !== undefined && { modelId: error.modelId }),
+	}
+}

+ 15 - 3
src/core/assistant-message/presentAssistantMessage.ts

@@ -3,6 +3,7 @@ import { serializeError } from "serialize-error"
 import { Anthropic } from "@anthropic-ai/sdk"
 
 import type { ToolName, ClineAsk, ToolProgressStatus } from "@roo-code/types"
+import { ConsecutiveMistakeError } from "@roo-code/types"
 import { TelemetryService } from "@roo-code/telemetry"
 
 import { t } from "../../i18n"
@@ -778,11 +779,22 @@ export async function presentAssistantMessage(cline: Task) {
 
 						// Add user feedback to chat.
 						await cline.say("user_feedback", text, images)
-
-						// Track tool repetition in telemetry.
-						TelemetryService.instance.captureConsecutiveMistakeError(cline.taskId)
 					}
 
+					// Track tool repetition in telemetry via PostHog exception tracking and event.
+					TelemetryService.instance.captureConsecutiveMistakeError(cline.taskId)
+					TelemetryService.instance.captureException(
+						new ConsecutiveMistakeError(
+							`Tool repetition limit reached for ${block.name}`,
+							cline.taskId,
+							cline.consecutiveMistakeCount,
+							cline.consecutiveMistakeLimit,
+							"tool_repetition",
+							cline.apiConfiguration.apiProvider,
+							cline.api.getModel().id,
+						),
+					)
+
 					// Return tool result message about the repetition
 					pushToolResult(
 						formatResponse.toolError(

+ 17 - 3
src/core/task/Task.ts

@@ -49,6 +49,7 @@ import {
 	MAX_CHECKPOINT_TIMEOUT_SECONDS,
 	MIN_CHECKPOINT_TIMEOUT_SECONDS,
 	TOOL_PROTOCOL,
+	ConsecutiveMistakeError,
 } from "@roo-code/types"
 import { TelemetryService } from "@roo-code/telemetry"
 import { CloudService, BridgeOrchestrator } from "@roo-code/cloud"
@@ -2263,6 +2264,22 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 			}
 
 			if (this.consecutiveMistakeLimit > 0 && this.consecutiveMistakeCount >= this.consecutiveMistakeLimit) {
+				// Track consecutive mistake errors in telemetry via event and PostHog exception tracking.
+				// The reason is "no_tools_used" because this limit is reached via initiateTaskLoop
+				// which increments consecutiveMistakeCount when the model doesn't use any tools.
+				TelemetryService.instance.captureConsecutiveMistakeError(this.taskId)
+				TelemetryService.instance.captureException(
+					new ConsecutiveMistakeError(
+						`Task reached consecutive mistake limit (${this.consecutiveMistakeLimit})`,
+						this.taskId,
+						this.consecutiveMistakeCount,
+						this.consecutiveMistakeLimit,
+						"no_tools_used",
+						this.apiConfiguration.apiProvider,
+						getModelId(this.apiConfiguration),
+					),
+				)
+
 				const { response, text, images } = await this.ask(
 					"mistake_limit_reached",
 					t("common:errors.mistake_limit_guidance"),
@@ -2277,9 +2294,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 					)
 
 					await this.say("user_feedback", text, images)
-
-					// Track consecutive mistake errors in telemetry.
-					TelemetryService.instance.captureConsecutiveMistakeError(this.taskId)
 				}
 
 				this.consecutiveMistakeCount = 0