Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error logging in posthog #6346

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions frontend/__tests__/services/actions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { handleStatusMessage } from "#/services/actions";
import store from "#/store";
import { logError } from "#/utils/error-handler";

// Mock dependencies
vi.mock("#/utils/error-handler", () => ({
logError: 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(logError).toHaveBeenCalledWith({
message: "Runtime connection failed",
source: "chat",
metadata: { msgId: "runtime.connection.failed" },
});

expect(store.dispatch).toHaveBeenCalledWith(expect.objectContaining({
payload: message,
}));
});
});
});
238 changes: 238 additions & 0 deletions frontend/__tests__/utils/error-handler.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,238 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { logError, showErrorToast, showChatError } from "#/utils/error-handler";
import posthog from "posthog-js";
import toast from "react-hot-toast";
import * as Actions from "#/services/actions";

// Mock dependencies
vi.mock("posthog-js", () => ({
default: {
capture: vi.fn(),
},
}));

vi.mock("react-hot-toast", () => ({
default: {
custom: vi.fn(),
},
}));

vi.mock("#/services/actions", () => ({
handleStatusMessage: vi.fn(),
}));

describe("Error Handler", () => {
beforeEach(() => {
vi.clearAllMocks();
});

afterEach(() => {
vi.clearAllMocks();
});

describe("logError", () => {
it("should log error to PostHog with basic info", () => {
const error = {
message: "Test error",
source: "test",
};

logError(error);

expect(posthog.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "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" },
},
};

logError(error);

expect(posthog.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "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.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "Toast error",
error_source: "toast-test",
});

// Verify toast was shown
expect(toast.custom).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.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "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.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "Agent error",
error_source: "agent-status",
id: "error.agent",
});

// Test VSCode error
showErrorToast({
message: "VSCode error",
source: "vscode",
metadata: { error: "connection failed" },
});

expect(posthog.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "VSCode error",
error_source: "vscode",
error: "connection failed",
});

// Test server error
showErrorToast({
message: "Server error",
source: "server",
metadata: { error_code: 500, details: "Internal error" },
});

expect(posthog.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "Server error",
error_source: "server",
error_code: 500,
details: "Internal error",
});
});

it("should log query and mutation errors with appropriate metadata", () => {
// Test query error
showErrorToast({
message: "Query failed",
source: "query",
metadata: { queryKey: ["users", "123"] },
});

expect(posthog.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "Query failed",
error_source: "query",
queryKey: ["users", "123"],
});

// Test mutation error
const error = new Error("Mutation failed");
showErrorToast({
message: error.message,
source: "mutation",
metadata: { error },
});

expect(posthog.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "Mutation failed",
error_source: "mutation",
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.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "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.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "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,
});
});

it("should include metadata in PostHog event when showing chat error", () => {
const error = {
message: "Chat error",
source: "chat-test",
msgId: "123",
metadata: {
context: "chat testing",
severity: "high",
},
};

showChatError(error);

expect(posthog.capture).toHaveBeenCalledWith("error_occurred", {
error_message: "Chat error",
error_source: "chat-test",
context: "chat testing",
severity: "high",
});
});
});
});
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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()) {
Expand Down
31 changes: 14 additions & 17 deletions frontend/src/context/ws-client-provider.tsx
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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<string, unknown> = {};

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<string, unknown>;
}
handleStatusMessage({
type: "error",

showChatError({
message: data.message,
id: msgId,
status_update: true,
source: "websocket",
metadata,
msgId,
});
}
}
Expand Down Expand Up @@ -153,7 +151,6 @@ export function WsClientProvider({
function handleError(data: unknown) {
setStatus(WsClientProviderStatus.DISCONNECTED);
updateStatusWhenErrorMessagePresent(data);
posthog.capture("socket_error");
}

React.useEffect(() => {
Expand Down
Loading
Loading