Skip to content

Commit

Permalink
[8.x] [Security Assistant] Fix error handling on new chat (#195507) (#…
Browse files Browse the repository at this point in the history 
…196215)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Assistant] Fix error handling on new chat
(#195507)](#195507)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Steph
Milovic","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-14T22:10:43Z","message":"[Security
Assistant] Fix error handling on new chat
(#195507)","sha":"a15940d9b939dbf29f74dbde28a2a543b8849cc1","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:
SecuritySolution","backport:prev-minor","Team:Security Generative
AI","v8.16.0"],"title":"[Security Assistant] Fix error handling on new
chat","number":195507,"url":"https://github.com/elastic/kibana/pull/195507","mergeCommit":{"message":"[Security
Assistant] Fix error handling on new chat
(#195507)","sha":"a15940d9b939dbf29f74dbde28a2a543b8849cc1"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195507","number":195507,"mergeCommit":{"message":"[Security
Assistant] Fix error handling on new chat
(#195507)","sha":"a15940d9b939dbf29f74dbde28a2a543b8849cc1"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Steph Milovic <[email protected]>
  • Loading branch information
@kibanamachine @stephmilovic
kibanamachine and stephmilovic authored Oct 14, 2024
1 parent db28b89 commit 65cff56
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 33 deletions.
8 changes: 7 additions & 1 deletion x-pack/packages/kbn-langchain/server/language_models/chat_openai.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,13 @@ export class ActionsClientChatOpenAI extends ChatOpenAI {
const actionResult = await this.#actionsClient.execute(requestBody);

if (actionResult.status === 'error') {
throw new Error(`${LLM_TYPE}: ${actionResult?.message} - ${actionResult?.serviceMessage}`);
const error = new Error(
`${LLM_TYPE}: ${actionResult?.message} - ${actionResult?.serviceMessage}`
);
if (actionResult?.serviceMessage) {
error.name = actionResult?.serviceMessage;
}
throw error;
}

if (!this.streaming) {
Expand Down
7 changes: 5 additions & 2 deletions x-pack/packages/kbn-langchain/server/language_models/chat_vertex/chat_vertex.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,14 @@ export class ActionsClientChatVertexAI extends ChatVertexAI {
};

const actionResult = await this.#actionsClient.execute(requestBody);

if (actionResult.status === 'error') {
throw new Error(
const error = new Error(
`ActionsClientChatVertexAI: action result status is error: ${actionResult?.message} - ${actionResult?.serviceMessage}`
);
if (actionResult?.serviceMessage) {
error.name = actionResult?.serviceMessage;
}
throw error;
}

const readable = get('data', actionResult) as Readable;
Expand Down
6 changes: 5 additions & 1 deletion x-pack/packages/kbn-langchain/server/language_models/chat_vertex/connection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,13 @@ export class ActionsClientChatConnection<Auth> extends ChatConnection<Auth> {
};

if (actionResult.status === 'error') {
throw new Error(
const error = new Error(
`ActionsClientChatVertexAI: action result status is error: ${actionResult?.message} - ${actionResult?.serviceMessage}`
);
if (actionResult?.serviceMessage) {
error.name = actionResult?.serviceMessage;
}
throw error;
}

if (actionResult.data.candidates && actionResult.data.candidates.length > 0) {
Expand Down
12 changes: 10 additions & 2 deletions x-pack/packages/kbn-langchain/server/language_models/gemini_chat.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,13 @@ export class ActionsClientGeminiChatModel extends ChatGoogleGenerativeAI {
};

if (actionResult.status === 'error') {
throw new Error(
const error = new Error(
`ActionsClientGeminiChatModel: action result status is error: ${actionResult?.message} - ${actionResult?.serviceMessage}`
);
if (actionResult?.serviceMessage) {
error.name = actionResult?.serviceMessage;
}
throw error;
}

if (actionResult.data.candidates && actionResult.data.candidates.length > 0) {
Expand Down Expand Up @@ -162,9 +166,13 @@ export class ActionsClientGeminiChatModel extends ChatGoogleGenerativeAI {
const actionResult = await this.#actionsClient.execute(requestBody);

if (actionResult.status === 'error') {
throw new Error(
const error = new Error(
`ActionsClientGeminiChatModel: action result status is error: ${actionResult?.message} - ${actionResult?.serviceMessage}`
);
if (actionResult?.serviceMessage) {
error.name = actionResult?.serviceMessage;
}
throw error;
}

const readable = get('data', actionResult) as Readable;
Expand Down
6 changes: 5 additions & 1 deletion x-pack/packages/kbn-langchain/server/language_models/llm.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,13 @@ export class ActionsClientLlm extends LLM {
const actionResult = await this.#actionsClient.execute(requestBody);

if (actionResult.status === 'error') {
throw new Error(
const error = new Error(
`${LLM_TYPE}: action result status is error: ${actionResult?.message} - ${actionResult?.serviceMessage}`
);
if (actionResult?.serviceMessage) {
error.name = actionResult?.serviceMessage;
}
throw error;
}

const content = get('data.message', actionResult);
Expand Down
12 changes: 10 additions & 2 deletions x-pack/packages/kbn-langchain/server/language_models/simple_chat_model.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,13 @@ export class ActionsClientSimpleChatModel extends SimpleChatModel {
const actionResult = await this.#actionsClient.execute(requestBody);

if (actionResult.status === 'error') {
throw new Error(
const error = new Error(
`ActionsClientSimpleChatModel: action result status is error: ${actionResult?.message} - ${actionResult?.serviceMessage}`
);
if (actionResult?.serviceMessage) {
error.name = actionResult?.serviceMessage;
}
throw error;
}

if (!this.streaming) {
Expand Down Expand Up @@ -217,9 +221,13 @@ export class ActionsClientSimpleChatModel extends SimpleChatModel {
const actionResult = await this.#actionsClient.execute(requestBody);

if (actionResult.status === 'error') {
throw new Error(
const error = new Error(
`ActionsClientSimpleChatModel: action result status is error: ${actionResult?.message} - ${actionResult?.serviceMessage}`
);
if (actionResult?.serviceMessage) {
error.name = actionResult?.serviceMessage;
}
throw error;
}

const readable = get('data', actionResult) as Readable;
Expand Down
39 changes: 24 additions & 15 deletions ...ssistant/server/lib/langchain/graphs/default_assistant_graph/nodes/generate_chat_title.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,31 @@ export async function generateChatTitle({
state,
model,
}: GenerateChatTitleParams): Promise<Partial<AgentState>> {
logger.debug(
() => `${NodeType.GENERATE_CHAT_TITLE}: Node state:\n${JSON.stringify(state, null, 2)}`
);
try {
logger.debug(
() => `${NodeType.GENERATE_CHAT_TITLE}: Node state:\n${JSON.stringify(state, null, 2)}`
);

const outputParser = new StringOutputParser();
const graph = GENERATE_CHAT_TITLE_PROMPT(state.responseLanguage, state.llmType)
.pipe(model)
.pipe(outputParser);
const outputParser = new StringOutputParser();
const graph = GENERATE_CHAT_TITLE_PROMPT(state.responseLanguage, state.llmType)
.pipe(model)
.pipe(outputParser);

const chatTitle = await graph.invoke({
input: JSON.stringify(state.input, null, 2),
});
logger.debug(`chatTitle: ${chatTitle}`);
const chatTitle = await graph.invoke({
input: JSON.stringify(state.input, null, 2),
});
logger.debug(`chatTitle: ${chatTitle}`);

return {
chatTitle,
lastNode: NodeType.GENERATE_CHAT_TITLE,
};
return {
chatTitle,
lastNode: NodeType.GENERATE_CHAT_TITLE,
};
} catch (e) {
return {
// generate a chat title if there is an error in order to complete the graph
// limit title to 60 characters
chatTitle: (e.name ?? e.message ?? e.toString()).slice(0, 60),
lastNode: NodeType.GENERATE_CHAT_TITLE,
};
}
}
11 changes: 5 additions & 6 deletions x-pack/test/security_solution_cypress/cypress/e2e/ai_assistant/conversations.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
createNewChat,
selectConversation,
assertMessageSent,
assertConversationTitle,
typeAndSendMessage,
assertErrorResponse,
selectRule,
Expand Down Expand Up @@ -145,18 +146,16 @@ describe('AI Assistant Conversations', { tags: ['@ess', '@serverless'] }, () =>
assertConnectorSelected(bedrockConnectorAPIPayload.name);
assertMessageSent('goodbye');
});
// This test is flakey due to the issue linked below and will be skipped until it is fixed
it.skip('Only allows one conversation called "New chat" at a time', () => {
it('Correctly titles new conversations, and only allows one conversation called "New chat" at a time', () => {
visitGetStartedPage();
openAssistant();
createNewChat();
assertNewConversation(false, 'New chat');
assertConnectorSelected(azureConnectorAPIPayload.name);
typeAndSendMessage('hello');
// TODO fix bug with new chat and error message
// https://github.com/elastic/kibana/issues/191025
// assertMessageSent('hello');
assertErrorResponse();
assertMessageSent('hello');
assertConversationTitle('Unexpected API Error: - Connection error.');
updateConversationTitle('New chat');
selectConversation('Welcome');
createNewChat();
assertErrorToastShown('Error creating conversation with title New chat');
Expand Down
9 changes: 6 additions & 3 deletions x-pack/test/security_solution_cypress/cypress/tasks/assistant.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const resetConversation = () => {
export const selectConversation = (conversationName: string) => {
cy.get(FLYOUT_NAV_TOGGLE).click();
cy.get(CONVERSATION_SELECT(conversationName)).click();
cy.get(CONVERSATION_TITLE + ' h2').should('have.text', conversationName);
assertConversationTitle(conversationName);
cy.get(FLYOUT_NAV_TOGGLE).click();
};

Expand All @@ -95,7 +95,7 @@ export const updateConversationTitle = (newTitle: string) => {
cy.get(CONVERSATION_TITLE + ' input').clear();
cy.get(CONVERSATION_TITLE + ' input').type(newTitle);
cy.get(CONVERSATION_TITLE + ' input').type('{enter}');
cy.get(CONVERSATION_TITLE + ' h2').should('have.text', newTitle);
assertConversationTitle(newTitle);
};

export const typeAndSendMessage = (message: string) => {
Expand Down Expand Up @@ -171,9 +171,12 @@ export const assertNewConversation = (isWelcome: boolean, title: string) => {
} else {
cy.get(EMPTY_CONVO).should('be.visible');
}
cy.get(CONVERSATION_TITLE + ' h2').should('have.text', title);
assertConversationTitle(title);
};

export const assertConversationTitle = (title: string) =>
cy.get(CONVERSATION_TITLE + ' h2').should('have.text', title);

export const assertSystemPromptSent = (message: string) => {
cy.get(CONVERSATION_MESSAGE).eq(0).should('contain', message);
};
Expand Down

0 comments on commit 65cff56

Please sign in to comment.