opensearch-project / opensearch-plugins

For all things OpenSearch plugins. You want to install, or develop a plugin? You've come to the right place.
Apache License 2.0
49 stars 61 forks source link

[PROPOSAL] Define a standardized struct for error responses #215

Open Jakob3xD opened 1 year ago

Jakob3xD commented 1 year ago

What/Why

What are you proposing?

Define a standardized struct for error responses.

I suggest that we agree one some fields that are required to exists in the error response, but plugins can add more fields to the response if they want.

For errors except a 400:

{
  "error": {
    "type": "status_exception",
    "reason": "xyz"
  },
  "status": int
}

For 400 errors:

{
  "error": "no handler found for uri [<URL-path>] and method [<method>]"
}

AFAIK this is what Opensearch Core does, except that error also has a root_cause for none 400 errors.

What users have asked for this feature?

What problems are you trying to solve?

I stated as a maintainer for the opensearch-go lib and notices that some plugins return errors differently then other, while rewriting some code. As everything needs to be predefined in golang, we now need to handle all errors per plugin and can´t create one general struct where the most important fields are defined. This results in more Code and unneeded handling of different errors. Moreover as a user, I think having one standardized error struct helps understanding the error.

Example Errors 404 for a missing index: ``` GET test { "error": { "root_cause": [ { "type": "index_not_found_exception", "reason": "no such index [test]", "index": "test", "resource.id": "test", "resource.type": "index_or_alias", "index_uuid": "_na_" } ], "type": "index_not_found_exception", "reason": "no such index [test]", "index": "test", "resource.id": "test", "resource.type": "index_or_alias", "index_uuid": "_na_" }, "status": 404 } ``` 404 for a none existing user ``` GET /_plugins/_security/api/internalusers/test { "status": "NOT_FOUND", "message": "Resource 'test' not found." } ``` 404 for a none existing ism policy ``` GET _plugins/_ism/policies/policy_1 { "error": { "root_cause": [ { "type": "status_exception", "reason": "Policy not found" } ], "type": "status_exception", "reason": "Policy not found" }, "status": 404 } ```

What is the developer experience going to be?

Plugins that don't return errors in the wanted struct would need to change the error responses and do a braking change.

Are there any security considerations?

No

Are there any breaking changes to the API

Yes, this will be a braking change for every plugin that don´t use the standardized error response struct.

What is the user experience going to be?

Users and maintainers have one standardized error struct to look at which makes it easier to read and handle.

Are there breaking changes to the User Experience?

Error responses will look different for some plugins but it will be simpler with it.

Why should it be built? Any reason not to?

Without it, each plugin can have a different error response which could lead to confusion and user opening issues in the future.
It would require lib maintainers to write code for each plugin as the responses are differently.

What will it take to execute?

As it is a standard, plugins are a bit limited in the way how they report errors.

Any remaining open questions?

Nope