fix(opensearch): handle index not present errors in search api #4965

tsdk02 · 2024-06-12T10:13:57Z

Type of Change

Description

Handled errors when index is not present, by logging the corresponding error message and treating the hits as 0.
Changed the structure of OpensearchOutput and corresponding structs which are used to handle the error.

Additional Changes

This PR modifies the API contract
This PR modifies the database schema
This PR modifies application configuration/environment variables

Motivation and Context

The msearch API errors out since the index is not present, so handled the error.

How did you test it?

Set up the global search locally
Test the global search with any keyword
Go through the log, and 5xx errors should not be present anymore, with the errors logged, and handling of the index not found errors.

Checklist

I formatted the code cargo +nightly fmt --all
I addressed lints thrown by cargo clippy
I reviewed the submitted code
I added unit tests for my changes where possible

lsampras

Can you also update the readme in crates/analytics/docs to add conditions for enabling the global search on dashboard as well

I believe we have only mentioned audit_log & system_metrics

lsampras · 2024-06-13T04:07:22Z

crates/analytics/Cargo.toml

@@ -53,3 +53,4 @@ strum = { version = "0.26.2", features = ["derive"] }
 thiserror = "1.0.58"
 time = { version = "0.3.35", features = ["serde", "serde-well-known", "std"] }
 tokio = { version = "1.37.0", features = ["macros", "rt-multi-thread"] }
+log = "0.4"


we use tracing crate for logging

lsampras · 2024-06-13T04:39:10Z

crates/analytics/src/search.rs

- .json::<OpenMsearchOutput<Value>>()
+ .json::<OpenMsearchOutput>()


Can we read this as a string first using .text() and then try to parse it?
that way we can print the string as part of the parsing error helping us debug this in production environments

lsampras · 2024-06-13T04:39:42Z

crates/analytics/src/search.rs

+ .hits
+ .hits
+ .into_iter()
+ .map(|hit| hit._source)


Suggested change

.map(|hit| hit._source)

.map(|hit| hit.source)

The _ prefix is a rust convention for unused variables can we remove it?

lsampras · 2024-06-13T04:39:54Z

crates/analytics/src/search.rs

+ .collect(),
+ })
+ } else {
+ error!("Unexpected status code: {}", success.status);


use the tracing library

tsdk02 · 2024-06-13T11:58:42Z

Can you also update the readme in crates/analytics/docs to add conditions for enabling the global search on dashboard as well

I believe we have only mentioned audit_log & system_metrics

Added global_search=true in the README.md file

lsampras · 2024-06-13T13:38:27Z

crates/analytics/src/search.rs

+ Err(parse_error) => {
+ tracing::error!(
+ "Failed to parse response: {:?}, raw response: {}",
+ parse_error,
+ response_text
+ );
+ return Err(error_stack::Report::from(OpenSearchError::ResponseError));
+ }


Suggested change

Err(parse_error) => {

tracing::error!(

"Failed to parse response: {:?}, raw response: {}",

parse_error,

response_text

);

return Err(error_stack::Report::from(OpenSearchError::ResponseError));

}

er@Err(parse_error) => {

tracing::error!(

"Failed to parse response: {:?}, raw response: {}",

parse_error,

response_text

);

return er.change_context(OpenSearchError::ResponseError);

}

can we do something like this ?

Can we do it a bit more functional ?

something like

.text() .change_context(OpensearchError::ResponseBodyError) .and_then(|body| serde_json::from_str(&body) .change_context(OpensearchError::ResponseDeserializationError) .attach_printable(&body) )?

lsampras · 2024-06-13T14:00:16Z

crates/api_models/src/analytics/search.rs

+impl OpensearchError {
+ pub fn error_type(&self) -> &str {
+ &self.error.error_type
+ }
+
+ pub fn reason(&self) -> &str {
+ &self.error.reason
+ }
+}


where is this being used?

Not being used, removed it.

…ress-skip * 'main' of github.com:juspay/hyperswitch: (27 commits) feat(cypress): add 2 more payout connectors and bank transfer support for payout (#4993) chore(version): 2024.06.20.0 Refactor(core): reverts the payment method list filtering using constraint graph (#5044) feat(router): add payment method type duplication check for `google_pay` (#5023) refactor(storage): remove `id` from payment intent, attempt and remove datamodel ext from payment intent (#4923) fix(events): Correct parsing of API events with user event_type for Clickhouse (#5022) fix(connector): add local bank redirect type in compatibility layer, default the country to AT for Local Bank Redirect and add creds_identifier in access token (#5038) refactor(connector): add amount conversion framework for noon (#4843) fix(logging): fix stack overflow on recording restricted keys (#4423) feat(core): Add logger for sessions call failure (#5036) chore(version): 2024.06.19.0 fix(opensearch): handle index not present errors in search api (#4965) feat(multitenancy): add tenant_id as a field for data pipeline and support individual database for clickhouse (#4867) refactor: add basic counter metrics for IMC (#5006) fix(payment_methods): populate card fields while saving card again during metadata change condition (#5019) feat(router): Override the `setup_future_usage` to `on_session` based on the merchant config (#5016) chore(docker-compose): pass correct configuration values for running SDK demo app (#5012) refactor: Move trait ConnectorIntegration to crate hyperswitch_interfaces (#4946) chore(version): 2024.06.17.0 chore(process_tracker): use `const` instead of `String` for `business_status` (#4849) ...

Handling index not present errors in search api

e2f40c4

tsdk02 requested a review from lsampras June 12, 2024 10:13

tsdk02 self-assigned this Jun 12, 2024

tsdk02 requested a review from a team as a code owner June 12, 2024 10:13

tsdk02 linked an issue Jun 12, 2024 that may be closed by this pull request

fix(opensearch): handle index not present errors in search api #4905

Closed

chore: run formatter

2d6d1dc

tsdk02 requested review from ivor-juspay and ShivanshMathurJuspay June 12, 2024 10:17

chore: update Cargo.lock

4564ac7

hyperswitch-bot bot requested a review from a team as a code owner June 12, 2024 10:23

tsdk02 added 2 commits June 12, 2024 16:59

Completed merge with main

edd2535

resolved conflicts

9c2e344

lsampras reviewed Jun 13, 2024

View reviewed changes

tsdk02 and others added 3 commits June 13, 2024 17:20

Merged main

ce05f82

Resolved comments

38343f9

chore: run formatter

190b61f

lsampras previously approved these changes Jun 13, 2024

View reviewed changes

lsampras reviewed Jun 13, 2024

View reviewed changes

tsdk02 added 2 commits June 18, 2024 14:54

merging main

b819ef0

Fixing errors and converting to more functional manner

4ff31ef

tsdk02 dismissed lsampras’s stale review via 4ff31ef June 18, 2024 09:30

tsdk02 requested a review from lsampras June 18, 2024 10:40

lsampras approved these changes Jun 18, 2024

View reviewed changes

ivor-juspay approved these changes Jun 18, 2024

View reviewed changes

Merge branch 'main' into opensearch-error-handling

ed90dda

Gnanasundari24 added this pull request to the merge queue Jun 18, 2024

Merged via the queue into main with commit ae1edb0 Jun 18, 2024
11 checks passed

Gnanasundari24 deleted the opensearch-error-handling branch June 18, 2024 19:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(opensearch): handle index not present errors in search api #4965

fix(opensearch): handle index not present errors in search api #4965

tsdk02 commented Jun 12, 2024 •

edited

Loading

lsampras left a comment

lsampras Jun 13, 2024

tsdk02 Jun 13, 2024

lsampras Jun 13, 2024

tsdk02 Jun 13, 2024

lsampras Jun 13, 2024

tsdk02 Jun 13, 2024

lsampras Jun 13, 2024

tsdk02 Jun 13, 2024

tsdk02 commented Jun 13, 2024

lsampras Jun 13, 2024

lsampras Jun 13, 2024

tsdk02 Jun 18, 2024

lsampras Jun 13, 2024

tsdk02 Jun 14, 2024

		.json::<OpenMsearchOutput<Value>>()
		.json::<OpenMsearchOutput>()

fix(opensearch): handle index not present errors in search api #4965

fix(opensearch): handle index not present errors in search api #4965

Conversation

tsdk02 commented Jun 12, 2024 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

lsampras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsdk02 commented Jun 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsdk02 commented Jun 12, 2024 •

edited

Loading