Fixed misunderstanding message 'No OpenSearchException found' when de…

…tailed_error disabled (#4669) * Fixed misunderstanding message 'No OpenSearchException found' when detailed_error disabled Signed-off-by: Xue Zhou <xuezhou@amazon.com>
opensearch-project · Oct 7, 2022 · f5ea1a4 · f5ea1a4
1 parent a2f95ce
commit f5ea1a4
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 10 deletions.
diff --git a/CHANGELOG.md b/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))

diff --git a/server/src/main/java/org/opensearch/ExceptionsHelper.java b/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;

diff --git a/server/src/main/java/org/opensearch/OpenSearchException.java b/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;
         }
 

diff --git a/server/src/test/java/org/opensearch/ExceptionsHelperTests.java b/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),

diff --git a/server/src/test/java/org/opensearch/OpenSearchExceptionTests.java b/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());

diff --git a/server/src/test/java/org/opensearch/rest/BytesRestResponseTests.java b/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 {