Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AI Assistant] Fix OpenAI, error race condition bug #205665

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

stephmilovic
Copy link
Contributor

@stephmilovic stephmilovic commented Jan 6, 2025

Summary

Fixes a race condition that caused generic errors to show some of the time in OpenAI. The actual fix is adding a condition to the on_llm_end event that does not call handleStreamEnd if generations is undefined. This should only happen when there is an error. The error is caught in post_actions_connector_execute.ts and reported to handleStreamEnd.

I changed the stream style to async iterator to match what we are doing in Bedrock/Gemini. This doesn't have to do with the fix, but makes the code a bit more readable.

To test

  1. Create an OpenAI connector with bad credentials
  2. Send a message with the bad connector selected
  3. Observe an error like 401 Incorrect API key provided. You should NOT see an error like Response from LLM API is empty or undefined
  4. Repeat test ~10 times to ensure race condition has been eliminated

Deleted files

I deleted some files I discovered are no longer in use. This is also unrelated to the bug fix

@stephmilovic stephmilovic added release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Security Generative AI Security Generative AI v8.18.0 labels Jan 6, 2025
@stephmilovic stephmilovic requested a review from a team as a code owner January 6, 2025 23:42
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Comment on lines +158 to +159
// if generation is null, an error occurred - do nothing and let error handling complete the stream
generation != null &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is the actual bug fix, the rest is cleanup to match the same syntax we're using for Bedrock/Gemini

Copy link
Member

@KDKHD KDKHD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally. Looks good!

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #104 / cloud_security_posture Cloud Posture Dashboard Page Kubernetes Dashboard displays accurate summary compliance score

Metrics [docs]

✅ unchanged

@stephmilovic stephmilovic merged commit 2c70e86 into elastic:main Jan 7, 2025
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12653967530

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 7, 2025
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 7, 2025
…205767)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[AI Assistant] Fix OpenAI, error race condition bug
(#205665)](#205665)

<!--- 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":"2025-01-07T15:12:20Z","message":"[AI
Assistant] Fix OpenAI, error race condition bug
(#205665)","sha":"2c70e8651e52966f7c47bedfcd898eeb23bc0b1e","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:
SecuritySolution","backport:prev-minor","Team:Security Generative
AI","v8.18.0"],"title":"[AI Assistant] Fix OpenAI, error race condition
bug","number":205665,"url":"https://github.com/elastic/kibana/pull/205665","mergeCommit":{"message":"[AI
Assistant] Fix OpenAI, error race condition bug
(#205665)","sha":"2c70e8651e52966f7c47bedfcd898eeb23bc0b1e"}},"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/205665","number":205665,"mergeCommit":{"message":"[AI
Assistant] Fix OpenAI, error race condition bug
(#205665)","sha":"2c70e8651e52966f7c47bedfcd898eeb23bc0b1e"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Steph Milovic <[email protected]>
kowalczyk-krzysztof pushed a commit to kowalczyk-krzysztof/kibana that referenced this pull request Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:fix Team:Security Generative AI Security Generative AI Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants