From 418ad3a16a1b934289be003baefc653b2a9e3c28 Mon Sep 17 00:00:00 2001 From: Steph Milovic Date: Fri, 29 Sep 2023 16:14:51 -0600 Subject: [PATCH] better error design --- .../impl/assistant/api.test.tsx | 22 ++++----- .../impl/assistant/api.tsx | 28 ++++++++--- .../impl/assistant/helpers.ts | 8 ++- .../assistant/use_send_messages/index.tsx | 8 ++- .../impl/assistant_context/types.tsx | 1 + .../assistant/get_comments/index.test.tsx | 49 +++++++++++++++++++ .../public/assistant/get_comments/index.tsx | 18 ++++++- 7 files changed, 112 insertions(+), 22 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/assistant/get_comments/index.test.tsx diff --git a/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.test.tsx b/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.test.tsx index 0f5c06ba782fb..92e170defd999 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.test.tsx +++ b/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.test.tsx @@ -84,7 +84,7 @@ describe('fetchConnectorExecuteAction', () => { const result = await fetchConnectorExecuteAction(testProps); - expect(result).toBe(API_ERROR); + expect(result).toEqual({ response: API_ERROR, isError: true }); }); it('returns API_ERROR when there are no choices', async () => { @@ -98,15 +98,15 @@ describe('fetchConnectorExecuteAction', () => { const result = await fetchConnectorExecuteAction(testProps); - expect(result).toBe(API_ERROR); + expect(result).toEqual({ response: API_ERROR, isError: true }); }); it('returns the value of the action_input property when assistantLangChain is true, and `content` has properly prefixed and suffixed JSON with the action_input property', async () => { - const content = '```json\n{"action_input": "value from action_input"}\n```'; + const response = '```json\n{"action_input": "value from action_input"}\n```'; (mockHttp.fetch as jest.Mock).mockResolvedValue({ status: 'ok', - data: content, + data: response, }); const testProps: FetchConnectorExecuteAction = { @@ -118,15 +118,15 @@ describe('fetchConnectorExecuteAction', () => { const result = await fetchConnectorExecuteAction(testProps); - expect(result).toBe('value from action_input'); + expect(result).toEqual({ response: 'value from action_input', isError: false }); }); it('returns the original content when assistantLangChain is true, and `content` has properly formatted JSON WITHOUT the action_input property', async () => { - const content = '```json\n{"some_key": "some value"}\n```'; + const response = '```json\n{"some_key": "some value"}\n```'; (mockHttp.fetch as jest.Mock).mockResolvedValue({ status: 'ok', - data: content, + data: response, }); const testProps: FetchConnectorExecuteAction = { @@ -138,15 +138,15 @@ describe('fetchConnectorExecuteAction', () => { const result = await fetchConnectorExecuteAction(testProps); - expect(result).toBe(content); + expect(result).toEqual({ response, isError: false }); }); it('returns the original when assistantLangChain is true, and `content` is not JSON', async () => { - const content = 'plain text content'; + const response = 'plain text content'; (mockHttp.fetch as jest.Mock).mockResolvedValue({ status: 'ok', - data: content, + data: response, }); const testProps: FetchConnectorExecuteAction = { @@ -158,6 +158,6 @@ describe('fetchConnectorExecuteAction', () => { const result = await fetchConnectorExecuteAction(testProps); - expect(result).toBe(content); + expect(result).toEqual({ response, isError: false }); }); }); diff --git a/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.tsx b/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.tsx index b5595cba40fa8..c7c1254656d61 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.tsx +++ b/x-pack/packages/kbn-elastic-assistant/impl/assistant/api.tsx @@ -23,13 +23,18 @@ export interface FetchConnectorExecuteAction { signal?: AbortSignal | undefined; } +export interface FetchConnectorExecuteResponse { + response: string; + isError: boolean; +} + export const fetchConnectorExecuteAction = async ({ assistantLangChain, http, messages, apiConfig, signal, -}: FetchConnectorExecuteAction): Promise => { +}: FetchConnectorExecuteAction): Promise => { const outboundMessages = messages.map((msg) => ({ role: msg.role, content: msg.content, @@ -77,14 +82,25 @@ export const fetchConnectorExecuteAction = async ({ if (response.status !== 'ok' || !response.data) { if (response.service_message) { - return `${API_ERROR} \n\n${response.service_message}`; + return { + response: `${API_ERROR}\n\n${response.service_message}`, + isError: true, + }; } - return API_ERROR; + return { + response: API_ERROR, + isError: true, + }; } - - return assistantLangChain ? getFormattedMessageContent(response.data) : response.data; + return { + response: assistantLangChain ? getFormattedMessageContent(response.data) : response.data, + isError: false, + }; } catch (error) { - return API_ERROR; + return { + response: API_ERROR, + isError: true, + }; } }; diff --git a/x-pack/packages/kbn-elastic-assistant/impl/assistant/helpers.ts b/x-pack/packages/kbn-elastic-assistant/impl/assistant/helpers.ts index 2b2c5b76851f7..b4eb89a092600 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/assistant/helpers.ts +++ b/x-pack/packages/kbn-elastic-assistant/impl/assistant/helpers.ts @@ -6,23 +6,27 @@ */ import { ActionConnector } from '@kbn/triggers-actions-ui-plugin/public'; +import { FetchConnectorExecuteResponse } from './api'; import { Conversation } from '../..'; import type { Message } from '../assistant_context/types'; import { enterpriseMessaging, WELCOME_CONVERSATION } from './use_conversation/sample_conversations'; -export const getMessageFromRawResponse = (rawResponse: string): Message => { +export const getMessageFromRawResponse = (rawResponse: FetchConnectorExecuteResponse): Message => { + const { response, isError } = rawResponse; const dateTimeString = new Date().toLocaleString(); // TODO: Pull from response if (rawResponse) { return { role: 'assistant', - content: rawResponse, + content: response, timestamp: dateTimeString, + isError, }; } else { return { role: 'assistant', content: 'Error: Response from LLM API is empty or undefined.', timestamp: dateTimeString, + isError: true, }; } }; diff --git a/x-pack/packages/kbn-elastic-assistant/impl/assistant/use_send_messages/index.tsx b/x-pack/packages/kbn-elastic-assistant/impl/assistant/use_send_messages/index.tsx index c68d82d99b9ac..38dc60c5fc9e7 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/assistant/use_send_messages/index.tsx +++ b/x-pack/packages/kbn-elastic-assistant/impl/assistant/use_send_messages/index.tsx @@ -11,7 +11,7 @@ import { HttpSetup } from '@kbn/core-http-browser'; import { useAssistantContext } from '../../assistant_context'; import { Conversation, Message } from '../../assistant_context/types'; -import { fetchConnectorExecuteAction } from '../api'; +import { fetchConnectorExecuteAction, FetchConnectorExecuteResponse } from '../api'; interface SendMessagesProps { http: HttpSetup; @@ -21,7 +21,11 @@ interface SendMessagesProps { interface UseSendMessages { isLoading: boolean; - sendMessages: ({ apiConfig, http, messages }: SendMessagesProps) => Promise; + sendMessages: ({ + apiConfig, + http, + messages, + }: SendMessagesProps) => Promise; } export const useSendMessages = (): UseSendMessages => { diff --git a/x-pack/packages/kbn-elastic-assistant/impl/assistant_context/types.tsx b/x-pack/packages/kbn-elastic-assistant/impl/assistant_context/types.tsx index bf0a20e2b6ea5..651eeee17f21e 100644 --- a/x-pack/packages/kbn-elastic-assistant/impl/assistant_context/types.tsx +++ b/x-pack/packages/kbn-elastic-assistant/impl/assistant_context/types.tsx @@ -17,6 +17,7 @@ export interface Message { role: ConversationRole; content: string; timestamp: string; + isError?: boolean; presentation?: MessagePresentation; } diff --git a/x-pack/plugins/security_solution/public/assistant/get_comments/index.test.tsx b/x-pack/plugins/security_solution/public/assistant/get_comments/index.test.tsx new file mode 100644 index 0000000000000..d842ae801223e --- /dev/null +++ b/x-pack/plugins/security_solution/public/assistant/get_comments/index.test.tsx @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { getComments } from '.'; + +describe('getComments', () => { + it('Does not add error state message has no error', () => { + const currentConversation = { + apiConfig: {}, + id: '1', + messages: [ + { + role: 'user', + content: 'Hello {name}', + timestamp: '2022-01-01', + isError: false, + }, + ], + }; + const lastCommentRef = { current: null }; + const showAnonymizedValues = false; + + const result = getComments({ currentConversation, lastCommentRef, showAnonymizedValues }); + expect(result[0].eventColor).toEqual(undefined); + }); + it('Adds error state when message has error', () => { + const currentConversation = { + apiConfig: {}, + id: '1', + messages: [ + { + role: 'user', + content: 'Hello {name}', + timestamp: '2022-01-01', + isError: true, + }, + ], + }; + const lastCommentRef = { current: null }; + const showAnonymizedValues = false; + + const result = getComments({ currentConversation, lastCommentRef, showAnonymizedValues }); + expect(result[0].eventColor).toEqual('danger'); + }); +}); diff --git a/x-pack/plugins/security_solution/public/assistant/get_comments/index.tsx b/x-pack/plugins/security_solution/public/assistant/get_comments/index.tsx index b6d4d9723d87b..d91caae855d22 100644 --- a/x-pack/plugins/security_solution/public/assistant/get_comments/index.tsx +++ b/x-pack/plugins/security_solution/public/assistant/get_comments/index.tsx @@ -7,10 +7,12 @@ import type { EuiCommentProps } from '@elastic/eui'; import type { Conversation } from '@kbn/elastic-assistant'; -import { EuiAvatar, EuiMarkdownFormat, EuiText } from '@elastic/eui'; +import { EuiAvatar, EuiMarkdownFormat, EuiText, tint } from '@elastic/eui'; import React from 'react'; import { AssistantAvatar } from '@kbn/elastic-assistant'; +import { css } from '@emotion/react/dist/emotion-react.cjs'; +import { euiThemeVars } from '@kbn/ui-theme'; import { CommentActions } from '../comment_actions'; import * as i18n from './translations'; @@ -64,5 +66,19 @@ export const getComments = ({ message.timestamp.length === 0 ? new Date().toLocaleString() : message.timestamp ), username: isUser ? i18n.YOU : i18n.ASSISTANT, + ...(message.isError + ? { + eventColor: 'danger', + css: css` + .euiCommentEvent { + border: 1px solid ${tint(euiThemeVars.euiColorDanger, 0.75)}; + } + .euiCommentEvent__header { + padding: 0 !important; + border-block-end: 1px solid ${tint(euiThemeVars.euiColorDanger, 0.75)}; + } + `, + } + : {}), }; });