mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 16:19:05 +08:00
fix: route LLM errors inline and keep conversation/server errors visible in the banner (#787)
This commit is contained in:
@@ -0,0 +1,48 @@
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
import { useErrorMessageStore } from "#/stores/error-message-store";
|
||||
|
||||
const getState = () => useErrorMessageStore.getState();
|
||||
|
||||
describe("error message store", () => {
|
||||
beforeEach(() => {
|
||||
useErrorMessageStore.setState({ errorMessage: null, errorType: null });
|
||||
});
|
||||
|
||||
it("defaults to a sticky conversation error", () => {
|
||||
getState().setErrorMessage("boom");
|
||||
expect(getState().errorMessage).toBe("boom");
|
||||
expect(getState().errorType).toBe("conversation");
|
||||
});
|
||||
|
||||
it("tags connection errors when the type is provided", () => {
|
||||
getState().setErrorMessage("offline", "connection");
|
||||
expect(getState().errorType).toBe("connection");
|
||||
});
|
||||
|
||||
it("removeErrorMessage clears any error regardless of type", () => {
|
||||
getState().setErrorMessage("boom");
|
||||
getState().removeErrorMessage();
|
||||
expect(getState().errorMessage).toBeNull();
|
||||
expect(getState().errorType).toBeNull();
|
||||
});
|
||||
|
||||
it("clearConnectionError clears a transient connection error", () => {
|
||||
getState().setErrorMessage("offline", "connection");
|
||||
getState().clearConnectionError();
|
||||
expect(getState().errorMessage).toBeNull();
|
||||
expect(getState().errorType).toBeNull();
|
||||
});
|
||||
|
||||
it("clearConnectionError preserves a sticky conversation error", () => {
|
||||
getState().setErrorMessage("bad api key");
|
||||
getState().clearConnectionError();
|
||||
expect(getState().errorMessage).toBe("bad api key");
|
||||
expect(getState().errorType).toBe("conversation");
|
||||
});
|
||||
|
||||
it("clearConnectionError is a no-op when there is no error", () => {
|
||||
getState().clearConnectionError();
|
||||
expect(getState().errorMessage).toBeNull();
|
||||
expect(getState().errorType).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -133,7 +133,8 @@ export function ConversationWebSocketProvider({
|
||||
const queryClient = useQueryClient();
|
||||
const addEvent = useEventStore((state) => state.addEvent);
|
||||
const addEvents = useEventStore((state) => state.addEvents);
|
||||
const { setErrorMessage, removeErrorMessage } = useErrorMessageStore();
|
||||
const { setErrorMessage, removeErrorMessage, clearConnectionError } =
|
||||
useErrorMessageStore();
|
||||
const consumeMatchingPendingMessage = useOptimisticUserMessageStore(
|
||||
(state) => state.consumeMatchingPendingMessage,
|
||||
);
|
||||
@@ -169,8 +170,10 @@ export function ConversationWebSocketProvider({
|
||||
path?.toUpperCase().endsWith("PLAN.MD") ?? false;
|
||||
|
||||
const handleNonErrorEvent = useCallback(() => {
|
||||
removeErrorMessage();
|
||||
}, [removeErrorMessage]);
|
||||
// A normal event means connectivity recovered: clear a transient connection
|
||||
// error, but keep sticky conversation errors (e.g. a wrong API key).
|
||||
clearConnectionError();
|
||||
}, [clearConnectionError]);
|
||||
|
||||
// Helper function to update metrics from stats event
|
||||
const updateMetricsFromStats = useCallback(
|
||||
@@ -426,7 +429,8 @@ export function ConversationWebSocketProvider({
|
||||
handleNonErrorEvent();
|
||||
}
|
||||
|
||||
// Track credit limit reached if AgentErrorEvent has budget-related error
|
||||
// LLM errors render inline in the chat (see ErrorEventMessage); track
|
||||
// them for analytics but keep them out of the banner above the chat box.
|
||||
if (isAgentErrorEvent(event)) {
|
||||
trackError({
|
||||
message: event.error,
|
||||
@@ -438,7 +442,6 @@ export function ConversationWebSocketProvider({
|
||||
},
|
||||
posthog,
|
||||
});
|
||||
setErrorMessage(event.error);
|
||||
}
|
||||
|
||||
// Clear optimistic user message when a user message is confirmed.
|
||||
@@ -608,7 +611,8 @@ export function ConversationWebSocketProvider({
|
||||
handleNonErrorEvent();
|
||||
}
|
||||
|
||||
// Handle AgentErrorEvent specifically
|
||||
// LLM errors render inline in the chat (see ErrorEventMessage); track
|
||||
// them for analytics but keep them out of the banner above the chat box.
|
||||
if (isAgentErrorEvent(event)) {
|
||||
trackError({
|
||||
message: event.error,
|
||||
@@ -620,7 +624,6 @@ export function ConversationWebSocketProvider({
|
||||
},
|
||||
posthog,
|
||||
});
|
||||
setErrorMessage(event.error);
|
||||
}
|
||||
|
||||
// Clear optimistic user message when a user message is confirmed.
|
||||
@@ -761,7 +764,7 @@ export function ConversationWebSocketProvider({
|
||||
onOpen: () => {
|
||||
setMainConnectionState("OPEN");
|
||||
hasConnectedRefMain.current = true; // Mark that we've successfully connected
|
||||
removeErrorMessage(); // Clear any previous error messages on successful connection
|
||||
clearConnectionError(); // Clear a previous connection error; keep sticky conversation errors
|
||||
},
|
||||
onClose: () => {
|
||||
setMainConnectionState("CLOSED");
|
||||
@@ -770,7 +773,7 @@ export function ConversationWebSocketProvider({
|
||||
setMainConnectionState("CLOSED");
|
||||
// Only show error message if we've previously connected successfully
|
||||
if (hasConnectedRefMain.current) {
|
||||
setErrorMessage(SERVER_CONNECTION_ERROR_MESSAGE);
|
||||
setErrorMessage(SERVER_CONNECTION_ERROR_MESSAGE, "connection");
|
||||
}
|
||||
},
|
||||
onMessage: handleMainMessage,
|
||||
@@ -778,7 +781,7 @@ export function ConversationWebSocketProvider({
|
||||
}, [
|
||||
handleMainMessage,
|
||||
setErrorMessage,
|
||||
removeErrorMessage,
|
||||
clearConnectionError,
|
||||
sessionApiKey,
|
||||
initialAfterTimestamp,
|
||||
]);
|
||||
@@ -802,7 +805,7 @@ export function ConversationWebSocketProvider({
|
||||
onOpen: async () => {
|
||||
setPlanningConnectionState("OPEN");
|
||||
hasConnectedRefPlanning.current = true; // Mark that we've successfully connected
|
||||
removeErrorMessage(); // Clear any previous error messages on successful connection
|
||||
clearConnectionError(); // Clear a previous connection error; keep sticky conversation errors
|
||||
|
||||
// Fetch expected event count for history loading detection
|
||||
if (
|
||||
@@ -834,7 +837,7 @@ export function ConversationWebSocketProvider({
|
||||
setPlanningConnectionState("CLOSED");
|
||||
// Only show error message if we've previously connected successfully
|
||||
if (hasConnectedRefPlanning.current) {
|
||||
setErrorMessage(SERVER_CONNECTION_ERROR_MESSAGE);
|
||||
setErrorMessage(SERVER_CONNECTION_ERROR_MESSAGE, "connection");
|
||||
}
|
||||
},
|
||||
onMessage: handlePlanningMessage,
|
||||
@@ -842,7 +845,7 @@ export function ConversationWebSocketProvider({
|
||||
}, [
|
||||
handlePlanningMessage,
|
||||
setErrorMessage,
|
||||
removeErrorMessage,
|
||||
clearConnectionError,
|
||||
sessionApiKey,
|
||||
subConversations,
|
||||
]);
|
||||
|
||||
@@ -1,30 +1,50 @@
|
||||
import { create } from "zustand";
|
||||
|
||||
/**
|
||||
* "connection" errors auto-clear once connectivity recovers; "conversation"
|
||||
* errors (e.g. a wrong API key) are sticky and clear only on an explicit user
|
||||
* action (dismiss, retry, new message).
|
||||
*/
|
||||
export type ErrorMessageType = "connection" | "conversation";
|
||||
|
||||
interface ErrorMessageState {
|
||||
errorMessage: string | null;
|
||||
errorType: ErrorMessageType | null;
|
||||
}
|
||||
|
||||
interface ErrorMessageActions {
|
||||
setErrorMessage: (message: string) => void;
|
||||
setErrorMessage: (message: string, type?: ErrorMessageType) => void;
|
||||
removeErrorMessage: () => void;
|
||||
/** Clears the error only when it is a transient connection error. */
|
||||
clearConnectionError: () => void;
|
||||
}
|
||||
|
||||
type ErrorMessageStore = ErrorMessageState & ErrorMessageActions;
|
||||
|
||||
const initialState: ErrorMessageState = {
|
||||
errorMessage: null,
|
||||
errorType: null,
|
||||
};
|
||||
|
||||
export const useErrorMessageStore = create<ErrorMessageStore>((set) => ({
|
||||
...initialState,
|
||||
|
||||
setErrorMessage: (message: string) =>
|
||||
setErrorMessage: (message: string, type: ErrorMessageType = "conversation") =>
|
||||
set(() => ({
|
||||
errorMessage: message,
|
||||
errorType: type,
|
||||
})),
|
||||
|
||||
removeErrorMessage: () =>
|
||||
set(() => ({
|
||||
errorMessage: null,
|
||||
errorType: null,
|
||||
})),
|
||||
|
||||
clearConnectionError: () =>
|
||||
set((state) =>
|
||||
state.errorType === "connection"
|
||||
? { errorMessage: null, errorType: null }
|
||||
: state,
|
||||
),
|
||||
}));
|
||||
|
||||
Reference in New Issue
Block a user