Skip to content

[Security Solution][Endpoint] Better error handling for ES errors#266524

Merged
paul-tavares merged 7 commits into
elastic:mainfrom
paul-tavares:task/olm-266355-fix-s1-agent-query
May 6, 2026
Merged

[Security Solution][Endpoint] Better error handling for ES errors#266524
paul-tavares merged 7 commits into
elastic:mainfrom
paul-tavares:task/olm-266355-fix-s1-agent-query

Conversation

@paul-tavares
Copy link
Copy Markdown
Contributor

@paul-tavares paul-tavares commented Apr 29, 2026

Summary

  • Improve error handling of errors, specifically those thrown by ES/SO clients
    • Logic in wrapErrorIfNeeded() will now check to see if the error being wrapped is from ES and if so, it will attempt to generate a better error message as well as capture additional debug data

@paul-tavares paul-tavares self-assigned this Apr 29, 2026
@paul-tavares paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution backport:version Backport to applied version labels v9.4.0 v9.5.0 labels Apr 29, 2026
@paul-tavares paul-tavares marked this pull request as ready for review April 30, 2026 18:28
@paul-tavares paul-tavares requested a review from a team as a code owner April 30, 2026 18:28
@paul-tavares paul-tavares requested review from pzl and tomsonpl April 30, 2026 18:28
@infra-vault-gh-plugin-prod
Copy link
Copy Markdown

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

Copy link
Copy Markdown
Contributor

@szwarckonrad szwarckonrad left a comment

Choose a reason for hiding this comment

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

I’ve left a few comments, but they don’t seem to be in the way. Please decide if they are worth pursuing.

logger.debug(stringify(error, 20));
} else {
logger.error(error);
logger.error(stringify(error, 20));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI generated:

This drops the structured logger path for errors. When an Error is passed to logger.error, Kibana's BaseLogger.createLogRecord builds a LogRecord with error: <Error> alongside message — ECS appenders use that to populate error.type, error.message, error.stack_trace, etc. Stringifying first collapses everything into message, and any consumer indexing on error.* (Filebeat → ES, etc.) silently loses those fields.

Could we keep the Error and put the new debug payload in LogMeta instead?

*/
export class EndpointError<MetaType = unknown> extends Error {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
public debug: any = undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe we could go with something like

export interface EndpointErrorDebug {
  es_request?: {
    method?: string;
    path?: string;
    querystring?: unknown;
    body?: unknown;
  };
  es_response?: { body?: unknown };
}

export class EndpointError<MetaType = unknown> extends Error {
  public debug?: EndpointErrorDebug;
  // ...
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

EndpointError is generic and also used by the UI, so I rather keep debug property set at any. Although with this change was triggered by the lack of detail while investigating a recent issue that originated from a ES error, the debug property is really to capture anything else that one might like to include when an error of this type if thrown.


try {
// Process known error Types and retrieve additional data not normally output to logs
if (error instanceof errors.ElasticsearchClientError) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be worth adding a small wrap_errors.test.ts alongside this?

try {
// Process known error Types and retrieve additional data not normally output to logs
if (error instanceof errors.ElasticsearchClientError) {
const esError = error as { meta?: DiagnosticResult; body?: any };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of typin we could probably go with

if (error instanceof errors.ResponseError) {
  const { params } = error.meta.meta.request;
  debug = {
    es_request: {
      method: params.method,
      ...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That will not work. Here I'm checking that the Error is one emitted by the Elastic search client (the base Error class) and that Error definition does not have a meta thus why I defined the type above. Tehre are a few errors that have the meta, so defining the type here allows us to capture data for all of them - including any new one that might be added in the future.

see node_modules/@elastic/elasticsearch/node_modules/@elastic/transport/lib/errors.d.ts to view the types defined for ES client errors

@kibanamachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
securitySolution 105 106 +1

ESLint disabled line counts

id before after diff
securitySolution 754 756 +2

Total ESLint disabled count

id before after diff
securitySolution 859 862 +3

History

cc @paul-tavares

Copy link
Copy Markdown
Contributor

@tomsonpl tomsonpl left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@paul-tavares paul-tavares merged commit 0e7f25b into elastic:main May 6, 2026
25 checks passed
@paul-tavares paul-tavares deleted the task/olm-266355-fix-s1-agent-query branch May 6, 2026 12:08
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 9.4

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

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
9.4

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 May 6, 2026
…rs (#266524) (#267961)

# Backport

This will backport the following commits from `main` to `9.4`:
- [[Security Solution][Endpoint] Better error handling for ES errors
(#266524)](#266524)

<!--- Backport version: 9.6.6 -->

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

<!--BACKPORT [{"author":{"name":"Paul
Tavares","email":"56442535+paul-tavares@users.noreply.github.com"},"sourceCommit":{"committedDate":"2026-05-06T12:08:17Z","message":"[Security
Solution][Endpoint] Better error handling for ES errors (#266524)\n\n##
Summary\n\n- Improve error handling of errors, specifically those thrown
by ES/SO\nclients\n- Logic in `wrapErrorIfNeeded()` will now check to
see if the error\nbeing wrapped is from ES and if so, it will attempt to
generate a better\nerror message as well as capture additional debug
data","sha":"0e7f25b99db349ee8388797c7ff5a25f0ba2cd50","branchLabelMapping":{"^v9.5.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Defend
Workflows","backport:version","v9.4.0","v9.5.0"],"title":"[Security
Solution][Endpoint] Better error handling for ES
errors","number":266524,"url":"https://github.com/elastic/kibana/pull/266524","mergeCommit":{"message":"[Security
Solution][Endpoint] Better error handling for ES errors (#266524)\n\n##
Summary\n\n- Improve error handling of errors, specifically those thrown
by ES/SO\nclients\n- Logic in `wrapErrorIfNeeded()` will now check to
see if the error\nbeing wrapped is from ES and if so, it will attempt to
generate a better\nerror message as well as capture additional debug
data","sha":"0e7f25b99db349ee8388797c7ff5a25f0ba2cd50"}},"sourceBranch":"main","suggestedTargetBranches":["9.4"],"targetPullRequestStates":[{"branch":"9.4","label":"v9.4.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.5.0","branchLabelMappingKey":"^v9.5.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/266524","number":266524,"mergeCommit":{"message":"[Security
Solution][Endpoint] Better error handling for ES errors (#266524)\n\n##
Summary\n\n- Improve error handling of errors, specifically those thrown
by ES/SO\nclients\n- Logic in `wrapErrorIfNeeded()` will now check to
see if the error\nbeing wrapped is from ES and if so, it will attempt to
generate a better\nerror message as well as capture additional debug
data","sha":"0e7f25b99db349ee8388797c7ff5a25f0ba2cd50"}}]}] BACKPORT-->

Co-authored-by: Paul Tavares <56442535+paul-tavares@users.noreply.github.com>
ersin-erdal pushed a commit to ersin-erdal/kibana that referenced this pull request May 6, 2026
…astic#266524)

## Summary

- Improve error handling of errors, specifically those thrown by ES/SO
clients
- Logic in `wrapErrorIfNeeded()` will now check to see if the error
being wrapped is from ES and if so, it will attempt to generate a better
error message as well as capture additional debug data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v9.4.0 v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants