Skip to content

Commit

Permalink
[REST] Render REST errors in a structural way
Browse files Browse the repository at this point in the history
This commit adds support for structural errors / failures / exceptions
on the elasticsearch REST layer. Exceptions are rendering with at least
a `type` and a `reason` corresponding to the exception name and the message.
Some expcetions like the ones associated with an index or a shard will have
additional information about the index the exception was triggered on or the
shard respectivly.

Each rendered response will also contain a list of root causes which is a list
of distinct shard level errors returned for the request. Root causes are the lowest
level elasticsearch exception found per shard response and are intended to be displayed
to the user to indicate the soruce of the exception.

Shard level response are by-default grouped by their type and reason to reduce the amount
of duplicates retunred. Yet, the same exception retunred from different indices will not be
grouped.

Closes #3303
  • Loading branch information
s1monw committed Apr 24, 2015
1 parent c9d7243 commit 15d58d9
Show file tree
Hide file tree
Showing 66 changed files with 737 additions and 275 deletions.
2 changes: 1 addition & 1 deletion rest-api-spec/test/create/50_parent.yaml
Expand Up @@ -14,7 +14,7 @@
wait_for_status: yellow

- do:
catch: /RoutingMissingException/
catch: /routing_missing_exception/
create:
index: test_1
type: test
Expand Down
2 changes: 1 addition & 1 deletion rest-api-spec/test/create/75_ttl.yaml
Expand Up @@ -89,7 +89,7 @@
type: test
id: 1
- do:
catch: /AlreadyExpiredException/
catch: /already_expired_exception/
create:
index: test_1
type: test
Expand Down
2 changes: 1 addition & 1 deletion rest-api-spec/test/delete/42_missing_parent.yml
Expand Up @@ -21,7 +21,7 @@
body: { foo: bar }

- do:
catch: /RoutingMissingException/
catch: /routing_missing_exception/
delete:
index: test_1
type: test
Expand Down
2 changes: 1 addition & 1 deletion rest-api-spec/test/index/50_parent.yaml
Expand Up @@ -13,7 +13,7 @@
wait_for_status: yellow

- do:
catch: /RoutingMissingException/
catch: /routing_missing_exception/
index:
index: test_1
type: test
Expand Down
2 changes: 1 addition & 1 deletion rest-api-spec/test/index/75_ttl.yaml
Expand Up @@ -74,7 +74,7 @@
# with timestamp

- do:
catch: /AlreadyExpiredException/
catch: /already_expired_exception/
index:
index: test_1
type: test
Expand Down
8 changes: 4 additions & 4 deletions rest-api-spec/test/mget/13_missing_metadata.yaml
Expand Up @@ -13,27 +13,27 @@
wait_for_status: yellow

- do:
catch: /ActionRequestValidationException.+ id is missing/
catch: /action_request_validation_exception.+ id is missing/
mget:
body:
docs:
- { _index: test_1, _type: test}

- do:
catch: /ActionRequestValidationException.+ index is missing/
catch: /action_request_validation_exception.+ index is missing/
mget:
body:
docs:
- { _type: test, _id: 1}

- do:
catch: /ActionRequestValidationException.+ no documents to get/
catch: /action_request_validation_exception.+ no documents to get/
mget:
body:
docs: []

- do:
catch: /ActionRequestValidationException.+ no documents to get/
catch: /action_request_validation_exception.+ no documents to get/
mget:
body: {}

Expand Down
4 changes: 2 additions & 2 deletions rest-api-spec/test/mget/15_ids.yaml
Expand Up @@ -59,14 +59,14 @@


- do:
catch: /ActionRequestValidationException.+ no documents to get/
catch: /action_request_validation_exception.+ no documents to get/
mget:
index: test_1
body:
ids: []

- do:
catch: /ActionRequestValidationException.+ no documents to get/
catch: /action_request_validation_exception.+ no documents to get/
mget:
index: test_1
body: {}
2 changes: 1 addition & 1 deletion rest-api-spec/test/mpercolate/10_basic.yaml
Expand Up @@ -37,5 +37,5 @@
foo: bar

- match: { responses.0.total: 1 }
- match: { responses.1.error: "IndexMissingException[[percolator_index1] missing]" }
- match: { responses.1.error: "/IndexMissingException.no.such.index./" }
- match: { responses.2.total: 1 }
2 changes: 1 addition & 1 deletion rest-api-spec/test/msearch/10_basic.yaml
Expand Up @@ -39,7 +39,7 @@
match: {foo: bar}

- match: { responses.0.hits.total: 3 }
- match: { responses.1.error: "IndexMissingException[[test_2] missing]" }
- match: { responses.1.error: "/IndexMissingException.no.such.index./" }
- match: { responses.2.hits.total: 1 }


4 changes: 2 additions & 2 deletions rest-api-spec/test/script/10_basic.yaml
Expand Up @@ -60,7 +60,7 @@


- do:
catch: /ElasticsearchIllegalArgumentException.Unable.to.parse.*/
catch: /Unable.to.parse.*/
put_script:
id: "1"
lang: "groovy"
Expand All @@ -74,7 +74,7 @@
body: { "script" : "_score * doc[\"myParent.weight\"].value" }

- do:
catch: /ElasticsearchIllegalArgumentException.script_lang.not.supported/
catch: /script_lang.not.supported/
put_script:
id: "1"
lang: "foobar"
Expand Down
2 changes: 1 addition & 1 deletion rest-api-spec/test/template/10_basic.yaml
Expand Up @@ -50,7 +50,7 @@
body: { "template": { "query": { "match{{}}_all": {}}, "size": "{{my_size}}" } }

- do:
catch: /ElasticsearchIllegalArgumentException\SUnable\sto\sparse.*/
catch: /Unable\sto\sparse.*/
put_template:
id: "1"
body: { "template": { "query": { "match{{}}_all": {}}, "size": "{{my_size}}" } }
2 changes: 1 addition & 1 deletion rest-api-spec/test/template/20_search.yaml
Expand Up @@ -37,7 +37,7 @@
- match: { hits.total: 1 }

- do:
catch: /ElasticsearchIllegalArgumentException.Unable.to.find.on.disk.script.simple1/
catch: /Unable.to.find.on.disk.script.simple1/
search_template:
body: { "template" : "simple1" }

2 changes: 1 addition & 1 deletion rest-api-spec/test/update/50_parent.yaml
Expand Up @@ -15,7 +15,7 @@ setup:
"Parent":

- do:
catch: /RoutingMissingException/
catch: /routing_missing_exception/
update:
index: test_1
type: test
Expand Down
2 changes: 1 addition & 1 deletion rest-api-spec/test/update/75_ttl.yaml
Expand Up @@ -81,7 +81,7 @@
# with timestamp

- do:
catch: /AlreadyExpiredException/
catch: /already_expired_exception/
index:
index: test_1
type: test
Expand Down
136 changes: 101 additions & 35 deletions src/main/java/org/elasticsearch/ElasticsearchException.java
Expand Up @@ -22,18 +22,23 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.rest.HasRestHeaders;
import org.elasticsearch.rest.RestStatus;

import java.io.IOException;
import java.util.List;
import java.util.Map;

/**
* A base class for all elasticsearch exceptions.
*/
public class ElasticsearchException extends RuntimeException {
public class ElasticsearchException extends RuntimeException implements ToXContent {

public static final String REST_EXCEPTION_SKIP_CAUSE = "rest.exception.skip_cause";

/**
* Construct a <code>ElasticsearchException</code> with the specified detail message.
Expand Down Expand Up @@ -62,12 +67,8 @@ public RestStatus status() {
Throwable cause = unwrapCause();
if (cause == this) {
return RestStatus.INTERNAL_SERVER_ERROR;
} else if (cause instanceof ElasticsearchException) {
return ((ElasticsearchException) cause).status();
} else if (cause instanceof IllegalArgumentException) {
return RestStatus.BAD_REQUEST;
} else {
return RestStatus.INTERNAL_SERVER_ERROR;
return ExceptionsHelper.status(cause);
}
}

Expand Down Expand Up @@ -114,19 +115,6 @@ public Throwable getRootCause() {
return rootCause;
}

/**
* Retrieve the most specific cause of this exception, that is,
* either the innermost cause (root cause) or this exception itself.
* <p>Differs from {@link #getRootCause()} in that it falls back
* to the present exception if there is no root cause.
*
* @return the most specific cause (never <code>null</code>)
*/
public Throwable getMostSpecificCause() {
Throwable rootCause = getRootCause();
return (rootCause != null ? rootCause : this);
}

/**
* Check whether this exception contains an exception of the given type:
* either it is of the given class itself or it contains a nested cause
Expand Down Expand Up @@ -175,21 +163,6 @@ public WithRestHeaders(String msg, Tuple<String, String[]>... headers) {
this.headers = headers(headers);
}

public WithRestHeaders(String msg, @Nullable ImmutableMap<String, List<String>> headers) {
super(msg);
this.headers = headers != null ? headers : ImmutableMap.<String, List<String>>of();
}

public WithRestHeaders(String msg, Throwable cause, Tuple<String, String[]>... headers) {
super(msg, cause);
this.headers = headers(headers);
}

public WithRestHeaders(String msg, Throwable cause, @Nullable ImmutableMap<String, List<String>> headers) {
super(msg, cause);
this.headers = headers != null ? headers : ImmutableMap.<String, List<String>>of();
}

@Override
public ImmutableMap<String, List<String>> getHeaders() {
return headers;
Expand All @@ -215,4 +188,97 @@ private static ImmutableMap<String, List<String>> headers(Tuple<String, String[]
return ImmutableMap.copyOf(map);
}
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
if (this instanceof ElasticsearchWrapperException) {
toXContent(builder, params, this);
} else {
builder.field("type", getExceptionName(this));
builder.field("reason", getMessage());
innerToXContent(builder, params);
}
return builder;
}

/**
* Renders additional per exception information into the xcontent
*/
protected void innerToXContent(XContentBuilder builder, Params params) throws IOException {
causeToXContent(builder, params);
}

/**
* Renders a cause exception as xcontent
*/
protected final void causeToXContent(XContentBuilder builder, Params params) throws IOException {
final Throwable cause = getCause();
if (cause != null && params.paramAsBoolean(REST_EXCEPTION_SKIP_CAUSE, false) == false) {
builder.field("caused_by");
builder.startObject();
toXContent(builder, params, cause);
builder.endObject();
}
}

/**
* Statis toXContent helper method that also renders non {@link org.elasticsearch.ElasticsearchException} instances as XContent.
*/
public static void toXContent(XContentBuilder builder, Params params, Throwable ex) throws IOException {
ex = ExceptionsHelper.unwrapCause(ex);
if (ex instanceof ElasticsearchException) {
((ElasticsearchException) ex).toXContent(builder, params);
} else {
builder.field("type", getExceptionName(ex));
builder.field("reason", ex.getMessage());
if (ex.getCause() != null) {
builder.field("caused_by");
builder.startObject();
toXContent(builder, params, ex.getCause());
builder.endObject();
}
}
}

/**
* Returns the root cause of this exception or mupltiple if different shards caused different exceptions
*/
public ElasticsearchException[] guessRootCauses() {
final Throwable cause = getCause();
if (cause != null && cause instanceof ElasticsearchException) {
return ((ElasticsearchException) cause).guessRootCauses();
}
return new ElasticsearchException[] {this};
}

/**
* Returns the root cause of this exception or mupltiple if different shards caused different exceptions.
* If the given exception is not an instance of {@link org.elasticsearch.ElasticsearchException} an empty array
* is returned.
*/
public static ElasticsearchException[] guessRootCauses(Throwable t) {
Throwable ex = ExceptionsHelper.unwrapCause(t);
if (ex instanceof ElasticsearchException) {
return ((ElasticsearchException) ex).guessRootCauses();
}
return new ElasticsearchException[0];
}

/**
* Returns a underscore case name for the given exception. This method strips <tt>Elasticsearch</tt> prefixes from exception names.
*/
public static String getExceptionName(Throwable ex) {
String simpleName = ex.getClass().getSimpleName();
if (simpleName.startsWith("Elasticsearch")) {
simpleName = simpleName.substring("Elasticsearch".length());
}
return Strings.toUnderscoreCase(simpleName);
}

@Override
public String toString() {
return ExceptionsHelper.detailedMessage(this).trim();
}


}
8 changes: 6 additions & 2 deletions src/main/java/org/elasticsearch/ExceptionsHelper.java
Expand Up @@ -54,8 +54,12 @@ public static ElasticsearchException convertToElastic(Throwable t) {
}

public static RestStatus status(Throwable t) {
if (t instanceof ElasticsearchException) {
return ((ElasticsearchException) t).status();
if (t != null) {
if (t instanceof ElasticsearchException) {
return ((ElasticsearchException) t).status();
} else if (t instanceof IllegalArgumentException) {
return RestStatus.BAD_REQUEST;
}
}
return RestStatus.INTERNAL_SERVER_ERROR;
}
Expand Down

0 comments on commit 15d58d9

Please sign in to comment.