Fixed misunderstanding message 'No OpenSearchException found' when detailed_error disabled (opensearch-project#4669)

* Fixed misunderstanding message 'No OpenSearchException found' when detailed_error disabled

Signed-off-by: Xue Zhou <[email protected]>

Author: xuezhou25

CHANGELOG.md

@@ -116,6 +116,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Segment Replication] Adding check to make sure checkpoint is not processed when a shard's shard routing is primary ([#4630](https://github.com/opensearch-project/OpenSearch/pull/4630))
 - [Bug]: Fixed invalid location of JDK dependency for arm64 architecture([#4613](https://github.com/opensearch-project/OpenSearch/pull/4613))
 - [Bug]: Alias filter lost after rollover ([#4499](https://github.com/opensearch-project/OpenSearch/pull/4499))
+- Fixed misunderstanding message "No OpenSearchException found" when detailed_error disabled ([#4669](https://github.com/opensearch-project/OpenSearch/pull/4669))
 - Attempt to fix Github workflow for Gradle Check job ([#4679](https://github.com/opensearch-project/OpenSearch/pull/4679))
 - Fix flaky DecommissionControllerTests.testTimesOut ([4683](https://github.com/opensearch-project/OpenSearch/pull/4683))
 - Fix new race condition in DecommissionControllerTests ([4688](https://github.com/opensearch-project/OpenSearch/pull/4688))

server/src/main/java/org/opensearch/ExceptionsHelper.java

@@ -99,6 +99,21 @@ public static RestStatus status(Throwable t) {
         return RestStatus.INTERNAL_SERVER_ERROR;
     }
 
+    public static String summaryMessage(Throwable t) {
+        if (t != null) {
+            if (t instanceof OpenSearchException) {
+                return t.getClass().getSimpleName() + "[" + t.getMessage() + "]";
+            } else if (t instanceof IllegalArgumentException) {
+                return "Invalid argument";
+            } else if (t instanceof JsonParseException) {
+                return "Failed to parse JSON";
+            } else if (t instanceof OpenSearchRejectedExecutionException) {
+                return "Too many requests";
+            }
+        }
+        return "Internal failure";
+    }
+
     public static Throwable unwrapCause(Throwable t) {
         int counter = 0;
         Throwable result = t;

server/src/main/java/org/opensearch/OpenSearchException.java

@@ -594,16 +594,14 @@ public static void generateFailureXContent(XContentBuilder builder, Params param
 
         // Render the exception with a simple message
         if (detailed == false) {
-            String message = "No OpenSearchException found";
             Throwable t = e;
             for (int counter = 0; counter < 10 && t != null; counter++) {
                 if (t instanceof OpenSearchException) {
-                    message = t.getClass().getSimpleName() + "[" + t.getMessage() + "]";
                     break;
                 }
                 t = t.getCause();
             }
-            builder.field(ERROR, message);
+            builder.field(ERROR, ExceptionsHelper.summaryMessage(t));
             return;
         }
 

server/src/test/java/org/opensearch/ExceptionsHelperTests.java

@@ -113,6 +113,12 @@ public void testStatus() {
         assertThat(ExceptionsHelper.status(new OpenSearchRejectedExecutionException("rejected")), equalTo(RestStatus.TOO_MANY_REQUESTS));
     }
 
+    public void testSummaryMessage() {
+        assertThat(ExceptionsHelper.summaryMessage(new IllegalArgumentException("illegal")), equalTo("Invalid argument"));
+        assertThat(ExceptionsHelper.summaryMessage(new JsonParseException(null, "illegal")), equalTo("Failed to parse JSON"));
+        assertThat(ExceptionsHelper.summaryMessage(new OpenSearchRejectedExecutionException("rejected")), equalTo("Too many requests"));
+    }
+
     public void testGroupBy() {
         ShardOperationFailedException[] failures = new ShardOperationFailedException[] {
             createShardFailureParsingException("error", "node0", "index", 0, null),

server/src/test/java/org/opensearch/OpenSearchExceptionTests.java

@@ -814,12 +814,7 @@ public void testFailureToAndFromXContentWithNoDetails() throws IOException {
         }
         assertNotNull(parsedFailure);
 
-        String reason;
-        if (failure instanceof OpenSearchException) {
-            reason = failure.getClass().getSimpleName() + "[" + failure.getMessage() + "]";
-        } else {
-            reason = "No OpenSearchException found";
-        }
+        String reason = ExceptionsHelper.summaryMessage(failure);
         assertEquals(OpenSearchException.buildMessage("exception", reason, null), parsedFailure.getMessage());
         assertEquals(0, parsedFailure.getHeaders().size());
         assertEquals(0, parsedFailure.getMetadata().size());

server/src/test/java/org/opensearch/rest/BytesRestResponseTests.java

@@ -117,7 +117,7 @@ public void testNonOpenSearchExceptionIsNotShownAsSimpleMessage() throws Excepti
         assertThat(text, not(containsString("UnknownException[an error occurred reading data]")));
         assertThat(text, not(containsString("FileNotFoundException[/foo/bar]")));
         assertThat(text, not(containsString("error_trace")));
-        assertThat(text, containsString("\"error\":\"No OpenSearchException found\""));
+        assertThat(text, containsString("\"error\":\"Internal failure\""));
     }
 
     public void testErrorTrace() throws Exception {