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

[BUG] Some error messages are being hidden during provisioning #670

Closed
amitgalitz opened this issue Apr 18, 2024 · 4 comments · Fixed by #676
Closed

[BUG] Some error messages are being hidden during provisioning #670

amitgalitz opened this issue Apr 18, 2024 · 4 comments · Fixed by #676
Assignees
Labels
bug Something isn't working v2.14.0

Comments

@amitgalitz
Copy link
Member

amitgalitz commented Apr 18, 2024

What is the bug?

Our error message that we write to the state index while provisioning don't have enough info.

For example if the user has a typo in the processor name this is the error we write

"org.opensearch.flowframework.exception.WorkflowStepException during step create_ingest_pipeline, restStatus: BAD_REQUEST" 

we use WorkflowStepException to let us know the step itself is the reason for the error, but we don't propagate outside the logs the fact that the issue is that we don't have text_embedding processor available in the cluster.

Today if a user hits the create ingest pipeline API directly they get more information without having to look at the logs:

{
    "error": {
        "root_cause": [
            {
                "type": "parse_exception",
                "reason": "No processor type exists with name [text_embeddinssg]",
                "processor_type": "text_embeddinssg"
            }
        ],
        "type": "parse_exception",
        "reason": "No processor type exists with name [text_embeddinssg]",
        "processor_type": "text_embeddinssg"
    },
    "status": 400
}

We should potentially write the exception itself too to the state index, however there can be times where the exception exposes too much information

@dbwiddis
Copy link
Member

however there can be times where the exception exposes too much information

This is the reason we restricted it so much. How can we programmatically decide what's safe to pass on?

@dbwiddis
Copy link
Member

The above exception is an OpenSearchException. These are designed to be returned to users. As a first step we should probably allow these exception messages to be passed through (along with their RestStatus)

@owaiskazi19
Copy link
Member

One of the hacky ways to handle this before writing exceptions is to check which exceptions can not be safely exposed (such as those from RemoteTransportException) and, while adding the rest of the exceptions separately and include them in the error message.

@dbwiddis
Copy link
Member

Looking at our implemented steps, we can probably just include:

  • OpenSearchStatusException (common in ML Commons)
  • OpenSearchParseException (a few in ML Commons)
  • OpenSearchException with a getCause() of OpenSearchParseException (Search and Ingest Pipeline configurations)

Haven't looked at any exceptions thrown in Create Index or Reindex, but that would likely complete it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v2.14.0
Projects
None yet
3 participants