Use status code 500 for errors if no shard failed (#88551)

If there is no shard failure we would like to use a generic
Internal Server Error other than a Service Unavailable.
Moreover, if the cause is known we use a status code that
reflects the cause.

Author: salvatore-campagna

docs/changelog/88551.yaml

@@ -0,0 +1,6 @@
+pr: 88551
+summary: "Fix: use status code 500 for aggregation reduce phase errors if no shard failed"
+area: Search
+type: bug
+issues:
+ - 20004

server/src/main/java/org/elasticsearch/action/search/ReduceSearchPhaseException.java

@@ -8,7 +8,9 @@
 
 package org.elasticsearch.action.search;
 
+import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.rest.RestStatus;
 
 import java.io.IOException;
 
@@ -27,4 +29,13 @@ public ReduceSearchPhaseException(String phaseName, String msg, Throwable cause,
     public ReduceSearchPhaseException(StreamInput in) throws IOException {
         super(in);
     }
+
+    @Override
+    public RestStatus status() {
+        final ShardSearchFailure[] shardFailures = shardFailures();
+        if (shardFailures.length == 0) {
+            return getCause() == null ? RestStatus.INTERNAL_SERVER_ERROR : ExceptionsHelper.status(getCause());
+        }
+        return super.status();
+    }
 }

server/src/main/java/org/elasticsearch/action/search/SearchPhaseExecutionException.java

@@ -75,7 +75,7 @@ public RestStatus status() {
         RestStatus status = shardFailures[0].status();
         if (shardFailures.length > 1) {
             for (int i = 1; i < shardFailures.length; i++) {
-                if (shardFailures[i].status().getStatus() >= 500) {
+                if (shardFailures[i].status().getStatus() >= RestStatus.INTERNAL_SERVER_ERROR.getStatus()) {
                     status = shardFailures[i].status();
                 }
             }

server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java

@@ -11,6 +11,7 @@
 import org.apache.lucene.util.Constants;
 import org.elasticsearch.action.NoShardAvailableActionException;
 import org.elasticsearch.action.RoutingMissingException;
+import org.elasticsearch.action.search.ReduceSearchPhaseException;
 import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.ShardSearchFailure;
 import org.elasticsearch.action.support.broadcast.BroadcastShardOperationFailedException;
@@ -202,6 +203,104 @@ public void testGuessRootCause() {
         }
     }
 
+    public void testReduceSearchPhaseExceptionWithNoShardFailuresAndNoCause() throws IOException {
+        final ReduceSearchPhaseException ex = new ReduceSearchPhaseException(
+            "search",
+            "no shard failure",
+            null,
+            ShardSearchFailure.EMPTY_ARRAY
+        );
+
+        XContentBuilder builder = XContentFactory.jsonBuilder();
+        builder.startObject();
+        ex.toXContent(builder, ToXContent.EMPTY_PARAMS);
+        builder.endObject();
+        String expected = """
+            {
+              "type": "reduce_search_phase_exception",
+              "reason": "[reduce] no shard failure",
+              "phase": "search",
+              "grouped": true,
+              "failed_shards": []
+            }""";
+        assertEquals(XContentHelper.stripWhitespace(expected), Strings.toString(builder));
+        assertEquals(RestStatus.INTERNAL_SERVER_ERROR.getStatus(), ex.status().getStatus());
+    }
+
+    public void testReduceSearchPhaseExceptionWithNoShardFailuresAndCause() throws IOException {
+        final ReduceSearchPhaseException ex = new ReduceSearchPhaseException(
+            "search",
+            "no shard failure",
+            new IllegalArgumentException("illegal argument"),
+            ShardSearchFailure.EMPTY_ARRAY
+        );
+
+        XContentBuilder builder = XContentFactory.jsonBuilder();
+        builder.startObject();
+        ex.toXContent(builder, ToXContent.EMPTY_PARAMS);
+        builder.endObject();
+        String expected = """
+            {
+              "type": "reduce_search_phase_exception",
+              "reason": "[reduce] no shard failure",
+              "phase": "search",
+              "grouped": true,
+              "failed_shards": [],
+              "caused_by":{"type":"illegal_argument_exception","reason":"illegal argument"}
+            }""";
+        assertEquals(XContentHelper.stripWhitespace(expected), Strings.toString(builder));
+        assertEquals(RestStatus.BAD_REQUEST.getStatus(), ex.status().getStatus());
+    }
+
+    public void testSearchPhaseExecutionExceptionWithNoShardFailuresAndNoCause() throws IOException {
+        final SearchPhaseExecutionException ex = new SearchPhaseExecutionException(
+            "search",
+            "no shard failure",
+            null,
+            ShardSearchFailure.EMPTY_ARRAY
+        );
+
+        XContentBuilder builder = XContentFactory.jsonBuilder();
+        builder.startObject();
+        ex.toXContent(builder, ToXContent.EMPTY_PARAMS);
+        builder.endObject();
+        String expected = """
+            {
+              "type": "search_phase_execution_exception",
+              "reason": "no shard failure",
+              "phase": "search",
+              "grouped": true,
+              "failed_shards": []
+            }""";
+        assertEquals(XContentHelper.stripWhitespace(expected), Strings.toString(builder));
+        assertEquals(RestStatus.SERVICE_UNAVAILABLE.getStatus(), ex.status().getStatus());
+    }
+
+    public void testReduceSearchPhaseExecutionExceptionWithNoShardFailuresAndCause() throws IOException {
+        final SearchPhaseExecutionException ex = new SearchPhaseExecutionException(
+            "search",
+            "no shard failure",
+            new IllegalArgumentException("illegal argument"),
+            ShardSearchFailure.EMPTY_ARRAY
+        );
+
+        XContentBuilder builder = XContentFactory.jsonBuilder();
+        builder.startObject();
+        ex.toXContent(builder, ToXContent.EMPTY_PARAMS);
+        builder.endObject();
+        String expected = """
+            {
+              "type": "search_phase_execution_exception",
+              "reason": "no shard failure",
+              "phase": "search",
+              "grouped": true,
+              "failed_shards": [],
+              "caused_by":{"type":"illegal_argument_exception","reason":"illegal argument"}
+            }""";
+        assertEquals(XContentHelper.stripWhitespace(expected), Strings.toString(builder));
+        assertEquals(RestStatus.BAD_REQUEST.getStatus(), ex.status().getStatus());
+    }
+
     public void testDeduplicate() throws IOException {
         {
             ShardSearchFailure failure = new ShardSearchFailure(