Skip to content

Commit e930cd0

Browse files
neubigopenhands-agentraymyers
authored
Better error logging in posthog (#6346)
Co-authored-by: openhands <[email protected]> Co-authored-by: Ray Myers <[email protected]>
1 parent 6655ec0 commit e930cd0

File tree

7 files changed

+300
-20
lines changed

7 files changed

+300
-20
lines changed

‎evaluation/benchmarks/swe_bench/resource/mapping.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import json
99
import os
10+
1011
from openhands.core.logger import openhands_logger as logger
1112

1213
CUR_DIR = os.path.dirname(os.path.abspath(__file__))

‎frontend/__tests__/services/actions.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest";
2+
import { handleStatusMessage } from "#/services/actions";
3+
import store from "#/store";
4+
import { trackError } from "#/utils/error-handler";
5+
6+
// Mock dependencies
7+
vi.mock("#/utils/error-handler", () => ({
8+
trackError: vi.fn(),
9+
}));
10+
11+
vi.mock("#/store", () => ({
12+
default: {
13+
dispatch: vi.fn(),
14+
},
15+
}));
16+
17+
describe("Actions Service", () => {
18+
beforeEach(() => {
19+
vi.clearAllMocks();
20+
});
21+
22+
describe("handleStatusMessage", () => {
23+
it("should dispatch info messages to status state", () => {
24+
const message = {
25+
type: "info",
26+
message: "Runtime is not available",
27+
id: "runtime.unavailable",
28+
status_update: true as const,
29+
};
30+
31+
handleStatusMessage(message);
32+
33+
expect(store.dispatch).toHaveBeenCalledWith(expect.objectContaining({
34+
payload: message,
35+
}));
36+
});
37+
38+
it("should log error messages and display them in chat", () => {
39+
const message = {
40+
type: "error",
41+
message: "Runtime connection failed",
42+
id: "runtime.connection.failed",
43+
status_update: true as const,
44+
};
45+
46+
handleStatusMessage(message);
47+
48+
expect(trackError).toHaveBeenCalledWith({
49+
message: "Runtime connection failed",
50+
source: "chat",
51+
metadata: { msgId: "runtime.connection.failed" },
52+
});
53+
54+
expect(store.dispatch).toHaveBeenCalledWith(expect.objectContaining({
55+
payload: message,
56+
}));
57+
});
58+
});
59+
});

‎frontend/__tests__/utils/error-handler.test.ts

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
2+
import { trackError, showErrorToast, showChatError } from "#/utils/error-handler";
3+
import posthog from "posthog-js";
4+
import toast from "react-hot-toast";
5+
import * as Actions from "#/services/actions";
6+
7+
vi.mock("posthog-js", () => ({
8+
default: {
9+
captureException: vi.fn(),
10+
},
11+
}));
12+
13+
vi.mock("react-hot-toast", () => ({
14+
default: {
15+
error: vi.fn(),
16+
},
17+
}));
18+
19+
vi.mock("#/services/actions", () => ({
20+
handleStatusMessage: vi.fn(),
21+
}));
22+
23+
describe("Error Handler", () => {
24+
beforeEach(() => {
25+
vi.clearAllMocks();
26+
});
27+
28+
afterEach(() => {
29+
vi.clearAllMocks();
30+
});
31+
32+
describe("trackError", () => {
33+
it("should send error to PostHog with basic info", () => {
34+
const error = {
35+
message: "Test error",
36+
source: "test",
37+
};
38+
39+
trackError(error);
40+
41+
expect(posthog.captureException).toHaveBeenCalledWith(new Error("Test error"), {
42+
error_source: "test",
43+
});
44+
});
45+
46+
it("should include additional metadata in PostHog event", () => {
47+
const error = {
48+
message: "Test error",
49+
source: "test",
50+
metadata: {
51+
extra: "info",
52+
details: { foo: "bar" },
53+
},
54+
};
55+
56+
trackError(error);
57+
58+
expect(posthog.captureException).toHaveBeenCalledWith(new Error("Test error"), {
59+
error_source: "test",
60+
extra: "info",
61+
details: { foo: "bar" },
62+
});
63+
});
64+
});
65+
66+
describe("showErrorToast", () => {
67+
it("should log error and show toast", () => {
68+
const error = {
69+
message: "Toast error",
70+
source: "toast-test",
71+
};
72+
73+
showErrorToast(error);
74+
75+
// Verify PostHog logging
76+
expect(posthog.captureException).toHaveBeenCalledWith(new Error("Toast error"), {
77+
error_source: "toast-test",
78+
});
79+
80+
// Verify toast was shown
81+
expect(toast.error).toHaveBeenCalled();
82+
});
83+
84+
it("should include metadata in PostHog event when showing toast", () => {
85+
const error = {
86+
message: "Toast error",
87+
source: "toast-test",
88+
metadata: { context: "testing" },
89+
};
90+
91+
showErrorToast(error);
92+
93+
expect(posthog.captureException).toHaveBeenCalledWith(new Error("Toast error"), {
94+
error_source: "toast-test",
95+
context: "testing",
96+
});
97+
});
98+
99+
it("should log errors from different sources with appropriate metadata", () => {
100+
// Test agent status error
101+
showErrorToast({
102+
message: "Agent error",
103+
source: "agent-status",
104+
metadata: { id: "error.agent" },
105+
});
106+
107+
expect(posthog.captureException).toHaveBeenCalledWith(new Error("Agent error"), {
108+
error_source: "agent-status",
109+
id: "error.agent",
110+
});
111+
112+
showErrorToast({
113+
message: "Server error",
114+
source: "server",
115+
metadata: { error_code: 500, details: "Internal error" },
116+
});
117+
118+
expect(posthog.captureException).toHaveBeenCalledWith(new Error("Server error"), {
119+
error_source: "server",
120+
error_code: 500,
121+
details: "Internal error",
122+
});
123+
});
124+
125+
it("should log feedback submission errors with conversation context", () => {
126+
const error = new Error("Feedback submission failed");
127+
showErrorToast({
128+
message: error.message,
129+
source: "feedback",
130+
metadata: { conversationId: "123", error },
131+
});
132+
133+
expect(posthog.captureException).toHaveBeenCalledWith(new Error("Feedback submission failed"), {
134+
error_source: "feedback",
135+
conversationId: "123",
136+
error,
137+
});
138+
});
139+
});
140+
141+
describe("showChatError", () => {
142+
it("should log error and show chat error message", () => {
143+
const error = {
144+
message: "Chat error",
145+
source: "chat-test",
146+
msgId: "123",
147+
};
148+
149+
showChatError(error);
150+
151+
// Verify PostHog logging
152+
expect(posthog.captureException).toHaveBeenCalledWith(new Error("Chat error"), {
153+
error_source: "chat-test",
154+
});
155+
156+
// Verify error message was shown in chat
157+
expect(Actions.handleStatusMessage).toHaveBeenCalledWith({
158+
type: "error",
159+
message: "Chat error",
160+
id: "123",
161+
status_update: true,
162+
});
163+
});
164+
});
165+
});

‎frontend/src/components/features/controls/agent-status-bar.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from "react";
22
import { useTranslation } from "react-i18next";
33
import { useSelector } from "react-redux";
4-
import toast from "react-hot-toast";
4+
import { showErrorToast } from "#/utils/error-handler";
55
import { RootState } from "#/store";
66
import { AgentState } from "#/types/agent-state";
77
import { AGENT_STATUS_MAP } from "../../agent-status-map.constant";
@@ -27,7 +27,11 @@ export function AgentStatusBar() {
2727
}
2828
}
2929
if (curStatusMessage?.type === "error") {
30-
toast.error(message);
30+
showErrorToast({
31+
message,
32+
source: "agent-status",
33+
metadata: { ...curStatusMessage },
34+
});
3135
return;
3236
}
3337
if (curAgentState === AgentState.LOADING && message.trim()) {

‎frontend/src/context/ws-client-provider.tsx

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
1-
import posthog from "posthog-js";
21
import React from "react";
32
import { io, Socket } from "socket.io-client";
43
import EventLogger from "#/utils/event-logger";
5-
import {
6-
handleAssistantMessage,
7-
handleStatusMessage,
8-
} from "#/services/actions";
4+
import { handleAssistantMessage } from "#/services/actions";
5+
import { showChatError } from "#/utils/error-handler";
96
import { useRate } from "#/hooks/use-rate";
107
import { OpenHandsParsedEvent } from "#/types/core";
118
import {
@@ -85,19 +82,20 @@ export function updateStatusWhenErrorMessagePresent(data: ErrorArg | unknown) {
8582
return;
8683
}
8784
let msgId: string | undefined;
88-
if (
89-
"data" in data &&
90-
isObject(data.data) &&
91-
"msg_id" in data.data &&
92-
isString(data.data.msg_id)
93-
) {
94-
msgId = data.data.msg_id;
85+
let metadata: Record<string, unknown> = {};
86+
87+
if ("data" in data && isObject(data.data)) {
88+
if ("msg_id" in data.data && isString(data.data.msg_id)) {
89+
msgId = data.data.msg_id;
90+
}
91+
metadata = data.data as Record<string, unknown>;
9592
}
96-
handleStatusMessage({
97-
type: "error",
93+
94+
showChatError({
9895
message: data.message,
99-
id: msgId,
100-
status_update: true,
96+
source: "websocket",
97+
metadata,
98+
msgId,
10199
});
102100
}
103101
}
@@ -153,7 +151,6 @@ export function WsClientProvider({
153151
function handleError(data: unknown) {
154152
setStatus(WsClientProviderStatus.DISCONNECTED);
155153
updateStatusWhenErrorMessagePresent(data);
156-
posthog.capture("socket_error");
157154
}
158155

159156
React.useEffect(() => {

‎frontend/src/services/actions.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
addUserMessage,
55
addErrorMessage,
66
} from "#/state/chat-slice";
7+
import { trackError } from "#/utils/error-handler";
78
import { appendSecurityAnalyzerInput } from "#/state/security-analyzer-slice";
89
import { setCode, setActiveFilepath } from "#/state/code-slice";
910
import { appendJupyterInput } from "#/state/jupyter-slice";
@@ -95,6 +96,11 @@ export function handleStatusMessage(message: StatusMessage) {
9596
}),
9697
);
9798
} else if (message.type === "error") {
99+
trackError({
100+
message: message.message,
101+
source: "chat",
102+
metadata: { msgId: message.id },
103+
});
98104
store.dispatch(
99105
addErrorMessage({
100106
...message,
@@ -111,9 +117,15 @@ export function handleAssistantMessage(message: Record<string, unknown>) {
111117
} else if (message.status_update) {
112118
handleStatusMessage(message as unknown as StatusMessage);
113119
} else {
120+
const errorMsg = "Unknown message type received";
121+
trackError({
122+
message: errorMsg,
123+
source: "chat",
124+
metadata: { raw_message: message },
125+
});
114126
store.dispatch(
115127
addErrorMessage({
116-
message: "Unknown message type received",
128+
message: errorMsg,
117129
}),
118130
);
119131
}

‎frontend/src/utils/error-handler.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import posthog from "posthog-js";
2+
import toast from "react-hot-toast";
3+
import { handleStatusMessage } from "#/services/actions";
4+
5+
interface ErrorDetails {
6+
message: string;
7+
source?: string;
8+
metadata?: Record<string, unknown>;
9+
msgId?: string;
10+
}
11+
12+
export function trackError({ message, source, metadata = {} }: ErrorDetails) {
13+
const error = new Error(message);
14+
posthog.captureException(error, {
15+
error_source: source || "unknown",
16+
...metadata,
17+
});
18+
}
19+
20+
export function showErrorToast({
21+
message,
22+
source,
23+
metadata = {},
24+
}: ErrorDetails) {
25+
trackError({ message, source, metadata });
26+
toast.error(message);
27+
}
28+
29+
export function showChatError({
30+
message,
31+
source,
32+
metadata = {},
33+
msgId,
34+
}: ErrorDetails) {
35+
trackError({ message, source, metadata });
36+
handleStatusMessage({
37+
type: "error",
38+
message,
39+
id: msgId,
40+
status_update: true,
41+
});
42+
}

0 commit comments

Comments
 (0)