mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
fix(frontend): clear error banner on successful V1 WebSocket events (#12398)
This commit is contained in:
@@ -15,10 +15,11 @@ import { MemoryRouter, Route, Routes } from "react-router";
|
||||
import { useOptimisticUserMessageStore } from "#/stores/optimistic-user-message-store";
|
||||
import { useBrowserStore } from "#/stores/browser-store";
|
||||
import { useCommandStore } from "#/stores/command-store";
|
||||
import { useErrorMessageStore } from "#/stores/error-message-store";
|
||||
import {
|
||||
createMockMessageEvent,
|
||||
createMockUserMessageEvent,
|
||||
createMockAgentErrorEvent,
|
||||
createMockConversationErrorEvent,
|
||||
createMockBrowserObservationEvent,
|
||||
createMockBrowserNavigateActionEvent,
|
||||
createMockExecuteBashActionEvent,
|
||||
@@ -51,6 +52,9 @@ afterEach(() => {
|
||||
mswServer.resetHandlers();
|
||||
// Clean up any React components
|
||||
cleanup();
|
||||
// Reset stores to prevent state leakage between tests
|
||||
useErrorMessageStore.getState().removeErrorMessage();
|
||||
useEventStore.getState().clearEvents();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
@@ -277,16 +281,23 @@ describe("Conversation WebSocket Handler", () => {
|
||||
|
||||
// 5. Error Handling Tests
|
||||
describe("Error Handling & Recovery", () => {
|
||||
it("should update error message store on AgentErrorEvent", async () => {
|
||||
// Create a mock AgentErrorEvent to send through WebSocket
|
||||
const mockAgentErrorEvent = createMockAgentErrorEvent();
|
||||
beforeEach(() => {
|
||||
// Clear stores before each error handling test to prevent state leakage
|
||||
useErrorMessageStore.getState().removeErrorMessage();
|
||||
useEventStore.getState().clearEvents();
|
||||
});
|
||||
|
||||
it("should update error message store on ConversationErrorEvent", async () => {
|
||||
// ConversationErrorEvent represents infrastructure/authentication errors
|
||||
// that should be shown as a banner to the user.
|
||||
const mockConversationErrorEvent = createMockConversationErrorEvent();
|
||||
|
||||
// Set up MSW to send the error event when connection is established
|
||||
mswServer.use(
|
||||
wsLink.addEventListener("connection", ({ client, server }) => {
|
||||
server.connect();
|
||||
// Send the mock error event after connection
|
||||
client.send(JSON.stringify(mockAgentErrorEvent));
|
||||
client.send(JSON.stringify(mockConversationErrorEvent));
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -299,7 +310,7 @@ describe("Conversation WebSocket Handler", () => {
|
||||
// Wait for connection and error event processing
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("error-message")).toHaveTextContent(
|
||||
"Failed to execute command: Permission denied",
|
||||
"Your session has expired. Please log in again.",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -438,6 +449,60 @@ describe("Conversation WebSocket Handler", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("should clear error message when a successful event is received after a ConversationErrorEvent", async () => {
|
||||
// This test verifies that error banners disappear when follow-up messages
|
||||
// are sent and received. Only ConversationErrorEvent sets the error banner,
|
||||
// and any non-error event should clear it.
|
||||
const conversationId = "test-conversation-error-clear";
|
||||
|
||||
// Set up MSW to mock event count API and send events
|
||||
mswServer.use(
|
||||
http.get(
|
||||
`http://localhost:3000/api/conversations/${conversationId}/events/count`,
|
||||
() => HttpResponse.json(2),
|
||||
),
|
||||
wsLink.addEventListener("connection", ({ client, server }) => {
|
||||
server.connect();
|
||||
|
||||
// Send a ConversationErrorEvent first (this sets the error banner)
|
||||
const mockConversationErrorEvent = createMockConversationErrorEvent();
|
||||
client.send(JSON.stringify(mockConversationErrorEvent));
|
||||
|
||||
// Send a successful (non-error) event immediately after
|
||||
// This simulates the user sending a follow-up message and receiving a response
|
||||
const mockSuccessEvent = createMockMessageEvent({
|
||||
id: "success-event-after-error",
|
||||
});
|
||||
client.send(JSON.stringify(mockSuccessEvent));
|
||||
}),
|
||||
);
|
||||
|
||||
// Verify error message store is initially empty
|
||||
expect(useErrorMessageStore.getState().errorMessage).toBeNull();
|
||||
|
||||
// Render with WebSocket context (minimal component just to trigger connection)
|
||||
renderWithWebSocketContext(
|
||||
<ConnectionStatusComponent />,
|
||||
conversationId,
|
||||
`http://localhost:3000/api/conversations/${conversationId}`,
|
||||
);
|
||||
|
||||
// Wait for connection
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("connection-state")).toHaveTextContent(
|
||||
"OPEN",
|
||||
);
|
||||
});
|
||||
|
||||
// Wait for both events to be received and error to be cleared
|
||||
// The error was set by the first event (ConversationErrorEvent),
|
||||
// then cleared by the second successful event (MessageEvent).
|
||||
await waitFor(() => {
|
||||
expect(useEventStore.getState().events.length).toBe(2);
|
||||
expect(useErrorMessageStore.getState().errorMessage).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it("should not create duplicate events when WebSocket reconnects with resend_all=true", async () => {
|
||||
const conversationId = "test-conversation-reconnect";
|
||||
let connectionCount = 0;
|
||||
|
||||
@@ -329,16 +329,17 @@ export function ConversationWebSocketProvider({
|
||||
if (isV1Event(event)) {
|
||||
addEvent(event);
|
||||
|
||||
// Handle ConversationErrorEvent specifically
|
||||
// Handle ConversationErrorEvent specifically - show error banner
|
||||
// AgentErrorEvent errors are displayed inline in the chat, not as banners
|
||||
if (isConversationErrorEvent(event)) {
|
||||
setErrorMessage(event.detail);
|
||||
} else {
|
||||
// Clear error message on any non-ConversationErrorEvent
|
||||
removeErrorMessage();
|
||||
}
|
||||
|
||||
// Handle AgentErrorEvent specifically
|
||||
// Track credit limit reached if AgentErrorEvent has budget-related error
|
||||
if (isAgentErrorEvent(event)) {
|
||||
setErrorMessage(event.error);
|
||||
|
||||
// Track credit limit reached if the error is budget-related
|
||||
if (isBudgetOrCreditError(event.error)) {
|
||||
trackCreditLimitReached({
|
||||
conversationId: conversationId || "unknown",
|
||||
@@ -417,6 +418,7 @@ export function ConversationWebSocketProvider({
|
||||
isLoadingHistoryMain,
|
||||
expectedEventCountMain,
|
||||
setErrorMessage,
|
||||
removeErrorMessage,
|
||||
removeOptimisticUserMessage,
|
||||
queryClient,
|
||||
conversationId,
|
||||
@@ -424,6 +426,7 @@ export function ConversationWebSocketProvider({
|
||||
appendInput,
|
||||
appendOutput,
|
||||
updateMetricsFromStats,
|
||||
trackCreditLimitReached,
|
||||
],
|
||||
);
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
import { AgentStateChangeObservation } from "#/types/core/observations";
|
||||
import { MessageEvent } from "#/types/v1/core";
|
||||
import { AgentErrorEvent } from "#/types/v1/core/events/observation-event";
|
||||
import { ConversationErrorEvent } from "#/types/v1/core/events/conversation-state-event";
|
||||
import { MockSessionMessaage } from "./session-history.mock";
|
||||
|
||||
export const generateAgentStateChangeObservation = (
|
||||
@@ -236,3 +237,19 @@ export const createMockBrowserNavigateActionEvent = (
|
||||
llm_response_id: "llm-response-789",
|
||||
security_risk: { level: "low" },
|
||||
});
|
||||
|
||||
/**
|
||||
* Creates a mock ConversationErrorEvent for testing conversation-level error handling
|
||||
* These are infrastructure/authentication errors that should show error banners
|
||||
*/
|
||||
export const createMockConversationErrorEvent = (
|
||||
overrides: Partial<ConversationErrorEvent> = {},
|
||||
): ConversationErrorEvent => ({
|
||||
id: "conversation-error-123",
|
||||
timestamp: new Date().toISOString(),
|
||||
source: "environment",
|
||||
kind: "ConversationErrorEvent",
|
||||
code: "AuthenticationError",
|
||||
detail: "Your session has expired. Please log in again.",
|
||||
...overrides,
|
||||
});
|
||||
|
||||
@@ -62,6 +62,11 @@ export interface ConversationState {
|
||||
}
|
||||
|
||||
interface ConversationStateUpdateEventBase extends BaseEvent {
|
||||
/**
|
||||
* Discriminator field for type guards
|
||||
*/
|
||||
kind: "ConversationStateUpdateEvent";
|
||||
|
||||
/**
|
||||
* The source is always "environment" for conversation state update events
|
||||
*/
|
||||
@@ -105,6 +110,11 @@ export type ConversationStateUpdateEvent =
|
||||
|
||||
// Conversation error event - contains error information
|
||||
export interface ConversationErrorEvent extends BaseEvent {
|
||||
/**
|
||||
* Discriminator field for type guards
|
||||
*/
|
||||
kind: "ConversationErrorEvent";
|
||||
|
||||
/**
|
||||
* The source is always "environment" for conversation error events
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user