avoid sending back verbose error message and wrong 500 error to user; fix hard code query size of historical analysis #150
Conversation
Signed-off-by: Yaliang Wu <ylwu@amazon.com>
Codecov Report
@@ Coverage Diff @@
## main #150 +/- ##
============================================
- Coverage 70.33% 70.31% -0.03%
- Complexity 2969 2972 +3
============================================
Files 268 268
Lines 14011 14051 +40
Branches 1405 1409 +4
============================================
+ Hits 9855 9880 +25
- Misses 3546 3554 +8
- Partials 610 617 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -425,7 +426,7 @@ public void onResponse(SearchResponse response) { | |||
| listener.onResponse(topEntities); | |||
| } else if (expirationEpochMs < clock.millis()) { | |||
| if (topEntities.isEmpty()) { | |||
| listener.onFailure(new IllegalStateException("timeout to get preview results. Please retry later.")); | |||
| listener.onFailure(new AnomalyDetectionException("timeout to get preview results. Please retry later.")); | |||
There was a problem hiding this comment.
In AD we have self-defined exceptions under class "AnomalyDetectionException" , this PR need to get concrete error message based on exception class, if it's "AnomalyDetectionException" (and several other exceptions, check RestHandlerUtils class), we will get message from exception directly, otherwise we use general error message to avoid returning verbose error message to user. Check RestHandlerUtils#wrapRestActionListener for details.
Security team has concern about returning verbose error .
| @@ -236,7 +244,7 @@ public void accept(GetResponse response) throws Exception { | |||
| if (!response.isExists()) { | |||
| listener | |||
| .onFailure( | |||
| new OpenSearchException("Can't find anomaly detector with id:" + response.getId(), RestStatus.NOT_FOUND) | |||
| new OpenSearchStatusException("Can't find anomaly detector with id:" + response.getId(), RestStatus.NOT_FOUND) | |||
There was a problem hiding this comment.
Why didnt we have to change these exception messages?
There was a problem hiding this comment.
Same as this comment https://github.com/opensearch-project/anomaly-detection/pull/150/files/713eb7970db4c021aed87d559f2eba68cb32e129#r680149988. we need to get concrete error message based on exception type. For OpenSearchException, we will return general error message rather than this concrete error message.
… fix hard code query size of historical analysis (opensearch-project#150) * avoid sending back verbose error message and wrong 500 error to user Signed-off-by: Yaliang Wu <ylwu@amazon.com> * put general error message into constants
… fix hard code query size of historical analysis (opensearch-project#150) * avoid sending back verbose error message and wrong 500 error to user Signed-off-by: Yaliang Wu <ylwu@amazon.com> * put general error message into constants
… fix hard code query size of historical analysis (#150) * avoid sending back verbose error message and wrong 500 error to user Signed-off-by: Yaliang Wu <ylwu@amazon.com> * put general error message into constants
Signed-off-by: Yaliang Wu ylwu@amazon.com
Description
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.