honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Core: Pick inner most parse exception as root cause (#30270) 

Just like `ElasticsearchException`, the inner most
`XContentParseException` tends to contain the root cause of the
exception and show be show to the user in the `root_cause` field.

The effectively undoes most of the changes that #29373 made to the
`root_cause` for parsing exceptions. The `type` field still changes from
`parse_exception` to `x_content_parse_exception`, but this seems like a
fairly safe change.

`ElasticsearchWrapperException` *looks* tempting to implement this but
the behavior isn't quite right. `ElasticsearchWrapperExceptions` are
entirely unwrapped until the cause no longer
`implements ElasticsearchWrapperException` but `XContentParseException`
should be unwrapped until its cause is no longer an
`XContentParseException` but no further. In other words,
`ElasticsearchWrapperException` are unwrapped one step too far.

Closes #30261
This commit is contained in:
Nik Everett 2018-05-01 07:44:58 -04:00 committed by GitHub
parent b9e1860f36
commit 99b98fab18
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 58 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
server/src/main/java/org/elasticsearch/ElasticsearchException.java
View File

@ -31,6 +31,7 @@ import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.LoggerMessageFormat;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParseException;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.shard.ShardId;
@ -635,8 +636,25 @@ public class ElasticsearchException extends RuntimeException implements ToXConte
public static ElasticsearchException[] guessRootCauses(Throwable t) {
Throwable ex = ExceptionsHelper.unwrapCause(t);
if (ex instanceof ElasticsearchException) {
// ElasticsearchException knows how to guess its own root cause
return ((ElasticsearchException) ex).guessRootCauses();
}
if (ex instanceof XContentParseException) {
/*
* We'd like to unwrap parsing exceptions to the inner-most
* parsing exception because that is generally the most interesting
* exception to return to the user. If that exception is caused by
* an ElasticsearchException we'd like to keep unwrapping because
* ElasticserachExceptions tend to contain useful information for
* the user.
*/
Throwable cause = ex.getCause();
if (cause != null) {
if (cause instanceof XContentParseException || cause instanceof ElasticsearchException) {
return guessRootCauses(ex.getCause());
}
}
}
return new ElasticsearchException[]{new ElasticsearchException(t.getMessage(), t) {
@Override
protected String getExceptionName() {

6
server/src/main/java/org/elasticsearch/ElasticsearchWrapperException.java
View File

@ -19,7 +19,11 @@
package org.elasticsearch;
/**
* An exception that is meant to be "unwrapped" when sent back to the user
* as an error because its is {@link #getCause() cause}, if non-null is
* <strong>always</strong> more useful to the user than the exception itself.
*/
public interface ElasticsearchWrapperException {
Throwable getCause();
}

48
server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java
View File

@ -41,6 +41,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentLocation;
import org.elasticsearch.common.xcontent.XContentParseException;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.discovery.DiscoverySettings;
@ -78,6 +79,7 @@ import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.CoreMatchers.hasItems;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.startsWith;
public class ElasticsearchExceptionTests extends ESTestCase {
@ -124,13 +126,13 @@ public class ElasticsearchExceptionTests extends ESTestCase {
} else {
rootCauses = ElasticsearchException.guessRootCauses(randomBoolean() ? new RemoteTransportException("remoteboom", ex) : ex);
}
assertEquals(ElasticsearchException.getExceptionName(rootCauses[0]), "parsing_exception");
assertEquals(rootCauses[0].getMessage(), "foobar");
assertEquals("parsing_exception", ElasticsearchException.getExceptionName(rootCauses[0]));
assertEquals("foobar", rootCauses[0].getMessage());
ElasticsearchException oneLevel = new ElasticsearchException("foo", new RuntimeException("foobar"));
rootCauses = oneLevel.guessRootCauses();
assertEquals(ElasticsearchException.getExceptionName(rootCauses[0]), "exception");
assertEquals(rootCauses[0].getMessage(), "foo");
assertEquals("exception", ElasticsearchException.getExceptionName(rootCauses[0]));
assertEquals("foo", rootCauses[0].getMessage());
}
{
ShardSearchFailure failure = new ShardSearchFailure(
@ -146,20 +148,40 @@ public class ElasticsearchExceptionTests extends ESTestCase {
assertEquals(rootCauses.length, 2);
assertEquals(ElasticsearchException.getExceptionName(rootCauses[0]), "parsing_exception");
assertEquals(rootCauses[0].getMessage(), "foobar");
assertEquals(((ParsingException) rootCauses[0]).getLineNumber(), 1);
assertEquals(((ParsingException) rootCauses[0]).getColumnNumber(), 2);
assertEquals(ElasticsearchException.getExceptionName(rootCauses[1]), "query_shard_exception");
assertEquals((rootCauses[1]).getIndex().getName(), "foo1");
assertEquals(rootCauses[1].getMessage(), "foobar");
assertEquals(1, ((ParsingException) rootCauses[0]).getLineNumber());
assertEquals(2, ((ParsingException) rootCauses[0]).getColumnNumber());
assertEquals("query_shard_exception", ElasticsearchException.getExceptionName(rootCauses[1]));
assertEquals("foo1", rootCauses[1].getIndex().getName());
assertEquals("foobar", rootCauses[1].getMessage());
}
{
final ElasticsearchException[] foobars = ElasticsearchException.guessRootCauses(new IllegalArgumentException("foobar"));
assertEquals(foobars.length, 1);
assertTrue(foobars[0] instanceof ElasticsearchException);
assertEquals(foobars[0].getMessage(), "foobar");
assertEquals(foobars[0].getCause().getClass(), IllegalArgumentException.class);
assertEquals(foobars[0].getExceptionName(), "illegal_argument_exception");
assertThat(foobars[0], instanceOf(ElasticsearchException.class));
assertEquals("foobar", foobars[0].getMessage());
assertEquals(IllegalArgumentException.class, foobars[0].getCause().getClass());
assertEquals("illegal_argument_exception", foobars[0].getExceptionName());
}
{
XContentParseException inner = new XContentParseException(null, "inner");
XContentParseException outer = new XContentParseException(null, "outer", inner);
final ElasticsearchException[] causes = ElasticsearchException.guessRootCauses(outer);
assertEquals(causes.length, 1);
assertThat(causes[0], instanceOf(ElasticsearchException.class));
assertEquals("inner", causes[0].getMessage());
assertEquals("x_content_parse_exception", causes[0].getExceptionName());
}
{
ElasticsearchException inner = new ElasticsearchException("inner");
XContentParseException outer = new XContentParseException(null, "outer", inner);
final ElasticsearchException[] causes = ElasticsearchException.guessRootCauses(outer);
assertEquals(causes.length, 1);
assertThat(causes[0], instanceOf(ElasticsearchException.class));
assertEquals("inner", causes[0].getMessage());
assertEquals("exception", causes[0].getExceptionName());
}
}