Skip to content

Commit

Permalink
Merge branch 'main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
neubig authored Feb 8, 2025
2 parents 8782e3a + 1c72676 commit d8ad8ba
Show file tree
Hide file tree
Showing 40 changed files with 1,323 additions and 204 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ghcr-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ jobs:
run: pipx install poetry
- name: Install Python dependencies using Poetry
run: make install-python-dependencies
- name: Run runtime tests
- name: Run docker runtime tests
run: |
# We install pytest-xdist in order to run tests across CPUs
poetry run pip install pytest-xdist
Expand Down
1 change: 0 additions & 1 deletion build.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#!/bin/bash
set -e

cp pyproject.toml poetry.lock openhands
poetry build -v
1 change: 1 addition & 0 deletions evaluation/benchmarks/swe_bench/resource/mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__))
Expand Down
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 { 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,
}));
});
});
});
165 changes: 165 additions & 0 deletions frontend/__tests__/utils/error-handler.test.ts
Original file line number Diff line number Diff line change
@@ -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,
});
});
});
});
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
13 changes: 12 additions & 1 deletion frontend/src/hooks/query/use-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import posthog from "posthog-js";
import { DEFAULT_SETTINGS } from "#/services/settings";
import OpenHands from "#/api/open-hands";
import { useAuth } from "#/context/auth-context";
import { useConfig } from "#/hooks/query/use-config";

const getSettingsQueryFn = async () => {
const apiSettings = await OpenHands.getSettings();
Expand All @@ -24,14 +25,16 @@ const getSettingsQueryFn = async () => {
};

export const useSettings = () => {
const { setGitHubTokenIsSet } = useAuth();
const { setGitHubTokenIsSet, githubTokenIsSet } = useAuth();
const { data: config } = useConfig();

const query = useQuery({
queryKey: ["settings"],
queryFn: getSettingsQueryFn,
initialData: DEFAULT_SETTINGS,
staleTime: 0,
retry: false,
enabled: config?.APP_MODE !== "saas" || githubTokenIsSet,
meta: {
disableToast: true,
},
Expand All @@ -47,5 +50,13 @@ export const useSettings = () => {
setGitHubTokenIsSet(!!query.data?.GITHUB_TOKEN_IS_SET);
}, [query.data?.GITHUB_TOKEN_IS_SET, query.isFetched]);

// Return default settings if in SAAS mode and not authenticated
if (config?.APP_MODE === "saas" && !githubTokenIsSet) {
return {
...query,
data: DEFAULT_SETTINGS,
};
}

return query;
};
Loading

0 comments on commit d8ad8ba

Please sign in to comment.