From a2a151f803d93a9418cf13f9c94d332b44e267b0 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 18 Dec 2025 14:53:36 -0500 Subject: [PATCH 1/2] feat(telemetry): add PostHog exception tracking for consecutive mistake errors Add structured exception tracking via PostHog for 'Roo is having trouble' events: - Add ConsecutiveMistakeError class with reason field (no_tools_used, tool_repetition, unknown) - Add type guard isConsecutiveMistakeError() and property extractor - Update PostHogTelemetryClient.captureException() to auto-extract properties - Track consecutive mistake limit reached in Task.ts with 'no_tools_used' reason - Track tool repetition limit in presentAssistantMessage.ts with 'tool_repetition' reason - Add comprehensive tests (69 tests pass) --- .../telemetry/src/PostHogTelemetryClient.ts | 7 +- .../types/src/__tests__/telemetry.test.ts | 127 ++++++++++++++++++ packages/types/src/telemetry.ts | 50 +++++++ .../presentAssistantMessage.ts | 13 +- src/core/task/Task.ts | 15 ++- 5 files changed, 207 insertions(+), 5 deletions(-) diff --git a/packages/telemetry/src/PostHogTelemetryClient.ts b/packages/telemetry/src/PostHogTelemetryClient.ts index f5eacd81915..b8b6586716d 100644 --- a/packages/telemetry/src/PostHogTelemetryClient.ts +++ b/packages/telemetry/src/PostHogTelemetryClient.ts @@ -10,6 +10,8 @@ import { shouldReportApiErrorToTelemetry, isApiProviderError, extractApiProviderErrorProperties, + isConsecutiveMistakeError, + extractConsecutiveMistakeErrorProperties, } from "@roo-code/types" import { BaseTelemetryClient } from "./BaseTelemetryClient" @@ -100,13 +102,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. diff --git a/packages/types/src/__tests__/telemetry.test.ts b/packages/types/src/__tests__/telemetry.test.ts index 74d0f1ddfe3..d5130a35aaa 100644 --- a/packages/types/src/__tests__/telemetry.test.ts +++ b/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,128 @@ 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 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") + }) + }) + + 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 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", + }) + }) + }) }) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index 5e96aa70416..7b5929cca17 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -477,3 +477,53 @@ 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", + ) { + 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 { + return { + taskId: error.taskId, + consecutiveMistakeCount: error.consecutiveMistakeCount, + consecutiveMistakeLimit: error.consecutiveMistakeLimit, + reason: error.reason, + } +} diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 2e8b791b349..4066cba6301 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/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" @@ -776,8 +777,16 @@ 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. + TelemetryService.instance.captureException( + new ConsecutiveMistakeError( + `Tool repetition limit reached for ${block.name}`, + cline.taskId, + cline.consecutiveMistakeCount, + cline.consecutiveMistakeLimit, + "tool_repetition", + ), + ) } // Return tool result message about the repetition diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 9161d3b462a..4c184bdf673 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -48,6 +48,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" @@ -2236,8 +2237,18 @@ export class Task extends EventEmitter implements TaskLike { await this.say("user_feedback", text, images) - // Track consecutive mistake errors in telemetry. - TelemetryService.instance.captureConsecutiveMistakeError(this.taskId) + // Track consecutive mistake errors in telemetry via 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.captureException( + new ConsecutiveMistakeError( + `Task reached consecutive mistake limit (${this.consecutiveMistakeLimit})`, + this.taskId, + this.consecutiveMistakeCount, + this.consecutiveMistakeLimit, + "no_tools_used", + ), + ) } this.consecutiveMistakeCount = 0 From 2ab2df54a4cc70b9dcfa01c30d6c13d8137f8aca Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 18 Dec 2025 15:50:21 -0500 Subject: [PATCH 2/2] feat: add PostHog exception tracking for consecutive mistake errors - Add ConsecutiveMistakeError class with provider and modelId tracking - Add type guard and property extraction for ConsecutiveMistakeError - Update PostHogTelemetryClient to handle ConsecutiveMistakeError - Track 'Roo is having trouble' events via captureException in Task.ts and presentAssistantMessage.ts - Include both telemetry event (captureConsecutiveMistakeError) and exception tracking - Add comprehensive tests for new error tracking functionality --- .../telemetry/src/PostHogTelemetryClient.ts | 10 +++- .../types/src/__tests__/telemetry.test.ts | 57 +++++++++++++++++++ packages/types/src/telemetry.ts | 4 ++ .../presentAssistantMessage.ts | 25 ++++---- src/core/task/Task.ts | 29 +++++----- 5 files changed, 98 insertions(+), 27 deletions(-) diff --git a/packages/telemetry/src/PostHogTelemetryClient.ts b/packages/telemetry/src/PostHogTelemetryClient.ts index b8b6586716d..adc8b89a7a1 100644 --- a/packages/telemetry/src/PostHogTelemetryClient.ts +++ b/packages/telemetry/src/PostHogTelemetryClient.ts @@ -65,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, }) } @@ -128,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) } /** diff --git a/packages/types/src/__tests__/telemetry.test.ts b/packages/types/src/__tests__/telemetry.test.ts index d5130a35aaa..29e6207794b 100644 --- a/packages/types/src/__tests__/telemetry.test.ts +++ b/packages/types/src/__tests__/telemetry.test.ts @@ -405,6 +405,27 @@ describe("telemetry error utilities", () => { 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) @@ -432,6 +453,12 @@ describe("telemetry error utilities", () => { 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", () => { @@ -492,6 +519,36 @@ describe("telemetry error utilities", () => { }) }) + 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) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index 7b5929cca17..f8127e69888 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -495,6 +495,8 @@ export class ConsecutiveMistakeError extends Error { 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" @@ -525,5 +527,7 @@ export function extractConsecutiveMistakeErrorProperties(error: ConsecutiveMista consecutiveMistakeCount: error.consecutiveMistakeCount, consecutiveMistakeLimit: error.consecutiveMistakeLimit, reason: error.reason, + ...(error.provider !== undefined && { provider: error.provider }), + ...(error.modelId !== undefined && { modelId: error.modelId }), } } diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 4066cba6301..b6fa657df83 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -776,19 +776,22 @@ export async function presentAssistantMessage(cline: Task) { // Add user feedback to chat. await cline.say("user_feedback", text, images) - - // Track tool repetition in telemetry via PostHog exception tracking. - TelemetryService.instance.captureException( - new ConsecutiveMistakeError( - `Tool repetition limit reached for ${block.name}`, - cline.taskId, - cline.consecutiveMistakeCount, - cline.consecutiveMistakeLimit, - "tool_repetition", - ), - ) } + // 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( diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 4c184bdf673..144482c26c9 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2222,6 +2222,22 @@ export class Task extends EventEmitter 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"), @@ -2236,19 +2252,6 @@ export class Task extends EventEmitter implements TaskLike { ) await this.say("user_feedback", text, images) - - // Track consecutive mistake errors in telemetry via 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.captureException( - new ConsecutiveMistakeError( - `Task reached consecutive mistake limit (${this.consecutiveMistakeLimit})`, - this.taskId, - this.consecutiveMistakeCount, - this.consecutiveMistakeLimit, - "no_tools_used", - ), - ) } this.consecutiveMistakeCount = 0