-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Consolidate the way we expose additional information on serialized Exceptions on the rest layer #27672
Description
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_byto N levels deep. These nested errors will never have aroot_causeset. - An optional
stack_tracestring can be returned whenerror_trace=trueis 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: stringindex_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: intcol: int;
ParsingException
line: intcol: int;
CircuitBreakException
bytes_wanted: longbytes_limit: long;
ScriptException
script_stack: string or string[]script: string;lang: string;
SearchPhaseException
phase: stringgrouped: 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 withsetShard()onElasticsearchExceptionindex: stringstatus: 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 withsetShard()onElasticsearchExceptionindex: stringnode: stringreason: ElasticsearchException
reason => caused_by
SnapshotShardFailure
index: stringindex_uuid: string (returns getIndexName() just likeindex?)shard_id: intreason: stringnode_id: stringstatus: 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: stringprimary: boolstatus: 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: stringtype: stringid: boolstatus: 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.