Skip to content

Commit 7c2ad41

Browse files
ishag4Isha Gupta
andauthored
Issue 1057: Refactored consistent responses and fixed unrelated exceptions (#1818)
* Refactored consistent responses and fixed unrelated exceptions * Refactored consistent responses and fixed unrelated exceptions * Refactored consistent responses and fixed unrelated exceptions Signed-off-by: Isha Gupta <[email protected]> * Refactored consistent responses and fixed unrelated exceptions Signed-off-by: Isha Gupta <[email protected]> * Testing Signed-off-by: Isha Gupta <[email protected]> * Trigger build Signed-off-by: Isha Gupta <[email protected]> --------- Signed-off-by: Isha Gupta <[email protected]> Co-authored-by: Isha Gupta <[email protected]>
1 parent 5454225 commit 7c2ad41

File tree

3 files changed

+166
-4
lines changed

3 files changed

+166
-4
lines changed

alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetMonitorAction.kt

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,11 @@ import org.opensearch.commons.utils.recreateObject
4141
import org.opensearch.core.action.ActionListener
4242
import org.opensearch.core.rest.RestStatus
4343
import org.opensearch.core.xcontent.NamedXContentRegistry
44+
import org.opensearch.index.IndexNotFoundException
4445
import org.opensearch.index.query.QueryBuilders
4546
import org.opensearch.search.builder.SearchSourceBuilder
4647
import org.opensearch.tasks.Task
48+
import org.opensearch.transport.RemoteTransportException
4749
import org.opensearch.transport.TransportService
4850
import org.opensearch.transport.client.Client
4951

@@ -148,14 +150,38 @@ class TransportGetMonitorAction @Inject constructor(
148150
}
149151
}
150152

151-
override fun onFailure(t: Exception) {
152-
actionListener.onFailure(AlertingException.wrap(t))
153+
override fun onFailure(ex: Exception) {
154+
if (isIndexNotFoundException(ex)) {
155+
log.error("Index not found while getting monitor", ex)
156+
actionListener.onFailure(
157+
AlertingException.wrap(
158+
OpenSearchStatusException("Monitor not found. Backing index is missing.", RestStatus.NOT_FOUND, ex)
159+
)
160+
)
161+
} else {
162+
log.error("Unexpected error while getting monitor", ex)
163+
actionListener.onFailure(AlertingException.wrap(ex))
164+
}
153165
}
154166
}
155167
)
156168
}
157169
}
158170

171+
// Checks if the exception is caused by an IndexNotFoundException (directly or nested).
172+
private fun isIndexNotFoundException(e: Exception): Boolean {
173+
if (e is IndexNotFoundException) {
174+
return true
175+
}
176+
if (e is RemoteTransportException) {
177+
val cause = e.cause
178+
if (cause is IndexNotFoundException) {
179+
return true
180+
}
181+
}
182+
return false
183+
}
184+
159185
private suspend fun getAssociatedWorkflows(id: String): List<AssociatedWorkflow> {
160186
try {
161187
val associatedWorkflows = mutableListOf<AssociatedWorkflow>()

alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@
66
package org.opensearch.alerting.transport
77

88
import org.apache.logging.log4j.LogManager
9+
import org.apache.lucene.search.TotalHits
10+
import org.apache.lucene.search.TotalHits.Relation
911
import org.opensearch.action.ActionRequest
1012
import org.opensearch.action.search.SearchRequest
1113
import org.opensearch.action.search.SearchResponse
14+
import org.opensearch.action.search.SearchResponse.Clusters
15+
import org.opensearch.action.search.ShardSearchFailure
1216
import org.opensearch.action.support.ActionFilters
1317
import org.opensearch.action.support.HandledTransportAction
1418
import org.opensearch.alerting.opensearchapi.addFilter
@@ -27,13 +31,21 @@ import org.opensearch.commons.authuser.User
2731
import org.opensearch.commons.utils.recreateObject
2832
import org.opensearch.core.action.ActionListener
2933
import org.opensearch.core.common.io.stream.NamedWriteableRegistry
34+
import org.opensearch.index.IndexNotFoundException
3035
import org.opensearch.index.query.BoolQueryBuilder
3136
import org.opensearch.index.query.ExistsQueryBuilder
3237
import org.opensearch.index.query.MatchQueryBuilder
3338
import org.opensearch.index.query.QueryBuilders
39+
import org.opensearch.search.SearchHits
40+
import org.opensearch.search.aggregations.InternalAggregations
41+
import org.opensearch.search.internal.InternalSearchResponse
42+
import org.opensearch.search.profile.SearchProfileShardResults
43+
import org.opensearch.search.suggest.Suggest
3444
import org.opensearch.tasks.Task
45+
import org.opensearch.transport.RemoteTransportException
3546
import org.opensearch.transport.TransportService
3647
import org.opensearch.transport.client.Client
48+
import java.util.Collections
3749

3850
private val log = LogManager.getLogger(TransportSearchMonitorAction::class.java)
3951

@@ -100,6 +112,39 @@ class TransportSearchMonitorAction @Inject constructor(
100112
}
101113
}
102114

115+
fun getEmptySearchResponse(): SearchResponse {
116+
val internalSearchResponse = InternalSearchResponse(
117+
SearchHits(emptyArray(), TotalHits(0L, Relation.EQUAL_TO), 0.0f),
118+
InternalAggregations.from(Collections.emptyList()),
119+
Suggest(Collections.emptyList()),
120+
SearchProfileShardResults(Collections.emptyMap()),
121+
false,
122+
false,
123+
0
124+
)
125+
126+
return SearchResponse(
127+
internalSearchResponse,
128+
"",
129+
0,
130+
0,
131+
0,
132+
0,
133+
ShardSearchFailure.EMPTY_ARRAY,
134+
SearchResponse.Clusters.EMPTY
135+
)
136+
}
137+
138+
// Checks if the exception is caused by an IndexNotFoundException (directly or nested).
139+
private fun isIndexNotFoundException(e: Exception): Boolean {
140+
if (e is IndexNotFoundException) return true
141+
if (e is RemoteTransportException) {
142+
val cause = e.cause
143+
if (cause is IndexNotFoundException) return true
144+
}
145+
return false
146+
}
147+
103148
fun search(searchRequest: SearchRequest, actionListener: ActionListener<SearchResponse>) {
104149
client.search(
105150
searchRequest,
@@ -108,8 +153,15 @@ class TransportSearchMonitorAction @Inject constructor(
108153
actionListener.onResponse(response)
109154
}
110155

111-
override fun onFailure(t: Exception) {
112-
actionListener.onFailure(AlertingException.wrap(t))
156+
override fun onFailure(ex: Exception) {
157+
if (isIndexNotFoundException(ex)) {
158+
log.error("Index not found while searching monitor", ex)
159+
val emptyResponse = getEmptySearchResponse()
160+
actionListener.onResponse(emptyResponse)
161+
} else {
162+
log.error("Unexpected error while searching monitor", ex)
163+
actionListener.onFailure(AlertingException.wrap(ex))
164+
}
113165
}
114166
}
115167
)

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,31 @@ class MonitorRestApiIT : AlertingRestTestCase() {
395395
}
396396
}
397397

398+
@Throws(Exception::class)
399+
fun `test get monitor returns 404 when alerting config index is missing`() {
400+
try {
401+
deleteIndex(".opendistro-alerting-config")
402+
} catch (e: ResponseException) {
403+
if (e.response.restStatus() != RestStatus.NOT_FOUND) {
404+
throw e
405+
}
406+
}
407+
val fakeMonitorId = "nonexistent-monitor-id"
408+
try {
409+
client().makeRequest("GET", "$ALERTING_BASE_URI/$fakeMonitorId")
410+
fail("Expected 404 when config index is missing")
411+
} catch (e: ResponseException) {
412+
val errorMessage = e.message ?: ""
413+
assertTrue(
414+
"Error message should indicate missing monitor or index",
415+
errorMessage.contains("Monitor not found") ||
416+
errorMessage.contains("index not found") ||
417+
errorMessage.contains("no such index") ||
418+
errorMessage.contains("Configured indices are not found")
419+
)
420+
}
421+
}
422+
398423
@Throws(Exception::class)
399424
fun `test checking if a monitor exists`() {
400425
val monitor = createRandomMonitor()
@@ -501,6 +526,65 @@ class MonitorRestApiIT : AlertingRestTestCase() {
501526
assertEquals("Monitor found during search when no document present.", 0, numberDocsFound)
502527
}
503528

529+
@Throws(Exception::class)
530+
fun `test search monitor returns empty response when index is missing`() {
531+
try {
532+
deleteIndex(".opendistro-alerting-config")
533+
} catch (e: ResponseException) {
534+
if (e.response.restStatus() != RestStatus.NOT_FOUND) {
535+
throw e
536+
}
537+
}
538+
val searchBody = """
539+
{
540+
"query": {
541+
"match_all": {}
542+
}
543+
}
544+
""".trimIndent()
545+
val response = client().makeRequest(
546+
"POST",
547+
"$ALERTING_BASE_URI/_search",
548+
emptyMap(),
549+
StringEntity(searchBody, ContentType.APPLICATION_JSON)
550+
)
551+
val responseBody = response.asMap()
552+
val total = ((responseBody["hits"] as? Map<*, *>)?.get("total") as? Map<*, *>)?.get("value") as? Int ?: 0
553+
assertEquals("Expected no search results when config index is missing", 0, total)
554+
}
555+
556+
@Throws(Exception::class)
557+
fun `test search monitor fails with unexpected error`() {
558+
val invalidSearchBody = """
559+
{
560+
"query": {
561+
"bad_query_type": {}
562+
}
563+
}
564+
""".trimIndent()
565+
try {
566+
client().makeRequest(
567+
"POST",
568+
"$ALERTING_BASE_URI/_search",
569+
emptyMap(),
570+
StringEntity(invalidSearchBody, ContentType.APPLICATION_JSON)
571+
)
572+
fail("Expected failure due to bad query")
573+
} catch (e: ResponseException) {
574+
val responseBody = e.response.entity.content.bufferedReader().use { it.readText() }
575+
assertTrue(
576+
"Should receive an error from unexpected query type",
577+
e.response.restStatus() === RestStatus.BAD_REQUEST ||
578+
e.response.restStatus() === RestStatus.INTERNAL_SERVER_ERROR
579+
)
580+
assertTrue(
581+
"Response body should indicate query parsing error",
582+
responseBody.contains("parsing_exception") ||
583+
responseBody.contains("failed to parse")
584+
)
585+
}
586+
}
587+
504588
fun `test query a monitor with UI metadata from OpenSearch Dashboards`() {
505589
val monitor = createRandomMonitor(refresh = true, withMetadata = true)
506590
val search = SearchSourceBuilder().query(QueryBuilders.termQuery("_id", monitor.id)).toString()

0 commit comments

Comments
 (0)