[MD] Improved error handling for the search API when a null value is passed for the dataSourceId.

[xinruiba]

Description

#5880

Good error handling is important to help our customer understand our production's expected behavior.

This is the PR to improve the error handling process by providing

1. More accurate status code
2. More descriptive error message

for the following scenario:

When MD is enabled, opensearch hosts not exist and search request send an empty datasource id. We should treat that request as a bad request with error code 400 instead of 500 along with a more descriptive error message.

Issues Resolved

Screenshot

N/A

Testing the changes

New test cases added

Check List

• All tests pass
  • yarn test:jest
  • yarn test:jest_integration
• New functionality includes testing.
• New functionality has been documented.
• Update CHANGELOG.md
• Commits are signed per the DCO using --signoff

[codecov]

Codecov Report

Attention: Patch coverage is 83.33333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 67.01%. Comparing base (6d5560e) to head (7f2616c).
Report is 1 commits behind head on main.

❗ Current head 7f2616c differs from pull request most recent head 657d954. Consider uploading reports for the commit 657d954 to get more accurate results

Files	Patch %	Lines
...ch/opensearch_search/opensearch_search_strategy.ts	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5882      +/-   ##
==========================================
- Coverage   67.01%   67.01%   -0.01%     
==========================================
  Files        3310     3307       -3     
  Lines       63647    63616      -31     
  Branches    10165    10164       -1     
==========================================
- Hits        42656    42633      -23     
+ Misses      18522    18514       -8     
  Partials     2469     2469              

Flag	Coverage Δ
Linux_1	35.21% <0.00%> (-0.01%)	⬇️
Linux_2	55.09% <0.00%> (-0.01%)	⬇️
Linux_3	43.63% <83.33%> (-0.76%)	⬇️
Linux_4	35.19% <0.00%> (-0.01%)	⬇️
Windows_1	35.23% <0.00%> (-0.01%)	⬇️
Windows_2	55.05% <0.00%> (-0.01%)	⬇️
Windows_3	43.65% <83.33%> (-0.76%)	⬇️
Windows_4	35.19% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[BionIT]

can we assume when data source is enabled, then context.dataSource is never undefined or vice versa?

[bandinib-amzn]

+1 @xinruiba Do you have scenario where withDataSourceEnabled is true but context.dataSource is undefined?

[xinruiba]

Thanks for the comments~

Yes, we can assume when data source is enabled, then context.dataSource is never undefined or vice versa because:

1. "dataSource" will always been registered as "registerRouteHandlerContext" in dataSource plugin setup which means once dataSource plugin's setup function get called, context.dataSource will never be undefined.
2. If dataSource.enabled turns on, dataSource plugin's setup function will always get called.

In that case, withDataSourceEnabled is not needed in this decide client function. I will remove it.

Thanks

[bandinib-amzn]

Can you correct this according to test description?

[xinruiba]

Test case updated here:
https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882/files#diff-96950b20277c55c90d7592913fcc77ed6bac02a71701c784f2ac3b03c31d25d6R203

Thanks~

[bandinib-amzn]

You are passing host empty. Can you correct test case description? Also do we have test case when host is not defined, means it is commented? In that case it should take default value and call opensearch client

[xinruiba]

Both updated in the test case:
https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882/files#diff-96950b20277c55c90d7592913fcc77ed6bac02a71701c784f2ac3b03c31d25d6R242

Thanks for the comment.

[bandinib-amzn]

Do we have to pass empty? What if we don't pass at all? Can we cover both case pass empty or undefined value and doesn't pass at all

[xinruiba]

Test case updated and handled here:
https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882/files#diff-96950b20277c55c90d7592913fcc77ed6bac02a71701c784f2ac3b03c31d25d6R237

Thanks

[Flyingliuhub]

I thought that you are testing send request without dataSourceid, but pass dataSourceId: 'Some dataSourceId'

[xinruiba]

Test cases updated, thanks for the comment.