Fix index not found reported as server error bug (#1353)

* Capture IndexNotFoundException and re-throw directly Signed-off-by: Chen Dai <[email protected]> * Add IT Signed-off-by: Chen Dai <[email protected]> --------- Signed-off-by: Chen Dai <[email protected]> (cherry picked from commit 76a8d29)
opensearch-project · Feb 17, 2023 · 0cfb7f6 · 0cfb7f6
1 parent cc2a768
commit 0cfb7f6
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 16 deletions.
diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/PPLPluginIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/PPLPluginIT.java
@@ -57,6 +57,14 @@ public void testQueryEndpointShouldFail() throws IOException {
     client().performRequest(makePPLRequest("search invalid"));
   }
 
+  @Test
+  public void testQueryEndpointShouldFailWithNonExistIndex() throws IOException {
+    exceptionRule.expect(ResponseException.class);
+    exceptionRule.expect(hasProperty("response", statusCode(400)));
+
+    client().performRequest(makePPLRequest("search source=non_exist_index"));
+  }
+
   @Test
   public void sqlEnableSettingsTest() throws IOException {
     String query =

diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java
@@ -26,8 +26,8 @@
 import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
 import org.opensearch.client.node.NodeClient;
 import org.opensearch.cluster.metadata.AliasMetadata;
-import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.index.IndexNotFoundException;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.sql.opensearch.mapping.IndexMapping;
 import org.opensearch.sql.opensearch.request.OpenSearchRequest;
@@ -42,15 +42,11 @@ public class OpenSearchNodeClient implements OpenSearchClient {
   /** Node client provided by OpenSearch container. */
   private final NodeClient client;
 
-  /** Index name expression resolver to get concrete index name. */
-  private final IndexNameExpressionResolver resolver;
-
   /**
    * Constructor of ElasticsearchNodeClient.
    */
   public OpenSearchNodeClient(NodeClient client) {
     this.client = client;
-    this.resolver = new IndexNameExpressionResolver(client.threadPool().getThreadContext());
   }
 
   @Override
@@ -96,6 +92,9 @@ public Map<String, IndexMapping> getIndexMappings(String... indexExpression) {
       return Streams.stream(mappingsResponse.mappings().iterator())
           .collect(Collectors.toMap(cursor -> cursor.key,
               cursor -> new IndexMapping(cursor.value)));
+    } catch (IndexNotFoundException e) {
+      // Re-throw directly to be treated as client error finally
+      throw e;
     } catch (Exception e) {
       throw new IllegalStateException(
           "Failed to read mapping for index pattern [" + indexExpression + "]", e);

diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java
@@ -59,6 +59,7 @@
 import org.opensearch.common.xcontent.NamedXContentRegistry;
 import org.opensearch.common.xcontent.XContentParser;
 import org.opensearch.common.xcontent.XContentType;
+import org.opensearch.index.IndexNotFoundException;
 import org.opensearch.search.SearchHit;
 import org.opensearch.search.SearchHits;
 import org.opensearch.sql.data.model.ExprIntegerValue;
@@ -200,8 +201,13 @@ void getIndexMappingsWithIOException() {
 
   @Test
   void getIndexMappingsWithNonExistIndex() {
-    mockNodeClient("test");
-    assertTrue(client.getIndexMappings("non_exist_index").isEmpty());
+    when(nodeClient.admin().indices()
+        .prepareGetMappings(any())
+        .setLocal(anyBoolean())
+        .get()
+    ).thenThrow(IndexNotFoundException.class);
+
+    assertThrows(IndexNotFoundException.class, () -> client.getIndexMappings("non_exist_index"));
   }
 
   @Test
@@ -375,15 +381,6 @@ public void mockNodeClientIndicesMappings(String indexName, String mappings) {
     }
   }
 
-  public void mockNodeClient(String indexName) {
-    GetMappingsResponse mockResponse = mock(GetMappingsResponse.class);
-    when(nodeClient.admin().indices()
-        .prepareGetMappings(any())
-        .setLocal(anyBoolean())
-        .get()).thenReturn(mockResponse);
-    when(mockResponse.mappings()).thenReturn(ImmutableOpenMap.of());
-  }
-
   private void mockNodeClientSettings(String indexName, String indexMetadata)
       throws IOException {
     GetSettingsResponse mockResponse = mock(GetSettingsResponse.class);