From 834aed420b9926a78b39455c1274465c7b574a0b Mon Sep 17 00:00:00 2001 From: David Newell Date: Thu, 24 Oct 2024 14:50:06 +0100 Subject: [PATCH] chore: skip when exception list is missing (#25784) --- rust/cymbal/src/main.rs | 9 +++++++-- rust/cymbal/src/resolver.rs | 2 +- rust/cymbal/src/types/mod.rs | 30 ++++++++---------------------- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/rust/cymbal/src/main.rs b/rust/cymbal/src/main.rs index 079aa5c92a6bf..699b4bbd64310 100644 --- a/rust/cymbal/src/main.rs +++ b/rust/cymbal/src/main.rs @@ -99,12 +99,17 @@ async fn main() -> Result<(), Error> { } }; - if properties.exception_list.is_empty() { + let Some(exception_list) = &properties.exception_list else { + // Known issue that $exception_list didn't exist on old clients + continue; + }; + + if exception_list.is_empty() { metrics::counter!(ERRORS, "cause" => "no_exception_list").increment(1); continue; } - let Some(trace) = properties.exception_list[0].stacktrace.as_ref() else { + let Some(trace) = exception_list[0].stacktrace.as_ref() else { metrics::counter!(ERRORS, "cause" => "no_stack_trace").increment(1); continue; }; diff --git a/rust/cymbal/src/resolver.rs b/rust/cymbal/src/resolver.rs index d2016cd06eba3..cd277cb654cc0 100644 --- a/rust/cymbal/src/resolver.rs +++ b/rust/cymbal/src/resolver.rs @@ -91,7 +91,7 @@ mod test { let exception: ClickHouseEvent = serde_json::from_str(EXAMPLE_EXCEPTION).unwrap(); let props: ErrProps = serde_json::from_str(&exception.properties.unwrap()).unwrap(); - let mut test_stack: Vec = props.exception_list[0] + let mut test_stack: Vec = props.exception_list.unwrap()[0] .stacktrace .as_ref() .unwrap() diff --git a/rust/cymbal/src/types/mod.rs b/rust/cymbal/src/types/mod.rs index 134a0fcb7420c..3a420e6410601 100644 --- a/rust/cymbal/src/types/mod.rs +++ b/rust/cymbal/src/types/mod.rs @@ -44,7 +44,7 @@ pub struct Exception { #[derive(Debug, Deserialize, Serialize, Clone)] pub struct ErrProps { #[serde(rename = "$exception_list")] - pub exception_list: Vec, // Required from exception producers - we will not process events without this + pub exception_list: Option>, // Required from exception producers - we will not process events without this. Optional to support older clients, should eventually be removed #[serde(skip_serializing_if = "Option::is_none")] #[serde(rename = "$exception_type")] pub exception_type: Option, // legacy, overridden by exception_list @@ -69,21 +69,6 @@ mod test { use super::ErrProps; - #[test] - fn it_requires_exception_list() { - let raw: &'static str = include_str!("../../tests/static/raw_ch_exception.json"); - - let raw: ClickHouseEvent = serde_json::from_str(raw).unwrap(); - - // errors out because of missing exception_list property, which is required - let props: Result = serde_json::from_str(&raw.properties.unwrap()); - assert!(props.is_err()); - assert_eq!( - props.unwrap_err().to_string(), - "missing field `$exception_list` at line 275 column 5" - ); - } - #[test] fn it_deserialises_error_props() { let raw: &'static str = include_str!("../../tests/static/raw_ch_exception_list.json"); @@ -91,23 +76,24 @@ mod test { let raw: ClickHouseEvent = serde_json::from_str(raw).unwrap(); let props: ErrProps = serde_json::from_str(&raw.properties.unwrap()).unwrap(); + let exception_list = &props.exception_list.unwrap(); - assert_eq!(props.exception_list.len(), 1); + assert_eq!(exception_list.len(), 1); assert_eq!( - props.exception_list[0].exception_type, + exception_list[0].exception_type, "UnhandledRejection".to_string() ); assert_eq!( - props.exception_list[0].exception_message, + exception_list[0].exception_message, "Unexpected usage".to_string() ); - let mechanism = props.exception_list[0].mechanism.as_ref().unwrap(); + let mechanism = exception_list[0].mechanism.as_ref().unwrap(); assert_eq!(mechanism.handled, Some(false)); assert_eq!(mechanism.mechanism_type, None); assert_eq!(mechanism.source, None); assert_eq!(mechanism.synthetic, Some(false)); - let stacktrace = props.exception_list[0].stacktrace.as_ref().unwrap(); + let stacktrace = exception_list[0].stacktrace.as_ref().unwrap(); assert_eq!(stacktrace.frames.len(), 2); let RawFrame::JavaScript(frame) = &stacktrace.frames[0]; @@ -144,7 +130,7 @@ mod test { let props: Result = serde_json::from_str(raw); assert!(props.is_ok()); - assert_eq!(props.unwrap().exception_list.len(), 0); + assert_eq!(props.unwrap().exception_list.unwrap().len(), 0); let raw: &'static str = r#"{ "$exception_list": [{