Consolidate the way we expose additional information on serialized Exceptions on the rest layer

[Mpdreamz]

When a request results in an exception it is presented back to the user as:

{
  "error" : {
    "root_cause" : [
      {
        "type" : "action_request_validation_exception",
        "reason" : "Validation Failed: 1: reindex cannot write into an index its reading from [nest-9aced9c5];"
      }
    ],
    "type" : "action_request_validation_exception",
    "reason" : "Validation Failed: 1: reindex cannot write into an index its reading from [nest-9aced9c5];"
  },
  "status" : 400
}

Notes:

• Additional inner exceptions can end up in caused_by to N levels deep. These nested errors will never have a root_cause set.
• An optional stack_trace string can be returned when error_trace=true is specified on the url.

So far this is very straight forward, there are however 3 different ways additional properties can end up under error.

1. Headers

returned as headers object on error, using addHeader() on ElasticsearchException

Ingest newCompoundProcessorException()

• processor_type: string
• processor_tag: string

Ingest addHeadersToException() (ConfigurationUtil clas)

• processor_type: string
• processor_tag: string
• property_name: string

Security Token Service

• WWW-Authenticate : string (Bearer).

These headers are also presented as HTTP Headers, are these duplicated here for clients that can not access HTTP Headers? Do we really need the Ingest related headers?

2. Exceptions addMetadata()

returned as additional properties on error directly, using addMetadata()

LicenseUItils.newComplianceException

• license.expired.feature: string

ElasticsearchException.setIndex

• index: string
• index_uuid: string

ElasticsearchException.setResources

• resource.type: string (could only find the following types: "index_expression", "index_or_alias", "aliases")
• resource.id: string or string[]

Not entirely sure what constitutes a resource but this reads like a normalization attempt at additional data's keys does anyone recall the idea behind this?

ElasticsearchException.setShard

• shard: string (int to string)

3a. Exceptions overriding metaDataToXContent()

SearchParseException

• line : int
• col : int;

ParsingException

• line : int
• col : int;

CircuitBreakException

• bytes_wanted : long
• bytes_limit : long;

ScriptException

• script_stack : string or string[]
• script : string;
• lang : string;

SearchPhaseException

• phase : string
• grouped : bool (controlled by group_shard_failures dfaults to true);
• failed_shards : ShardOperationFailedException[] (controlled by group_shard_failures dfaults to true);

3b. ShardOperationFailedException.toXContent

ShardOperationFailedException

implements XContent so they also add additional fields to the nested ElasticsearchException under failed_shards

DefaultShardOperationFailedException

• shard: int (note that this conflicts with setShard() on ElasticsearchException
• index: string
• status: string (RestStatus.name)
• reason: ElasticsearchException

^ I think it would make sense if we include type and rename reason to caused_by so that it can be parsed as any
other ElasticsearchException

ShardSearchFailure

• shard: int (note that this conflicts with setShard() on ElasticsearchException
• index: string
• node: string
• reason: ElasticsearchException

reason => caused_by

SnapshotShardFailure

• index: string
• index_uuid: string (returns getIndexName() just like index ?)
• shard_id: int
• reason: string
• node_id: string
• status: string (RestStatus.name)

shard_id here should really be shard for consistency
reason is a string here but ElasticsearchException in other ShardOperationFailedException implementations

ReplicationResponse.ShardInfo.Failure

• _index: string
• _shard: string
• _node: string
• primary: bool
• status: string (RestStatus.name)
• reason: ElasticsearchException

IndicesShardStoresResponse.Failure

• node: string

^ Inherits from DefaultsShardOperationFailedException and calls its toXContent

Note we have now three different ways to serialize node id's (_node, node and node_id). Same goes for shard ids.

I think the latter three are never part of failed_shards on SearchPhaseException so it might make sense to introduce a seperate SearchShardOperationFailedException interface.

4. Failures

Several api's return a collection failures that maps to elasticsearch's Failure java class which currently writes exceptions as:

• index: string
• type: string
• id: bool
• status: string (RestStatus.name)
• cause: ElasticsearchException

Note that the cuase property prevents this class from being a superset of ElasticsearchException deserialization which uses caused_by. It'd be good to normalize these.

[Mpdreamz]

error is also sometimes returned as a string. as can be seen when getting an alias that does not exists e.g GET /_alias/unknown-alias returns:

{"error":"alias [unknown-alias] missing","status":404}

The way NEST currently deals with this is to map it as it if it were returned as:

{
    "errror" : { 
        "reason" : "alias [unknown-alias] missing"
     }
}

[javanna]

Possibly related to #22928

[DaveCTurner]

We discussed this in FixItFriday and were generally positive on the idea. I broke this down to #27724, #27728 and #27729, and #22928 is a pre-existing task that takes us in the right direction.

There are a number of different Failure classes and I'm not sure which of them the fourth point refers to. @Mpdreamz could you clarify?

[nik9000]

@Mpdreamz could you clarify?

[Mpdreamz]

The 4th point is about the appearant similarity of all Failure classes and Exception but the listed properties prevent them from being the same thing conceptually or at least be represented on the REST layer the same conceptually.