feat(errors): Use exception list and remove stack trace raw (#25655)

PostHog · Oct 21, 2024 · abd7ae0 · abd7ae0
1 parent c6dc127
commit abd7ae0
Show file tree

Hide file tree

Showing 7 changed files with 185 additions and 28 deletions.
diff --git a/rust/cymbal/src/langs/js.rs b/rust/cymbal/src/langs/js.rs
@@ -1,5 +1,5 @@
 use reqwest::Url;
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use sourcemap::Token;
 
 use crate::{
@@ -10,7 +10,7 @@ use crate::{
 // A minifed JS stack frame. Just the minimal information needed to lookup some
 // sourcemap for it and produce a "real" stack frame.
 // TODO - how do we know if this frame is minified? If it isn't, we can skip a lot of work, but I think we have to guess? Based on whether we can get a sourcemap for it?
-#[derive(Debug, Clone, Deserialize)]
+#[derive(Debug, Clone, Deserialize, Serialize)]
 pub struct RawJSFrame {
     #[serde(rename = "lineno")]
     pub line: u32,
@@ -23,6 +23,24 @@ pub struct RawJSFrame {
     pub fn_name: String,
 }
 
+// export interface StackFrame {
+//     filename?: string
+//     function?: string
+//     module?: string
+//     platform?: string
+//     lineno?: number
+//     colno?: number
+//     abs_path?: string
+//     context_line?: string
+//     pre_context?: string[]
+//     post_context?: string[]
+//     in_app?: boolean
+//     instruction_addr?: string
+//     addr_mode?: string
+//     vars?: { [key: string]: any }
+//     debug_id?: string
+// }
+
 impl RawJSFrame {
     pub fn source_ref(&self) -> Result<Url, JsResolveErr> {
         // We can't resolve a frame without a source ref, and are forced

diff --git a/rust/cymbal/src/main.rs b/rust/cymbal/src/main.rs
@@ -99,23 +99,21 @@ async fn main() -> Result<(), Error> {
             }
         };
 
-        let Some(trace) = properties.exception_stack_trace_raw.as_ref() else {
+        if properties.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 {
             metrics::counter!(ERRORS, "cause" => "no_stack_trace").increment(1);
             continue;
         };
 
-        let stack_trace: Vec<RawFrame> = match serde_json::from_str(trace) {
-            Ok(r) => r,
-            Err(err) => {
-                metrics::counter!(ERRORS, "cause" => "invalid_stack_trace").increment(1);
-                error!("Error parsing stack trace: {:?}", err);
-                continue;
-            }
-        };
+        let stack_trace: &Vec<RawFrame> = &trace.frames;
 
         let mut resolved_frames = Vec::new();
         for frame in stack_trace {
-            let resolved = match context.resolver.resolve(frame, 1).await {
+            let resolved = match context.resolver.resolve(frame.clone(), 1).await {
                 Ok(r) => r,
                 Err(err) => {
                     metrics::counter!(ERRORS, "cause" => "frame_not_parsable").increment(1);

diff --git a/rust/cymbal/src/resolver.rs b/rust/cymbal/src/resolver.rs
@@ -72,7 +72,7 @@ mod test {
     const CHUNK_PATH: &str = "/static/chunk-PGUQKT6S.js";
     const MINIFIED: &[u8] = include_bytes!("../tests/static/chunk-PGUQKT6S.js");
     const MAP: &[u8] = include_bytes!("../tests/static/chunk-PGUQKT6S.js.map");
-    const EXAMPLE_EXCEPTION: &str = include_str!("../tests/static/raw_ch_exception.json");
+    const EXAMPLE_EXCEPTION: &str = include_str!("../tests/static/raw_ch_exception_list.json");
 
     #[tokio::test]
     async fn end_to_end_resolver_test() {
@@ -91,8 +91,12 @@ 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<RawFrame> =
-            serde_json::from_str(props.exception_stack_trace_raw.as_ref().unwrap()).unwrap();
+        let mut test_stack: Vec<RawFrame> = props.exception_list[0]
+            .stacktrace
+            .as_ref()
+            .unwrap()
+            .frames
+            .clone();
 
         // We're going to pretend out stack consists exclusively of JS frames whose source
         // we have locally

diff --git a/rust/cymbal/src/types/frames.rs b/rust/cymbal/src/types/frames.rs
@@ -8,7 +8,7 @@ use crate::{
 
 // We consume a huge variety of differently shaped stack frames, which we have special-case
 // transformation for, to produce a single, unified representation of a frame.
-#[derive(Debug, Deserialize)]
+#[derive(Debug, Deserialize, Serialize, Clone)]
 #[serde(untagged)]
 pub enum RawFrame {
     JavaScript(RawJSFrame),

diff --git a/rust/cymbal/src/types/mod.rs b/rust/cymbal/src/types/mod.rs
@@ -5,46 +5,172 @@ use serde_json::Value;
 
 pub mod frames;
 
+#[derive(Debug, Deserialize, Serialize, Clone)]
+pub struct Mechanism {
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub handled: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    #[serde(rename = "type")]
+    pub mechanism_type: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub source: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub synthetic: Option<bool>,
+}
+
+#[derive(Debug, Deserialize, Serialize, Clone)]
+pub struct Stacktrace {
+    pub frames: Vec<frames::RawFrame>,
+}
+
+#[derive(Debug, Deserialize, Serialize, Clone)]
+pub struct Exception {
+    #[serde(rename = "type")]
+    pub exception_type: String,
+    #[serde(rename = "value")]
+    pub exception_message: String,
+    pub mechanism: Option<Mechanism>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub module: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub thread_id: Option<i32>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub stacktrace: Option<Stacktrace>,
+}
+
 // Given a Clickhouse Event's properties, we care about the contents
 // of only a small subset. This struct is used to give us a strongly-typed
 // "view" of those event properties we care about.
 #[derive(Debug, Deserialize, Serialize, Clone)]
 pub struct ErrProps {
+    #[serde(rename = "$exception_list")]
+    pub exception_list: Vec<Exception>, // Required from exception producers - we will not process events without this
+    #[serde(skip_serializing_if = "Option::is_none")]
     #[serde(rename = "$exception_type")]
-    pub exception_type: String, // Required from exception producers - we will not process events without this
+    pub exception_type: Option<String>, // legacy, overridden by exception_list
+    #[serde(skip_serializing_if = "Option::is_none")]
     #[serde(rename = "$exception_message")]
-    pub exception_message: String, // Required from exception producers - we will not process events without this
+    pub exception_message: Option<String>, // legacy, overridden by exception_list
+    #[serde(skip_serializing_if = "Option::is_none")]
     #[serde(rename = "$exception_stack_trace_raw")]
     pub exception_stack_trace_raw: Option<String>, // Not all exceptions have a stack trace
     #[serde(rename = "$exception_level")]
     pub exception_level: Option<String>, // We generally don't touch this, but we break it out explicitly for users. Not all exceptions have a level
-    #[serde(rename = "$exception_source")]
-    pub exception_source: Option<String>, // For some languages, we can associate the exception with e.g. a source file or binary.
-    #[serde(rename = "$exception_lineno")]
-    pub exception_line: Option<u32>, // Some exceptions have a source line
-    #[serde(rename = "$exception_colno")]
-    pub exception_col: Option<u32>, // Some even have a column
     #[serde(flatten)] // A catch-all for all the properties we don't "care" about
     pub other: HashMap<String, Value>,
 }
 
 #[cfg(test)]
 mod test {
     use common_types::ClickHouseEvent;
+    use serde_json::Error;
 
     use crate::types::frames::RawFrame;
 
     use super::ErrProps;
 
     #[test]
-    fn it_symbolifies() {
+    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<ErrProps, Error> = 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");
+
+        let raw: ClickHouseEvent = serde_json::from_str(raw).unwrap();
+
         let props: ErrProps = serde_json::from_str(&raw.properties.unwrap()).unwrap();
 
-        let _stack_trace: Vec<RawFrame> =
-            serde_json::from_str(props.exception_stack_trace_raw.as_ref().unwrap()).unwrap();
+        assert_eq!(props.exception_list.len(), 1);
+        assert_eq!(
+            props.exception_list[0].exception_type,
+            "UnhandledRejection".to_string()
+        );
+        assert_eq!(
+            props.exception_list[0].exception_message,
+            "Unexpected usage".to_string()
+        );
+        let mechanism = props.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();
+        assert_eq!(stacktrace.frames.len(), 2);
+        let RawFrame::JavaScript(frame) = &stacktrace.frames[0];
+
+        assert_eq!(
+            frame.source_url,
+            Some("https://app-static.eu.posthog.com/static/chunk-PGUQKT6S.js".to_string())
+        );
+        assert_eq!(frame.fn_name, "?".to_string());
+        assert_eq!(frame.in_app, true);
+        assert_eq!(frame.line, 64);
+        assert_eq!(frame.column, 25112);
+
+        let RawFrame::JavaScript(frame) = &stacktrace.frames[1];
+        assert_eq!(
+            frame.source_url,
+            Some("https://app-static.eu.posthog.com/static/chunk-PGUQKT6S.js".to_string())
+        );
+        assert_eq!(frame.fn_name, "n.loadForeignModule".to_string());
+        assert_eq!(frame.in_app, true);
+        assert_eq!(frame.line, 64);
+        assert_eq!(frame.column, 15003);
+
+        assert_eq!(props.exception_type, None);
+        assert_eq!(props.exception_message, None);
+        assert_eq!(props.exception_stack_trace_raw, None);
+        assert_eq!(props.exception_level, Some("error".to_string()));
+    }
+
+    #[test]
+    fn it_rejects_invalid_error_props() {
+        let raw: &'static str = r#"{
+            "$exception_list": []
+        }"#;
+
+        let props: Result<ErrProps, Error> = serde_json::from_str(&raw);
+        assert!(props.is_ok());
+        assert_eq!(props.unwrap().exception_list.len(), 0);
+
+        let raw: &'static str = r#"{
+            "$exception_list": [{
+                "type": "UnhandledRejection"
+            }]
+        }"#;
+
+        let props: Result<ErrProps, Error> = serde_json::from_str(&raw);
+        assert!(props.is_err());
+        assert_eq!(
+            props.unwrap_err().to_string(),
+            "missing field `value` at line 4 column 13"
+        );
+
+        let raw: &'static str = r#"{
+            "$exception_list": [{
+                "typo": "UnhandledRejection",
+                "value": "x"
+            }]
+        }"#;
+
+        let props: Result<ErrProps, Error> = serde_json::from_str(&raw);
+        assert!(props.is_err());
+        assert_eq!(
+            props.unwrap_err().to_string(),
+            "missing field `type` at line 5 column 13"
+        );
     }
 }
diff --git a/rust/cymbal/tests/static/raw_ch_exception_list.json b/rust/cymbal/tests/static/raw_ch_exception_list.json
@@ -0,0 +1,11 @@
+{
+    "uuid": "019295b1-519f-71a6-aacf-c97b5db73696",
+    "event": "$exception",
+    "properties": "{\"$os\":\"Mac OS X\",\"$os_version\":\"10.15.7\",\"$browser\":\"Chrome\",\"$device_type\":\"Desktop\",\"$current_url\":\"https://eu.posthog.com/project/12557/feature_flags/31624\",\"$host\":\"eu.posthog.com\",\"$pathname\":\"/project/12557/feature_flags/31624\",\"$raw_user_agent\":\"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/129.0.0.0 Safari/537.36\",\"$browser_version\":129,\"$browser_language\":\"en-GB\",\"$screen_height\":1080,\"$screen_width\":1920,\"$viewport_height\":934,\"$viewport_width\":1920,\"$lib\":\"web\",\"$lib_version\":\"1.170.1\",\"$insert_id\":\"xjfjg606eo2x7n4x\",\"$time\":1729088278.943,\"distinct_id\":\"pQC9X9Fe7BPzJXVxpY0fx37UwFOCd1vXHzh8rjUPv1G\",\"$device_id\":\"018ccedb-d598-79bb-94e0-4751a3b956f4\",\"$console_log_recording_enabled_server_side\":true,\"$autocapture_disabled_server_side\":false,\"$web_vitals_enabled_server_side\":true,\"$exception_capture_enabled_server_side\":true,\"$exception_capture_endpoint\":\"/e/\",\"realm\":\"cloud\",\"email_service_available\":true,\"slack_service_available\":true,\"commit_sha\":\"bafa32953e\",\"$user_id\":\"pQC9X9Fe7BPzJXVxpY0fx37UwFOCd1vXHzh8rjUPv1G\",\"is_demo_project\":false,\"$groups\":{\"project\":\"018c1057-288d-0000-93bb-3bd44c845f22\",\"organization\":\"018afaa6-8b2e-0000-2311-d58d2df832ad\",\"customer\":\"cus_P5B9QmoUKLAUlx\",\"instance\":\"https://eu.posthog.com\"},\"has_billing_plan\":true,\"$referrer\":\"$direct\",\"$referring_domain\":\"$direct\",\"$session_recording_start_reason\":\"session_id_changed\",\"$exception_list\":[{\"type\":\"UnhandledRejection\",\"value\":\"Unexpected usage\",\"stacktrace\":{\"frames\":[{\"filename\":\"https://app-static.eu.posthog.com/static/chunk-PGUQKT6S.js\",\"function\":\"?\",\"in_app\":true,\"lineno\":64,\"colno\":25112},{\"filename\":\"https://app-static.eu.posthog.com/static/chunk-PGUQKT6S.js\",\"function\":\"n.loadForeignModule\",\"in_app\":true,\"lineno\":64,\"colno\":15003}]},\"mechanism\":{\"handled\":false,\"synthetic\":false}}],\"$exception_level\":\"error\",\"$exception_personURL\":\"https://us.posthog.com/project/sTMFPsFhdP1Ssg/person/pQC9X9Fe7BPzJXVxpY0fx37UwFOCd1vXHzh8rjUPv1G\",\"token\":\"sTMFPsFhdP1Ssg\",\"$session_id\":\"019295b0-db2b-7e02-8010-0a1c4db680df\",\"$window_id\":\"019295b0-db2b-7e02-8010-0a1dee88e5f5\",\"$lib_custom_api_host\":\"https://internal-t.posthog.com\",\"$is_identified\":true,\"$lib_rate_limit_remaining_tokens\":97.28999999999999,\"$sent_at\":\"2024-10-16T14:17:59.543000+00:00\",\"$geoip_city_name\":\"Lisbon\",\"$geoip_city_confidence\":null,\"$geoip_country_name\":\"Portugal\",\"$geoip_country_code\":\"PT\",\"$geoip_country_confidence\":null,\"$geoip_continent_name\":\"Europe\",\"$geoip_continent_code\":\"EU\",\"$geoip_postal_code\":\"1269-001\",\"$geoip_postal_code_confidence\":null,\"$geoip_latitude\":38.731,\"$geoip_longitude\":-9.1373,\"$geoip_accuracy_radius\":100,\"$geoip_time_zone\":\"Europe/Lisbon\",\"$geoip_subdivision_1_code\":\"11\",\"$geoip_subdivision_1_name\":\"Lisbon\",\"$geoip_subdivision_1_confidence\":null,\"$lib_version__major\":1,\"$lib_version__minor\":170,\"$lib_version__patch\":1,\"$group_2\":\"018c1057-288d-0000-93bb-3bd44c845f22\",\"$group_0\":\"018afaa6-8b2e-0000-2311-d58d2df832ad\",\"$group_3\":\"cus_P5B9QmoUKLAUlx\",\"$group_1\":\"https://eu.posthog.com\"}",
+    "timestamp": "2024-10-16T07:17:59.088000-07:00",
+    "team_id": 2,
+    "distinct_id": "pQC9X9Fe7BPzJXVxpY0fx37UwFOCd1vXHzh8rjUPv1G",
+    "elements_chain": "",
+    "created_at": "2024-10-16T07:18:00.100000-07:00",
+    "person_mode": "full"
+}
diff --git a/rust/cymbal/tests/types.rs b/rust/cymbal/tests/types.rs
@@ -6,7 +6,7 @@ use serde_json::Value;
 
 #[test]
 fn serde_passthrough() {
-    let raw: &'static str = include_str!("./static/raw_ch_exception.json");
+    let raw: &'static str = include_str!("./static/raw_ch_exception_list.json");
     let before: Value = serde_json::from_str(raw).unwrap();
     let raw: ClickHouseEvent = serde_json::from_str(raw).unwrap();