Browse Source

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
Nik Everett 7 years ago
parent
commit
bf05c600c4

+ 21 - 0
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<String> values) throws IOException {
@@ -416,6 +429,7 @@ public class ElasticsearchException extends RuntimeException implements ToXConte
         Map<String, List<String>> metadata = new HashMap<>();
         Map<String, List<String>> headers = new HashMap<>();
         List<ElasticsearchException> rootCauses = new ArrayList<>();
+        List<ElasticsearchException> 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;
     }
 

+ 21 - 3
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<Throwable, ElasticsearchException> 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<Throwable, ElasticsearchException> 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<String> keys = new ArrayList<>(parser.mapOrdered().keySet());
+                assertEquals("last index should be [suppressed]", "suppressed", keys.get(keys.size() - 1));
+            }
+        }
     }
 
     public void testUnknownFailureToAndFromXContent() throws IOException {