opensearch-ruby icon indicating copy to clipboard operation
opensearch-ruby copied to clipboard

[FEATURE] Detailed exception data for InternalServerError

Open Mark-ICS opened this issue 2 years ago • 3 comments

Is your feature request related to a problem?

I am using the client to communicate with an OpenSearch instance that uses the ML Commons plugin.

With the ML commons plugin, the model must be loaded each time the OpenSearch server restarts. The model can only be loaded via an API request.

If I make a query using a model id for a model that is not loaded, the client throws the following exception:

OpenSearch::Transport::Transport::Errors::InternalServerError

With the message:

[500] {"error":{"root_cause":[{"type":"m_l_exception","reason":"model not loaded"}],"type":"m_l_exception","reason":"model not loaded"},"status":500}

What solution would you like?

While the information in the exception message does contain the relevant information, it's clunky because I need to trim the [500] in order to parse the JSON in the message.

Would would be ideal, is if the exception had a status attribute and parsed_data which contained the response as a hash containing the parsed data.

An example of usage would be:

def my_search_method(index, body)
  client.search(index: index, body: body)
rescue OpenSearch::Transport::Transport::Errors::InternalServerError => e
  if e.status == 500 && e.parsed_data["error"]["root_cause"][0]["reason"] == 'model not loaded'
    load_model
  end
end

What alternatives have you considered?

A clear and concise description of any alternative solutions or features you've considered.

The alternative is to trim the [500] from the beginning and the parse the rest. This would work but it's prone to errors and also a bit clunky.

Do you have any additional context?

Hopefully I've provided enough context above but happy to clarify and questions.

Also, if it turns out I'm being an idiot and there is already a better way to do this that I'm not thinking of, I'd be grateful for the feedback 😅

Mark-ICS avatar Oct 17 '23 08:10 Mark-ICS

If all server-side exceptions have the error structure, you probably want to be able to do e.error.root_causes.first.reason.

dblock avatar Oct 17 '23 15:10 dblock

That makes sense @dblock. Just to clarify though, are you suggesting that in terms of design, or should that currently work?

Mark-ICS avatar Oct 17 '23 15:10 Mark-ICS

In terms of design, someone should implement this (wink wink :)).

dblock avatar Oct 17 '23 18:10 dblock