Skip to content

Commit e814981

Browse files
ritvibhattritvibhatt
authored
[Enhancement] Fix error messages for invalid index mapping (#5370)
1 parent 419b45a commit e814981

4 files changed

Lines changed: 146 additions & 4 deletions

File tree

‎opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,11 @@ public Map<String, IndexMapping> getIndexMappings(String... indexExpression) {
115115
.build();
116116
} catch (Exception e) {
117117
throw new IllegalStateException(
118-
"Failed to read mapping for index pattern [" + indexExpression + "]", e);
118+
"Failed to read mapping for index pattern ["
119+
+ String.join(",", indexExpression)
120+
+ "]: "
121+
+ e.getMessage(),
122+
e);
119123
}
120124
}
121125

@@ -145,7 +149,11 @@ public Map<String, Integer> getIndexMaxResultWindows(String... indexExpression)
145149
throw e;
146150
} catch (Exception e) {
147151
throw new IllegalStateException(
148-
"Failed to read setting for index pattern [" + indexExpression + "]", e);
152+
"Failed to read setting for index pattern ["
153+
+ String.join(",", indexExpression)
154+
+ "]: "
155+
+ e.getMessage(),
156+
e);
149157
}
150158
}
151159

‎opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchRestClient.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,12 @@ public Map<String, IndexMapping> getIndexMappings(String... indexExpression) {
7979
return response.mappings().entrySet().stream()
8080
.collect(Collectors.toMap(Map.Entry::getKey, e -> new IndexMapping(e.getValue())));
8181
} catch (IOException e) {
82-
throw new IllegalStateException("Failed to get index mappings for " + indexExpression, e);
82+
throw new IllegalStateException(
83+
"Failed to get index mappings for "
84+
+ String.join(",", indexExpression)
85+
+ ": "
86+
+ e.getMessage(),
87+
e);
8388
}
8489
}
8590

@@ -111,7 +116,12 @@ public Map<String, Integer> getIndexMaxResultWindows(String... indexExpression)
111116

112117
return result;
113118
} catch (IOException e) {
114-
throw new IllegalStateException("Failed to get max result window for " + indexExpression, e);
119+
throw new IllegalStateException(
120+
"Failed to get max result window for "
121+
+ String.join(",", indexExpression)
122+
+ ": "
123+
+ e.getMessage(),
124+
e);
115125
}
116126
}
117127

‎opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,66 @@ void ml() {
499499
assertNotNull(client.getNodeClient());
500500
}
501501

502+
@Test
503+
void get_index_mappings_error_message_includes_single_index() {
504+
String underlyingError = "Connection timeout";
505+
when(nodeClient.admin().indices()).thenThrow(new RuntimeException(underlyingError));
506+
507+
IllegalStateException exception =
508+
assertThrows(IllegalStateException.class, () -> client.getIndexMappings("test_index"));
509+
510+
assertAll(
511+
() -> assertTrue(exception.getMessage().contains("test_index")),
512+
() -> assertTrue(exception.getMessage().contains(underlyingError)));
513+
}
514+
515+
@Test
516+
void get_index_mappings_error_message_includes_multiple_indices() {
517+
String underlyingError = "Access denied";
518+
when(nodeClient.admin().indices()).thenThrow(new RuntimeException(underlyingError));
519+
520+
IllegalStateException exception =
521+
assertThrows(
522+
IllegalStateException.class,
523+
() -> client.getIndexMappings("index1", "index2", "index3"));
524+
525+
assertAll(
526+
() -> assertTrue(exception.getMessage().contains("index1")),
527+
() -> assertTrue(exception.getMessage().contains("index2")),
528+
() -> assertTrue(exception.getMessage().contains("index3")),
529+
() -> assertTrue(exception.getMessage().contains(underlyingError)));
530+
}
531+
532+
@Test
533+
void get_index_max_result_windows_error_message_includes_single_index() {
534+
String underlyingError = "Network error";
535+
when(nodeClient.admin().indices()).thenThrow(new RuntimeException(underlyingError));
536+
537+
IllegalStateException exception =
538+
assertThrows(
539+
IllegalStateException.class, () -> client.getIndexMaxResultWindows("test_index"));
540+
541+
assertAll(
542+
() -> assertTrue(exception.getMessage().contains("test_index")),
543+
() -> assertTrue(exception.getMessage().contains(underlyingError)));
544+
}
545+
546+
@Test
547+
void get_index_max_result_windows_error_message_includes_multiple_indices() {
548+
String underlyingError = "Permission denied";
549+
when(nodeClient.admin().indices()).thenThrow(new RuntimeException(underlyingError));
550+
551+
IllegalStateException exception =
552+
assertThrows(
553+
IllegalStateException.class,
554+
() -> client.getIndexMaxResultWindows("logs-2024", "metrics-2024"));
555+
556+
assertAll(
557+
() -> assertTrue(exception.getMessage().contains("logs-2024")),
558+
() -> assertTrue(exception.getMessage().contains("metrics-2024")),
559+
() -> assertTrue(exception.getMessage().contains(underlyingError)));
560+
}
561+
502562
public void mockNodeClientIndicesMappings(String indexName, String mappings) {
503563
GetMappingsResponse mockResponse = mock(GetMappingsResponse.class);
504564
MappingMetadata emptyMapping = mock(MappingMetadata.class);

‎opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,70 @@ void ml_with_exception() {
531531
assertEquals(Optional.empty(), client.getNodeClient());
532532
}
533533

534+
@Test
535+
void get_index_mappings_error_message_includes_single_index() throws IOException {
536+
String underlyingError = "Network timeout";
537+
when(restClient.indices().getMapping(any(GetMappingsRequest.class), any()))
538+
.thenThrow(new IOException(underlyingError));
539+
540+
IllegalStateException exception =
541+
assertThrows(IllegalStateException.class, () -> client.getIndexMappings("test_index"));
542+
543+
assertAll(
544+
() -> assertTrue(exception.getMessage().contains("test_index")),
545+
() -> assertTrue(exception.getMessage().contains(underlyingError)));
546+
}
547+
548+
@Test
549+
void get_index_mappings_error_message_includes_multiple_indices() throws IOException {
550+
String underlyingError = "Connection refused";
551+
when(restClient.indices().getMapping(any(GetMappingsRequest.class), any()))
552+
.thenThrow(new IOException(underlyingError));
553+
554+
IllegalStateException exception =
555+
assertThrows(
556+
IllegalStateException.class,
557+
() -> client.getIndexMappings("index1", "index2", "index3"));
558+
559+
assertAll(
560+
() -> assertTrue(exception.getMessage().contains("index1")),
561+
() -> assertTrue(exception.getMessage().contains("index2")),
562+
() -> assertTrue(exception.getMessage().contains("index3")),
563+
() -> assertTrue(exception.getMessage().contains(underlyingError)));
564+
}
565+
566+
@Test
567+
void get_index_max_result_windows_error_message_includes_single_index() throws IOException {
568+
String underlyingError = "Authentication failed";
569+
when(restClient.indices().getSettings(any(GetSettingsRequest.class), any()))
570+
.thenThrow(new IOException(underlyingError));
571+
572+
IllegalStateException exception =
573+
assertThrows(
574+
IllegalStateException.class, () -> client.getIndexMaxResultWindows("test_index"));
575+
576+
assertAll(
577+
() -> assertTrue(exception.getMessage().contains("test_index")),
578+
() -> assertTrue(exception.getMessage().contains(underlyingError)));
579+
}
580+
581+
@Test
582+
void get_index_max_result_windows_error_message_includes_multiple_indices() throws IOException {
583+
String underlyingError = "Timeout";
584+
when(restClient.indices().getSettings(any(GetSettingsRequest.class), any()))
585+
.thenThrow(new IOException(underlyingError));
586+
587+
IllegalStateException exception =
588+
assertThrows(
589+
IllegalStateException.class,
590+
() -> client.getIndexMaxResultWindows("logs-2024", "metrics-2024"));
591+
592+
assertAll(
593+
() -> assertTrue(exception.getMessage().contains("logs-2024")),
594+
() -> assertTrue(exception.getMessage().contains("metrics-2024")),
595+
() -> assertTrue(exception.getMessage().contains(underlyingError)));
596+
}
597+
534598
private Map<String, MappingMetadata> mockFieldMappings(String indexName, String mappings)
535599
throws IOException {
536600
return ImmutableMap.of(indexName, IndexMetadata.fromXContent(createParser(mappings)).mapping());

0 commit comments

Comments
 (0)