From bf05c600c4822ef049443e6469fed3a6f7e4347a Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 19 Mar 2018 10:52:50 -0400 Subject: [PATCH] REST: Include suppressed exceptions on failures (#29115) This modifies xcontent serialization of Exceptions to contain suppressed exceptions. If there are any suppressed exceptions they are included in the exception response by default. The reasoning here is that they are fairly rare but when they exist they almost always add extra useful information. Take, for example, the response when you specify two broken ingest pipelines: ``` { "error" : { "root_cause" : ...snip... "type" : "parse_exception", "reason" : "[field] required property is missing", "header" : { "processor_type" : "set", "property_name" : "field" }, "suppressed" : [ { "type" : "parse_exception", "reason" : "[field] required property is missing", "header" : { "processor_type" : "convert", "property_name" : "field" } } ] }, "status" : 400 } ``` Moreover, when suppressed exceptions come from 500 level errors should give us more useful debugging information. Closes #23392 --- .../elasticsearch/ElasticsearchException.java | 21 ++++++++++++++++ .../ElasticsearchExceptionTests.java | 24 ++++++++++++++++--- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index bfa37808402..b1c02c4ac27 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -23,6 +23,7 @@ import org.elasticsearch.action.support.replication.ReplicationOperation; import org.elasticsearch.cluster.action.shard.ShardStateAction; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -85,6 +86,7 @@ public class ElasticsearchException extends RuntimeException implements ToXConte private static final String TYPE = "type"; private static final String REASON = "reason"; private static final String CAUSED_BY = "caused_by"; + private static final ParseField SUPPRESSED = new ParseField("suppressed"); private static final String STACK_TRACE = "stack_trace"; private static final String HEADER = "header"; private static final String ERROR = "error"; @@ -372,6 +374,17 @@ public class ElasticsearchException extends RuntimeException implements ToXConte if (params.paramAsBoolean(REST_EXCEPTION_SKIP_STACK_TRACE, REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT) == false) { builder.field(STACK_TRACE, ExceptionsHelper.stackTrace(throwable)); } + + Throwable[] allSuppressed = throwable.getSuppressed(); + if (allSuppressed.length > 0) { + builder.startArray(SUPPRESSED.getPreferredName()); + for (Throwable suppressed : allSuppressed) { + builder.startObject(); + generateThrowableXContent(builder, params, suppressed); + builder.endObject(); + } + builder.endArray(); + } } private static void headerToXContent(XContentBuilder builder, String key, List values) throws IOException { @@ -416,6 +429,7 @@ public class ElasticsearchException extends RuntimeException implements ToXConte Map> metadata = new HashMap<>(); Map> headers = new HashMap<>(); List rootCauses = new ArrayList<>(); + List suppressed = new ArrayList<>(); for (; token == XContentParser.Token.FIELD_NAME; token = parser.nextToken()) { String currentFieldName = parser.currentName(); @@ -467,6 +481,10 @@ public class ElasticsearchException extends RuntimeException implements ToXConte while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { rootCauses.add(fromXContent(parser)); } + } else if (SUPPRESSED.match(currentFieldName, parser.getDeprecationHandler())) { + while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { + suppressed.add(fromXContent(parser)); + } } else { // Parse the array and add each item to the corresponding list of metadata. // Arrays of objects are not supported yet and just ignored and skipped. @@ -507,6 +525,9 @@ public class ElasticsearchException extends RuntimeException implements ToXConte for (ElasticsearchException rootCause : rootCauses) { e.addSuppressed(rootCause); } + for (ElasticsearchException s : suppressed) { + e.addSuppressed(s); + } return e; } diff --git a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index 4c095efbbf8..d3560fc6db3 100644 --- a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -35,7 +35,6 @@ import org.elasticsearch.common.breaker.CircuitBreakingException; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; -import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -77,7 +76,6 @@ import static java.util.Collections.singletonList; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.CoreMatchers.hasItems; -import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.startsWith; @@ -602,6 +600,13 @@ public class ElasticsearchExceptionTests extends ESTestCase { final Tuple exceptions = randomExceptions(); final Throwable throwable = exceptions.v1(); + final ElasticsearchException expected = exceptions.v2(); + int suppressedCount = randomBoolean() ? 0 : between(1, 5); + for (int i = 0; i < suppressedCount; i++) { + final Tuple suppressed = randomExceptions(); + throwable.addSuppressed(suppressed.v1()); + expected.addSuppressed(suppressed.v2()); + } BytesReference throwableBytes = toShuffledXContent((builder, params) -> { ElasticsearchException.generateThrowableXContent(builder, params, throwable); @@ -615,7 +620,20 @@ public class ElasticsearchExceptionTests extends ESTestCase { assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken()); assertNull(parser.nextToken()); } - assertDeepEquals(exceptions.v2(), parsedException); + assertDeepEquals(expected, parsedException); + + if (suppressedCount > 0) { + XContentBuilder builder = XContentBuilder.builder(xContent); + builder.startObject(); + ElasticsearchException.generateThrowableXContent(builder, ToXContent.EMPTY_PARAMS, throwable); + builder.endObject(); + throwableBytes = BytesReference.bytes(builder); + try (XContentParser parser = createParser(xContent, throwableBytes)) { + assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); + List keys = new ArrayList<>(parser.mapOrdered().keySet()); + assertEquals("last index should be [suppressed]", "suppressed", keys.get(keys.size() - 1)); + } + } } public void testUnknownFailureToAndFromXContent() throws IOException {