diff --git a/evaluation/benchmarks/swe_bench/resource/mapping.py b/evaluation/benchmarks/swe_bench/resource/mapping.py index ed2f433c262b..cf2323270d03 100644 --- a/evaluation/benchmarks/swe_bench/resource/mapping.py +++ b/evaluation/benchmarks/swe_bench/resource/mapping.py @@ -7,6 +7,7 @@ import json import os + from openhands.core.logger import openhands_logger as logger CUR_DIR = os.path.dirname(os.path.abspath(__file__)) diff --git a/frontend/__tests__/services/actions.test.ts b/frontend/__tests__/services/actions.test.ts new file mode 100644 index 000000000000..511473b1d613 --- /dev/null +++ b/frontend/__tests__/services/actions.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { handleStatusMessage } from "#/services/actions"; +import store from "#/store"; +import { trackError } from "#/utils/error-handler"; + +// Mock dependencies +vi.mock("#/utils/error-handler", () => ({ + trackError: vi.fn(), +})); + +vi.mock("#/store", () => ({ + default: { + dispatch: vi.fn(), + }, +})); + +describe("Actions Service", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe("handleStatusMessage", () => { + it("should dispatch info messages to status state", () => { + const message = { + type: "info", + message: "Runtime is not available", + id: "runtime.unavailable", + status_update: true as const, + }; + + handleStatusMessage(message); + + expect(store.dispatch).toHaveBeenCalledWith(expect.objectContaining({ + payload: message, + })); + }); + + it("should log error messages and display them in chat", () => { + const message = { + type: "error", + message: "Runtime connection failed", + id: "runtime.connection.failed", + status_update: true as const, + }; + + handleStatusMessage(message); + + expect(trackError).toHaveBeenCalledWith({ + message: "Runtime connection failed", + source: "chat", + metadata: { msgId: "runtime.connection.failed" }, + }); + + expect(store.dispatch).toHaveBeenCalledWith(expect.objectContaining({ + payload: message, + })); + }); + }); +}); diff --git a/frontend/__tests__/utils/error-handler.test.ts b/frontend/__tests__/utils/error-handler.test.ts new file mode 100644 index 000000000000..0d87913ffa09 --- /dev/null +++ b/frontend/__tests__/utils/error-handler.test.ts @@ -0,0 +1,165 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { trackError, showErrorToast, showChatError } from "#/utils/error-handler"; +import posthog from "posthog-js"; +import toast from "react-hot-toast"; +import * as Actions from "#/services/actions"; + +vi.mock("posthog-js", () => ({ + default: { + captureException: vi.fn(), + }, +})); + +vi.mock("react-hot-toast", () => ({ + default: { + error: vi.fn(), + }, +})); + +vi.mock("#/services/actions", () => ({ + handleStatusMessage: vi.fn(), +})); + +describe("Error Handler", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + describe("trackError", () => { + it("should send error to PostHog with basic info", () => { + const error = { + message: "Test error", + source: "test", + }; + + trackError(error); + + expect(posthog.captureException).toHaveBeenCalledWith(new Error("Test error"), { + error_source: "test", + }); + }); + + it("should include additional metadata in PostHog event", () => { + const error = { + message: "Test error", + source: "test", + metadata: { + extra: "info", + details: { foo: "bar" }, + }, + }; + + trackError(error); + + expect(posthog.captureException).toHaveBeenCalledWith(new Error("Test error"), { + error_source: "test", + extra: "info", + details: { foo: "bar" }, + }); + }); + }); + + describe("showErrorToast", () => { + it("should log error and show toast", () => { + const error = { + message: "Toast error", + source: "toast-test", + }; + + showErrorToast(error); + + // Verify PostHog logging + expect(posthog.captureException).toHaveBeenCalledWith(new Error("Toast error"), { + error_source: "toast-test", + }); + + // Verify toast was shown + expect(toast.error).toHaveBeenCalled(); + }); + + it("should include metadata in PostHog event when showing toast", () => { + const error = { + message: "Toast error", + source: "toast-test", + metadata: { context: "testing" }, + }; + + showErrorToast(error); + + expect(posthog.captureException).toHaveBeenCalledWith(new Error("Toast error"), { + error_source: "toast-test", + context: "testing", + }); + }); + + it("should log errors from different sources with appropriate metadata", () => { + // Test agent status error + showErrorToast({ + message: "Agent error", + source: "agent-status", + metadata: { id: "error.agent" }, + }); + + expect(posthog.captureException).toHaveBeenCalledWith(new Error("Agent error"), { + error_source: "agent-status", + id: "error.agent", + }); + + showErrorToast({ + message: "Server error", + source: "server", + metadata: { error_code: 500, details: "Internal error" }, + }); + + expect(posthog.captureException).toHaveBeenCalledWith(new Error("Server error"), { + error_source: "server", + error_code: 500, + details: "Internal error", + }); + }); + + it("should log feedback submission errors with conversation context", () => { + const error = new Error("Feedback submission failed"); + showErrorToast({ + message: error.message, + source: "feedback", + metadata: { conversationId: "123", error }, + }); + + expect(posthog.captureException).toHaveBeenCalledWith(new Error("Feedback submission failed"), { + error_source: "feedback", + conversationId: "123", + error, + }); + }); + }); + + describe("showChatError", () => { + it("should log error and show chat error message", () => { + const error = { + message: "Chat error", + source: "chat-test", + msgId: "123", + }; + + showChatError(error); + + // Verify PostHog logging + expect(posthog.captureException).toHaveBeenCalledWith(new Error("Chat error"), { + error_source: "chat-test", + }); + + // Verify error message was shown in chat + expect(Actions.handleStatusMessage).toHaveBeenCalledWith({ + type: "error", + message: "Chat error", + id: "123", + status_update: true, + }); + }); + }); +}); diff --git a/frontend/src/components/features/controls/agent-status-bar.tsx b/frontend/src/components/features/controls/agent-status-bar.tsx index 08efefdd78af..ace99f8f4832 100644 --- a/frontend/src/components/features/controls/agent-status-bar.tsx +++ b/frontend/src/components/features/controls/agent-status-bar.tsx @@ -1,7 +1,7 @@ import React from "react"; import { useTranslation } from "react-i18next"; import { useSelector } from "react-redux"; -import toast from "react-hot-toast"; +import { showErrorToast } from "#/utils/error-handler"; import { RootState } from "#/store"; import { AgentState } from "#/types/agent-state"; import { AGENT_STATUS_MAP } from "../../agent-status-map.constant"; @@ -27,7 +27,11 @@ export function AgentStatusBar() { } } if (curStatusMessage?.type === "error") { - toast.error(message); + showErrorToast({ + message, + source: "agent-status", + metadata: { ...curStatusMessage }, + }); return; } if (curAgentState === AgentState.LOADING && message.trim()) { diff --git a/frontend/src/context/ws-client-provider.tsx b/frontend/src/context/ws-client-provider.tsx index bdfa8b8b0b10..a5610fc50f42 100644 --- a/frontend/src/context/ws-client-provider.tsx +++ b/frontend/src/context/ws-client-provider.tsx @@ -1,11 +1,8 @@ -import posthog from "posthog-js"; import React from "react"; import { io, Socket } from "socket.io-client"; import EventLogger from "#/utils/event-logger"; -import { - handleAssistantMessage, - handleStatusMessage, -} from "#/services/actions"; +import { handleAssistantMessage } from "#/services/actions"; +import { showChatError } from "#/utils/error-handler"; import { useRate } from "#/hooks/use-rate"; import { OpenHandsParsedEvent } from "#/types/core"; import { @@ -85,19 +82,20 @@ export function updateStatusWhenErrorMessagePresent(data: ErrorArg | unknown) { return; } let msgId: string | undefined; - if ( - "data" in data && - isObject(data.data) && - "msg_id" in data.data && - isString(data.data.msg_id) - ) { - msgId = data.data.msg_id; + let metadata: Record = {}; + + if ("data" in data && isObject(data.data)) { + if ("msg_id" in data.data && isString(data.data.msg_id)) { + msgId = data.data.msg_id; + } + metadata = data.data as Record; } - handleStatusMessage({ - type: "error", + + showChatError({ message: data.message, - id: msgId, - status_update: true, + source: "websocket", + metadata, + msgId, }); } } @@ -153,7 +151,6 @@ export function WsClientProvider({ function handleError(data: unknown) { setStatus(WsClientProviderStatus.DISCONNECTED); updateStatusWhenErrorMessagePresent(data); - posthog.capture("socket_error"); } React.useEffect(() => { diff --git a/frontend/src/services/actions.ts b/frontend/src/services/actions.ts index 8061c7242be7..d6702aad088f 100644 --- a/frontend/src/services/actions.ts +++ b/frontend/src/services/actions.ts @@ -4,6 +4,7 @@ import { addUserMessage, addErrorMessage, } from "#/state/chat-slice"; +import { trackError } from "#/utils/error-handler"; import { appendSecurityAnalyzerInput } from "#/state/security-analyzer-slice"; import { setCode, setActiveFilepath } from "#/state/code-slice"; import { appendJupyterInput } from "#/state/jupyter-slice"; @@ -95,6 +96,11 @@ export function handleStatusMessage(message: StatusMessage) { }), ); } else if (message.type === "error") { + trackError({ + message: message.message, + source: "chat", + metadata: { msgId: message.id }, + }); store.dispatch( addErrorMessage({ ...message, @@ -111,9 +117,15 @@ export function handleAssistantMessage(message: Record) { } else if (message.status_update) { handleStatusMessage(message as unknown as StatusMessage); } else { + const errorMsg = "Unknown message type received"; + trackError({ + message: errorMsg, + source: "chat", + metadata: { raw_message: message }, + }); store.dispatch( addErrorMessage({ - message: "Unknown message type received", + message: errorMsg, }), ); } diff --git a/frontend/src/utils/error-handler.ts b/frontend/src/utils/error-handler.ts new file mode 100644 index 000000000000..54f5d65917ed --- /dev/null +++ b/frontend/src/utils/error-handler.ts @@ -0,0 +1,42 @@ +import posthog from "posthog-js"; +import toast from "react-hot-toast"; +import { handleStatusMessage } from "#/services/actions"; + +interface ErrorDetails { + message: string; + source?: string; + metadata?: Record; + msgId?: string; +} + +export function trackError({ message, source, metadata = {} }: ErrorDetails) { + const error = new Error(message); + posthog.captureException(error, { + error_source: source || "unknown", + ...metadata, + }); +} + +export function showErrorToast({ + message, + source, + metadata = {}, +}: ErrorDetails) { + trackError({ message, source, metadata }); + toast.error(message); +} + +export function showChatError({ + message, + source, + metadata = {}, + msgId, +}: ErrorDetails) { + trackError({ message, source, metadata }); + handleStatusMessage({ + type: "error", + message, + id: msgId, + status_update: true, + }); +}