Implement toXContent on ShardOpertionFailureException

ShardOperationFailureException implementations alread provide structured exception support but it's not yet exposed on the interface. This change allows nice rendering of structured REST exceptions also if searches fail on only a subset of the shards etc. Closes elastic#11017
s1monw · May 18, 2015 · 8b8ba9a · 8b8ba9a
1 parent f7696ec
commit 8b8ba9a
Show file tree

Hide file tree

Showing 27 changed files with 181 additions and 129 deletions.
diff --git a/src/main/java/org/elasticsearch/ElasticsearchException.java b/src/main/java/org/elasticsearch/ElasticsearchException.java
@@ -22,16 +22,18 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import org.elasticsearch.action.ShardOperationFailedException;
 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.index.Index;
+import org.elasticsearch.index.IndexException;
 import org.elasticsearch.rest.HasRestHeaders;
 import org.elasticsearch.rest.RestStatus;
 
 import java.io.IOException;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 /**
  * A base class for all elasticsearch exceptions.
@@ -288,6 +290,4 @@ public static String getExceptionName(Throwable ex) {
     public String toString() {
         return ExceptionsHelper.detailedMessage(this).trim();
     }
-
-
 }
diff --git a/src/main/java/org/elasticsearch/ExceptionsHelper.java b/src/main/java/org/elasticsearch/ExceptionsHelper.java
@@ -22,15 +22,21 @@
 import org.apache.lucene.index.CorruptIndexException;
 import org.apache.lucene.index.IndexFormatTooNewException;
 import org.apache.lucene.index.IndexFormatTooOldException;
+import org.elasticsearch.action.ShardOperationFailedException;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.Loggers;
+import org.elasticsearch.index.Index;
+import org.elasticsearch.index.IndexException;
 import org.elasticsearch.rest.RestStatus;
 
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 /**
  *
@@ -214,4 +220,59 @@ public static boolean reThrowIfNotNull(@Nullable Throwable e) {
         }
         return true;
     }
+
+
+    /**
+     * Deduplicate the failures by exception message and index.
+     */
+    public static ShardOperationFailedException[] groupBy(ShardOperationFailedException[] failures) {
+        List<ShardOperationFailedException> uniqueFailures = new ArrayList<>();
+        Set<GroupBy> reasons = new HashSet<>();
+        for (ShardOperationFailedException failure : failures) {
+            GroupBy reason = new GroupBy(failure.getCause());
+            if (reasons.contains(reason) == false) {
+                reasons.add(reason);
+                uniqueFailures.add(failure);
+            }
+        }
+        return uniqueFailures.toArray(new ShardOperationFailedException[0]);
+    }
+
+    static class GroupBy {
+        final String reason;
+        final Index index;
+        final Class<? extends Throwable> causeType;
+
+        public GroupBy(Throwable t) {
+            if (t instanceof IndexException) {
+                index = ((IndexException) t).index();
+            } else {
+                index = null;
+            }
+            reason = t.getMessage();
+            causeType = t.getClass();
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            GroupBy groupBy = (GroupBy) o;
+
+            if (!causeType.equals(groupBy.causeType)) return false;
+            if (index != null ? !index.equals(groupBy.index) : groupBy.index != null) return false;
+            if (reason != null ? !reason.equals(groupBy.reason) : groupBy.reason != null) return false;
+
+            return true;
+        }
+
+        @Override
+        public int hashCode() {
+            int result = reason != null ? reason.hashCode() : 0;
+            result = 31 * result + (index != null ? index.hashCode() : 0);
+            result = 31 * result + causeType.hashCode();
+            return result;
+        }
+    }
 }
diff --git a/src/main/java/org/elasticsearch/action/ActionWriteResponse.java b/src/main/java/org/elasticsearch/action/ActionWriteResponse.java
@@ -231,6 +231,11 @@ public RestStatus status() {
                 return status;
             }
 
+            @Override
+            public Throwable getCause() {
+                return cause;
+            }
+
             /**
              * @return Whether this failure occurred on a primary shard.
              * (this only reports true for delete by query)

diff --git a/src/main/java/org/elasticsearch/action/ShardOperationFailedException.java b/src/main/java/org/elasticsearch/action/ShardOperationFailedException.java
@@ -20,16 +20,23 @@
 package org.elasticsearch.action;
 
 import org.elasticsearch.common.io.stream.Streamable;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.index.Index;
+import org.elasticsearch.index.IndexException;
 import org.elasticsearch.rest.RestStatus;
 
 import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
 
 /**
  * An exception indicating that a failure occurred performing an operation on the shard.
  *
  *
  */
-public interface ShardOperationFailedException extends Streamable, Serializable {
+public interface ShardOperationFailedException extends Streamable, Serializable, ToXContent {
 
     /**
      * The index the operation failed on. Might return <tt>null</tt> if it can't be derived.
@@ -50,4 +57,9 @@ public interface ShardOperationFailedException extends Streamable, Serializable
      * The status of the failure.
      */
     RestStatus status();
+
+    /**
+     * The cause of this failure
+     */
+    Throwable getCause();
 }
diff --git a/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java b/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.bulk;
 
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.action.ActionWriteResponse;
 import org.elasticsearch.action.delete.DeleteResponse;
@@ -27,6 +28,7 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Streamable;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.RestStatus;
 
 import java.io.IOException;
@@ -44,26 +46,17 @@ public static class Failure {
         private final String index;
         private final String type;
         private final String id;
-        private final String message;
+        private final Throwable cause;
         private final RestStatus status;
 
         public Failure(String index, String type, String id, Throwable t) {
             this.index = index;
             this.type = type;
             this.id = id;
-            this.message = t.toString();
+            this.cause = t;
             this.status = ExceptionsHelper.status(t);
         }
 
-
-        public Failure(String index, String type, String id, String message, RestStatus status) {
-            this.index = index;
-            this.type = type;
-            this.id = id;
-            this.message = message;
-            this.status = status;
-        }
-
         /**
          * The index name of the action.
          */
@@ -89,7 +82,7 @@ public String getId() {
          * The failure message.
          */
         public String getMessage() {
-            return this.message;
+            return this.cause.toString();
         }
 
         /**
@@ -98,6 +91,10 @@ public String getMessage() {
         public RestStatus getStatus() {
             return this.status;
         }
+
+        public Throwable getCause() {
+            return cause;
+        }
     }
 
     private int id;
@@ -265,9 +262,8 @@ public void readFrom(StreamInput in) throws IOException {
             String fIndex = in.readString();
             String fType = in.readString();
             String fId = in.readOptionalString();
-            String fMessage = in.readString();
-            RestStatus status = RestStatus.readFrom(in);
-            failure = new Failure(fIndex, fType, fId, fMessage, status);
+            Throwable throwable = in.readThrowable();
+            failure = new Failure(fIndex, fType, fId, throwable);
         }
     }
 
@@ -295,8 +291,7 @@ public void writeTo(StreamOutput out) throws IOException {
             out.writeString(failure.getIndex());
             out.writeString(failure.getType());
             out.writeOptionalString(failure.getId());
-            out.writeString(failure.getMessage());
-            RestStatus.writeTo(out, failure.getStatus());
+            out.writeThrowable(failure.getCause());
         }
     }
 }
diff --git a/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java b/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java
@@ -284,7 +284,7 @@ private void executeBulk(final BulkRequest bulkRequest, final long startTime, fi
                 MappingMetaData mappingMd = clusterState.metaData().index(concreteIndex).mappingOrDefault(updateRequest.type());
                 if (mappingMd != null && mappingMd.routing().required() && updateRequest.routing() == null) {
                     BulkItemResponse.Failure failure = new BulkItemResponse.Failure(updateRequest.index(), updateRequest.type(),
-                            updateRequest.id(), "routing is required for this item", RestStatus.BAD_REQUEST);
+                            updateRequest.id(), new IllegalArgumentException("routing is required for this item"));
                     responses.set(i, new BulkItemResponse(i, updateRequest.type(), failure));
                     continue;
                 }
@@ -328,21 +328,19 @@ public void onResponse(BulkShardResponse bulkShardResponse) {
                 @Override
                 public void onFailure(Throwable e) {
                     // create failures for all relevant requests
-                    String message = ExceptionsHelper.detailedMessage(e);
-                    RestStatus status = ExceptionsHelper.status(e);
                     for (BulkItemRequest request : requests) {
                         if (request.request() instanceof IndexRequest) {
                             IndexRequest indexRequest = (IndexRequest) request.request();
                             responses.set(request.id(), new BulkItemResponse(request.id(), indexRequest.opType().toString().toLowerCase(Locale.ENGLISH),
-                                    new BulkItemResponse.Failure(concreteIndices.getConcreteIndex(indexRequest.index()), indexRequest.type(), indexRequest.id(), message, status)));
+                                    new BulkItemResponse.Failure(concreteIndices.getConcreteIndex(indexRequest.index()), indexRequest.type(), indexRequest.id(), e)));
                         } else if (request.request() instanceof DeleteRequest) {
                             DeleteRequest deleteRequest = (DeleteRequest) request.request();
                             responses.set(request.id(), new BulkItemResponse(request.id(), "delete",
-                                    new BulkItemResponse.Failure(concreteIndices.getConcreteIndex(deleteRequest.index()), deleteRequest.type(), deleteRequest.id(), message, status)));
+                                    new BulkItemResponse.Failure(concreteIndices.getConcreteIndex(deleteRequest.index()), deleteRequest.type(), deleteRequest.id(), e)));
                         } else if (request.request() instanceof UpdateRequest) {
                             UpdateRequest updateRequest = (UpdateRequest) request.request();
                             responses.set(request.id(), new BulkItemResponse(request.id(), "update",
-                                    new BulkItemResponse.Failure(concreteIndices.getConcreteIndex(updateRequest.index()), updateRequest.type(), updateRequest.id(), message, status)));
+                                    new BulkItemResponse.Failure(concreteIndices.getConcreteIndex(updateRequest.index()), updateRequest.type(), updateRequest.id(), e)));
                         }
                     }
                     if (counter.decrementAndGet() == 0) {

diff --git a/src/main/java/org/elasticsearch/action/percolate/PercolateResponse.java b/src/main/java/org/elasticsearch/action/percolate/PercolateResponse.java
@@ -116,7 +116,7 @@ public Iterator<Match> iterator() {
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.field(Fields.TOOK, tookInMillis);
-        RestActions.buildBroadcastShardsHeader(builder, this);
+        RestActions.buildBroadcastShardsHeader(builder, params, this);
 
         builder.field(Fields.TOTAL, count);
         if (matches != null) {

diff --git a/src/main/java/org/elasticsearch/action/search/SearchPhaseExecutionException.java b/src/main/java/org/elasticsearch/action/search/SearchPhaseExecutionException.java
@@ -20,6 +20,8 @@
 package org.elasticsearch.action.search;
 
 import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.ExceptionsHelper;
+import org.elasticsearch.action.ShardOperationFailedException;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexException;
@@ -92,8 +94,8 @@ protected void innerToXContent(XContentBuilder builder, Params params) throws IO
         builder.field("grouped", group); // notify that it's grouped
         builder.field("failed_shards");
         builder.startArray();
-        ShardSearchFailure[] failures = params.paramAsBoolean("group_shard_failures", true) ? groupBy(shardFailures) : shardFailures;
-        for (ShardSearchFailure failure : failures) {
+        ShardOperationFailedException[] failures = params.paramAsBoolean("group_shard_failures", true) ? ExceptionsHelper.groupBy(shardFailures) : shardFailures;
+        for (ShardOperationFailedException failure : failures) {
             builder.startObject();
             failure.toXContent(builder, params);
             builder.endObject();
@@ -103,25 +105,11 @@ protected void innerToXContent(XContentBuilder builder, Params params) throws IO
 
     }
 
-    private ShardSearchFailure[] groupBy(ShardSearchFailure[] failures) {
-        List<ShardSearchFailure> uniqueFailures = new ArrayList<>();
-        Set<GroupBy> reasons = new HashSet<>();
-        for (ShardSearchFailure failure : failures) {
-            GroupBy reason = new GroupBy(failure.getCause());
-            if (reasons.contains(reason) == false) {
-                reasons.add(reason);
-                uniqueFailures.add(failure);
-            }
-        }
-        return uniqueFailures.toArray(new ShardSearchFailure[0]);
-
-    }
-
     @Override
     public ElasticsearchException[] guessRootCauses() {
-        ShardSearchFailure[] failures = groupBy(shardFailures);
+        ShardOperationFailedException[] failures = ExceptionsHelper.groupBy(shardFailures);
         List<ElasticsearchException> rootCauses = new ArrayList<>(failures.length);
-        for (ShardSearchFailure failure : failures) {
+        for (ShardOperationFailedException failure : failures) {
             ElasticsearchException[] guessRootCauses = ElasticsearchException.guessRootCauses(failure.getCause());
             rootCauses.addAll(Arrays.asList(guessRootCauses));
         }
@@ -132,42 +120,4 @@ public ElasticsearchException[] guessRootCauses() {
     public String toString() {
         return buildMessage(phaseName, getMessage(), shardFailures);
     }
-
-    static class GroupBy {
-        final String reason;
-        final Index index;
-        final Class<? extends Throwable> causeType;
-
-        public GroupBy(Throwable t) {
-            if (t instanceof IndexException) {
-                index = ((IndexException) t).index();
-            } else {
-                index = null;
-            }
-            reason = t.getMessage();
-            causeType = t.getClass();
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            if (this == o) return true;
-            if (o == null || getClass() != o.getClass()) return false;
-
-            GroupBy groupBy = (GroupBy) o;
-
-            if (!causeType.equals(groupBy.causeType)) return false;
-            if (index != null ? !index.equals(groupBy.index) : groupBy.index != null) return false;
-            if (reason != null ? !reason.equals(groupBy.reason) : groupBy.reason != null) return false;
-
-            return true;
-        }
-
-        @Override
-        public int hashCode() {
-            int result = reason != null ? reason.hashCode() : 0;
-            result = 31 * result + (index != null ? index.hashCode() : 0);
-            result = 31 * result + causeType.hashCode();
-            return result;
-        }
-    }
 }
diff --git a/src/main/java/org/elasticsearch/action/search/SearchResponse.java b/src/main/java/org/elasticsearch/action/search/SearchResponse.java
@@ -177,7 +177,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         if (isTerminatedEarly() != null) {
             builder.field(Fields.TERMINATED_EARLY, isTerminatedEarly());
         }
-        RestActions.buildBroadcastShardsHeader(builder, getTotalShards(), getSuccessfulShards(), getFailedShards(), getShardFailures());
+        RestActions.buildBroadcastShardsHeader(builder, params, getTotalShards(), getSuccessfulShards(), getFailedShards(), getShardFailures());
         internalResponse.toXContent(builder, params);
         return builder;
     }