One thing I'd like to call out here. If Elasticsearch returns an object missing all the properties that we care about, is this a case where we want to throw? If ES for some reason sends:

{
  error: { }
}

Do we care about throwing an error for that? Would that happen if we didn't? Just telling the user "ES threw an unknown error" is not going to be super helpful for them.

@kpatticha since you're more aware of the actual situation we are addressing here, do you have a thought on a case where the error has none of the keys we're filtering for here?

When it comes to the "why", even though it might be rare for ES to thrown an empty error, it can happen. We got a similar behaviour here, where we fallback to a default message

const message = causeReason ? causeReason : i18n.translate('searchErrors.esError.unknownRootCause', { defaultMessage: 'unknown' });

since you're more aware of the actual situation we are addressing here, do you have a thought on a case where the error has none of the keys we're filtering for here?

Good call out @justinkambic . I’m not aware of a specific case, but I think we should align with the pattern used in Discover here.

Thanks for taking this Jorge!

I didn't test it, but a few early suggestions

• Try to reuse type, taking a quickly look maybe we can reuse ErrorCause from @elastic/elasticsearch instead of the inline cast { type?; reason?; root_cause? } .
• Returning string | undefined is ok but maybe we want to make some logic in the future and it will depends of the reason, type, etc. Instead of manage with strings what if we create a Class with properties?
• Instead of checking the response inline after getESQLResults, a small wrapper could wrap the API call and guarantee the response is clean so executeEsqlQuery stays linear.
• Add tests to cover new functions
• One concern: since we're throwing an error, we should make sure every consumer of executeEsqlQuery handles it properly. Today we're safe because useAsyncFn in useFetchMetricsData catches everything. But if a future consumer calls executeEsqlQuery without a try/catch or useAsyncFn, will we break the experience?

I put together one possible approach to illustrate the idea, just a starting point:

// esql_response_error.ts

export class EsqlResponseError extends Error {
  public readonly type?: string;
  public readonly reason?: string;
  public readonly rootCause?: ErrorCause[];

  constructor(errorCause: ErrorCause) {
    super(formatErrorCause(errorCause));
    this.name = 'EsqlResponseError';
    this.type = errorCause.type;
    this.reason = errorCause.reason ?? undefined;
    this.rootCause = errorCause.root_cause;
  }
}

// execute_esql_query.ts

const fetchEsqlResponseOrThrow = async (
  params: Parameters<typeof getESQLResults>[0]
): Promise<ESQLSearchResponse> => {
  const { response } = await getESQLResults(params);
  const errorCause = extractEsqlResponseErrorCause(response);
  if (errorCause) {
    throw new EsqlResponseError(errorCause);
  }
  return response;
};

// execute_esql_query.ts (usage in executeEsqlQuery)

const response = await fetchEsqlResponseOrThrow({
  esqlQuery, search, signal, filter, timeRange, variables,
  ...getMetricsExecutionContext(
    MetricsExecutionContextAction.FETCH,
    MetricsExecutionContextName.METRICS_INFO
  ),
});

return esqlResultToPlainObjects<TDocument>(response);

Let me know what you think!

Thanks for your comment!

When it comes to your suggestions:

• Sounds great and I already have a small change in place
• If we're talking a simple, small class, it could make sense even if we're most likely just going to use a general message
• Sounds good
• Yes, some are already in place but with these changes I'll add a few more most likely
• That makes sense, we could change the return type to force the users to handle the response properly but, it could also be a bit more costly. Shall we create a small follow-up for this specific point?

That makes sense, we could change the return type to force the users to handle the response properly but, it could also be a bit more costly. Shall we create a small follow-up for this specific point?

It depends, what happens when we throw an error? We should now how it behaves now before taking a decision

This is probably fine for now, but we should not implement custom error messages. If such handlers are needed, they should be handled at the ESQL level rather than on our side.

Totally agree, we created this issue to handle things like this and more

A try/catch handler would likely handle the same