Add error handling when open chatbot and loading converstaion (#485)

* Add error handling when open chatbot and loading converstaion Signed-off-by: yyfamazon <yyf@amazon.com> * fix tests Signed-off-by: Yulong Ruan <ruanyl@amazon.com> * fix the new conversation entry from top right text input Signed-off-by: yyfamazon <yyf@amazon.com> --------- Signed-off-by: yyfamazon <yyf@amazon.com> Signed-off-by: Yulong Ruan <ruanyl@amazon.com> Co-authored-by: Yulong Ruan <ruanyl@amazon.com>
opensearch-project · Mar 5, 2025 · 63a2f08 · 63a2f08
1 parent a59395e
commit 63a2f08
Show file tree

Hide file tree

Showing 10 changed files with 171 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - add flag to control if display conversation list ([#438](https://github.com/opensearch-project/dashboards-assistant/pull/438))
 - when open chatbot, load the last conversation automatically ([#439](https://github.com/opensearch-project/dashboards-assistant/pull/439))
 - add index type detection ([#454](https://github.com/opensearch-project/dashboards-assistant/pull/454))
+- add error handling when open chatbot and loading converstaion ([#485](https://github.com/opensearch-project/dashboards-assistant/pull/485))
 
 ### Enhancements
 

diff --git a/public/chat_header_button.tsx b/public/chat_header_button.tsx
@@ -114,6 +114,8 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => {
     if (e.key === 'Enter' && inputRef.current && inputRef.current.value.trim().length > 0) {
       // open chat window
       setFlyoutVisible(true);
+      // clear conversations error
+      core.services.conversations.status$.next('idle');
       // start a new chat
       await props.assistantActions.loadChat();
       // send message
@@ -234,7 +236,6 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => {
     if (flyoutVisible) {
       return;
     }
-    core.services.conversationLoad.status$.next('loading');
     core.services.conversations
       .load({
         page: 1,

diff --git a/public/components/__tests__/chat_window_header_title.test.tsx b/public/components/__tests__/chat_window_header_title.test.tsx
@@ -31,6 +31,7 @@ const setup = ({
     services: {
       ...coreMock.createStart(),
       conversations: {
+        status$: new BehaviorSubject('idle'),
         conversations$: new BehaviorSubject({
           objects: [
             {

diff --git a/public/components/chat_window_header_title.tsx b/public/components/chat_window_header_title.tsx
@@ -85,6 +85,7 @@ export const ChatWindowHeaderTitle = React.memo(() => {
       key="new-conversation"
       onClick={() => {
         closePopover();
+        core.services.conversations.status$.next('idle');
         loadChat(undefined);
         // Only show toast when previous conversation saved
         if (!!chatContext.conversationId) {

diff --git a/public/tabs/chat/chat_page.test.tsx b/public/tabs/chat/chat_page.test.tsx
@@ -8,19 +8,22 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/react';
 
 import { coreMock } from '../../../../../src/core/public/mocks';
 import { ConversationLoadService } from '../../services/conversation_load_service';
+import { ConversationsService } from '../../services/conversations_service';
 import { ChatPage } from './chat_page';
 import * as chatContextExports from '../../contexts/chat_context';
 import * as coreContextExports from '../../contexts/core_context';
 import * as hookExports from '../../hooks/use_chat_state';
+import { DataSourceServiceMock } from '../../services/data_source_service.mock';
+import { httpServiceMock } from '../../../../../src/core/public/mocks';
 
 jest.mock('./controls/chat_input_controls', () => {
   return { ChatInputControls: () => <div /> };
 });
 
 jest.mock('./chat_page_content', () => {
   return {
-    ChatPageContent: ({ onRefresh }: { onRefresh: () => void }) => (
-      <button onClick={onRefresh}>refresh</button>
+    ChatPageContent: ({ onRefreshConversation }: { onRefreshConversation: () => void }) => (
+      <button onClick={onRefreshConversation}>refresh</button>
     ),
   };
 });
@@ -35,10 +38,17 @@ describe('<ChatPage />', () => {
     messages: [],
     interactions: [],
   });
+  const loadMockConversations = jest.fn().mockResolvedValue({});
+  const dataSourceServiceMock = new DataSourceServiceMock();
   const conversationLoadService = new ConversationLoadService(coreMock.createStart().http);
+  const conversationsService = new ConversationsService(
+    httpServiceMock.createStartContract(),
+    dataSourceServiceMock
+  );
 
   beforeEach(() => {
     jest.spyOn(conversationLoadService, 'load').mockImplementation(loadMock);
+    jest.spyOn(conversationsService, 'load').mockImplementation(loadMockConversations);
 
     jest.spyOn(chatContextExports, 'useChatContext').mockReturnValue({
       conversationId: 'mocked_conversation_id',
@@ -53,6 +63,7 @@ describe('<ChatPage />', () => {
     jest.spyOn(coreContextExports, 'useCore').mockReturnValue({
       services: {
         conversationLoad: conversationLoadService,
+        conversations: conversationsService,
       },
     });
   });

diff --git a/public/tabs/chat/chat_page.tsx b/public/tabs/chat/chat_page.tsx
@@ -21,9 +21,11 @@ export const ChatPage: React.FC<ChatPageProps> = (props) => {
   const chatContext = useChatContext();
   const { chatState, chatStateDispatch } = useChatState();
   const conversationLoadStatus = useObservable(core.services.conversationLoad.status$);
+  const conversationsStatus = useObservable(core.services.conversations.status$);
   const messagesLoading = conversationLoadStatus === 'loading';
+  const conversationsLoading = conversationsStatus === 'loading';
 
-  const refresh = useCallback(async () => {
+  const refreshConversation = useCallback(async () => {
     if (!chatContext.conversationId) {
       return;
     }
@@ -39,19 +41,54 @@ export const ChatPage: React.FC<ChatPageProps> = (props) => {
     }
   }, [chatContext.conversationId, chatStateDispatch]);
 
+  const refreshConversationsList = useCallback(async () => {
+    if (!chatContext.conversationId) {
+      core.services.conversations
+        .load({
+          page: 1,
+          perPage: 1,
+          fields: ['createdTimeMs', 'updatedTimeMs', 'title'],
+          sortField: 'updatedTimeMs',
+          sortOrder: 'DESC',
+          searchFields: ['title'],
+        })
+        .then(async () => {
+          const data = core.services.conversations.conversations$.getValue();
+          if (data?.objects?.length) {
+            const { id } = data.objects[0];
+            const conversation = await core.services.conversationLoad.load(id);
+            if (conversation) {
+              chatStateDispatch({
+                type: 'receive',
+                payload: {
+                  messages: conversation.messages,
+                  interactions: conversation.interactions,
+                },
+              });
+            }
+          }
+        });
+    }
+  }, [chatStateDispatch]);
+
   return (
     <>
       <EuiFlyoutBody className={cs(props.className, 'llm-chat-flyout-body')}>
         <EuiPage paddingSize="s">
           <EuiPageBody component="div">
             <ChatPageContent
               messagesLoading={messagesLoading}
+              conversationsLoading={conversationsLoading}
               messagesLoadingError={
                 typeof conversationLoadStatus !== 'string'
                   ? conversationLoadStatus?.error
                   : undefined
               }
-              onRefresh={refresh}
+              conversationsError={
+                typeof conversationsStatus !== 'string' ? conversationsStatus?.error : undefined
+              }
+              onRefreshConversation={refreshConversation}
+              onRefreshConversationsList={refreshConversationsList}
             />
           </EuiPageBody>
         </EuiPage>

diff --git a/public/tabs/chat/chat_page_content.test.tsx b/public/tabs/chat/chat_page_content.test.tsx
@@ -85,13 +85,27 @@ describe('<ChatPageContent />', () => {
   });
 
   it('should display welcome message by default', () => {
-    render(<ChatPageContent messagesLoading={false} onRefresh={jest.fn()} />);
+    render(
+      <ChatPageContent
+        messagesLoading={false}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
+    );
     expect(screen.queryAllByLabelText('chat message bubble')).toHaveLength(1);
     expect(screen.queryByLabelText('chat welcome message')).toBeInTheDocument();
   });
 
   it('should display a default suggested action', () => {
-    render(<ChatPageContent messagesLoading={false} onRefresh={jest.fn()} />);
+    render(
+      <ChatPageContent
+        messagesLoading={false}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
+    );
     expect(screen.queryAllByLabelText('chat suggestions')).toHaveLength(1);
     expect(screen.queryByText('What are the indices in my cluster?')).toBeInTheDocument();
   });
@@ -114,7 +128,14 @@ describe('<ChatPageContent />', () => {
       chatState: { messages, llmResponding: false, interactions: [] },
       chatStateDispatch: jest.fn(),
     });
-    render(<ChatPageContent messagesLoading={false} onRefresh={jest.fn()} />);
+    render(
+      <ChatPageContent
+        messagesLoading={false}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
+    );
     expect(screen.queryAllByLabelText('chat message bubble')).toHaveLength(3);
   });
 
@@ -147,7 +168,14 @@ describe('<ChatPageContent />', () => {
       chatState: { messages, llmResponding: false, interactions: [] },
       chatStateDispatch: jest.fn(),
     });
-    render(<ChatPageContent messagesLoading={false} onRefresh={jest.fn()} />);
+    render(
+      <ChatPageContent
+        messagesLoading={false}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
+    );
     expect(screen.queryAllByLabelText('chat suggestions')).toHaveLength(1);
     expect(screen.queryByText('suggested action mock')).toBeInTheDocument();
   });
@@ -170,7 +198,14 @@ describe('<ChatPageContent />', () => {
       chatState: { messages, llmResponding: false, interactions: [] },
       chatStateDispatch: jest.fn(),
     });
-    render(<ChatPageContent messagesLoading={false} onRefresh={jest.fn()} />);
+    render(
+      <ChatPageContent
+        messagesLoading={false}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
+    );
     expect(screen.queryAllByLabelText('chat suggestions')).toHaveLength(0);
   });
 
@@ -197,12 +232,26 @@ describe('<ChatPageContent />', () => {
       chatState: { messages, llmResponding: false, interactions: [] },
       chatStateDispatch: jest.fn(),
     });
-    render(<ChatPageContent messagesLoading={false} onRefresh={jest.fn()} />);
+    render(
+      <ChatPageContent
+        messagesLoading={false}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
+    );
     expect(screen.queryAllByLabelText('chat suggestions')).toHaveLength(0);
   });
 
   it('should display loading screen when loading the messages', () => {
-    render(<ChatPageContent messagesLoading={true} onRefresh={jest.fn()} />);
+    render(
+      <ChatPageContent
+        messagesLoading={true}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
+    );
     expect(screen.queryByText('Loading conversation')).toBeInTheDocument();
     expect(screen.queryAllByLabelText('chat message bubble')).toHaveLength(0);
   });
@@ -212,8 +261,10 @@ describe('<ChatPageContent />', () => {
     render(
       <ChatPageContent
         messagesLoading={false}
+        conversationsLoading={false}
         messagesLoadingError={new Error('failed to get response')}
-        onRefresh={onRefreshMock}
+        onRefreshConversation={onRefreshMock}
+        onRefreshConversationsList={onRefreshMock}
       />
     );
     expect(screen.queryByText('failed to get response')).toBeInTheDocument();
@@ -228,14 +279,28 @@ describe('<ChatPageContent />', () => {
       chatState: { messages: [], llmResponding: true, interactions: [] },
       chatStateDispatch: jest.fn(),
     });
-    render(<ChatPageContent messagesLoading={false} onRefresh={jest.fn()} />);
+    render(
+      <ChatPageContent
+        messagesLoading={false}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
+    );
     expect(screen.queryByText('Stop generating response')).toBeInTheDocument();
     fireEvent.click(screen.getByText('Stop generating response'));
     expect(abortActionMock).toHaveBeenCalledWith('test_conversation_id');
   });
 
   it('should call executeAction', () => {
-    render(<ChatPageContent messagesLoading={false} onRefresh={jest.fn()} />);
+    render(
+      <ChatPageContent
+        messagesLoading={false}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
+    );
     fireEvent.click(screen.getByText('What are the indices in my cluster?'));
     expect(executeActionMock).toHaveBeenCalled();
   });
@@ -269,7 +334,12 @@ describe('<ChatPageContent />', () => {
     });
 
     const { queryAllByTestId } = render(
-      <ChatPageContent messagesLoading={false} onRefresh={jest.fn()} />
+      <ChatPageContent
+        messagesLoading={false}
+        conversationsLoading={false}
+        onRefreshConversation={jest.fn()}
+        onRefreshConversationsList={jest.fn()}
+      />
     );
     expect(queryAllByTestId(`showRegenerate-false`).length).toEqual(2);
   });

diff --git a/public/tabs/chat/chat_page_content.tsx b/public/tabs/chat/chat_page_content.tsx
@@ -28,8 +28,11 @@ import { getConfigSchema } from '../../services';
 
 interface ChatPageContentProps {
   messagesLoading: boolean;
+  conversationsLoading: boolean;
   messagesLoadingError?: Error;
-  onRefresh: () => void;
+  conversationsError?: Error;
+  onRefreshConversation: () => void;
+  onRefreshConversationsList: () => void;
 }
 
 export const ChatPageContent: React.FC<ChatPageContentProps> = React.memo((props) => {
@@ -45,7 +48,30 @@ export const ChatPageContent: React.FC<ChatPageContentProps> = React.memo((props
     pageEndRef.current?.scrollIntoView();
   }, [chatState.messages, loading]);
 
-  if (props.messagesLoading) {
+  if (props.conversationsError) {
+    return (
+      <>
+        <EuiSpacer size="xl" />
+        <EuiEmptyPrompt
+          icon={<EuiIcon type="alert" color="danger" size="xl" />}
+          title={<h1>Error loading conversation</h1>}
+          body={props.conversationsError.body.message}
+          titleSize="l"
+          actions={
+            <EuiSmallButton
+              className="llm-chat-error-refresh-button"
+              fill
+              iconType="refresh"
+              onClick={props.onRefreshConversationsList}
+            >
+              Refresh
+            </EuiSmallButton>
+          }
+        />
+      </>
+    );
+  }
+  if (props.messagesLoading || props.conversationsLoading) {
     return (
       <>
         <EuiSpacer size="xl" />
@@ -72,7 +98,7 @@ export const ChatPageContent: React.FC<ChatPageContentProps> = React.memo((props
               className="llm-chat-error-refresh-button"
               fill
               iconType="refresh"
-              onClick={props.onRefresh}
+              onClick={props.onRefreshConversation}
             >
               Refresh
             </EuiSmallButton>

diff --git a/server/routes/chat_routes.ts b/server/routes/chat_routes.ts
@@ -18,6 +18,7 @@ import { AgentFrameworkStorageService } from '../services/storage/agent_framewor
 import { RoutesOptions } from '../types';
 import { ChatService } from '../services/chat/chat_service';
 import { getOpenSearchClientTransport } from '../utils/get_opensearch_client_transport';
+import { handleError } from './error_handler';
 
 const llmRequestRoute = {
   path: ASSISTANT_API.SEND_MESSAGE,
@@ -289,7 +290,7 @@ export function registerChatRoutes(router: IRouter, routeOptions: RoutesOptions)
         return response.ok({ body: getResponse });
       } catch (error) {
         context.assistant_plugin.logger.error(error);
-        return response.custom({ statusCode: error.statusCode || 500, body: error.message });
+        return handleError(error, response, context.assistant_plugin.logger);
       }
     }
   );