feat(err): improve js frame results (#26336)

rust/Cargo.toml

@@ -66,7 +66,7 @@ serde = { version = "1.0", features = ["derive"] }
 serde_derive = { version = "1.0" }
 serde_json = { version = "1.0" }
 serde_urlencoded = "0.7.1"
-sqlx = { version = "0.7", features = [
+sqlx = { version = "0.8.2", features = [
     "chrono",
     "json",
     "migrate",

rust/cyclotron-core/src/types.rs

@@ -1,6 +1,5 @@
 use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
-use sqlx::postgres::{PgHasArrayType, PgTypeInfo};
 use std::str::FromStr;
 use uuid::Uuid;
 
@@ -31,13 +30,6 @@ impl FromStr for JobState {
     }
 }
 
-impl PgHasArrayType for JobState {
-    fn array_type_info() -> sqlx::postgres::PgTypeInfo {
-        // Postgres default naming convention for array types is "_typename"
-        PgTypeInfo::with_name("_JobState")
-    }
-}
-
 // The chunk of data needed to enqueue a job
 #[derive(Debug, Deserialize, Serialize, Clone, Eq, PartialEq)]
 pub struct JobInit {

rust/cymbal/Cargo.toml

@@ -32,6 +32,7 @@ sqlx = { workspace = true }
 serde_json = { workspace = true }
 serde = { workspace = true }
 sourcemap = "9.0.0"
+symbolic = { version = "12.12.1", features = ["sourcemapcache"] }
 reqwest = { workspace = true }
 sha2 = "0.10.8"
 aws-config = { workspace = true }

rust/cymbal/src/app_context.rs

@@ -7,7 +7,7 @@ use tokio::sync::Mutex;
 use tracing::info;
 
 use crate::{
-    config::Config,
+    config::{init_global_state, Config},
     error::UnhandledError,
     frames::resolver::Resolver,
     hack::kafka::{create_kafka_producer, KafkaContext, SingleTopicConsumer},
@@ -33,6 +33,7 @@ pub struct AppContext {
 
 impl AppContext {
     pub async fn new(config: &Config) -> Result<Self, UnhandledError> {
+        init_global_state(config);
         let health_registry = HealthRegistry::new("liveness");
         let worker_liveness = health_registry
             .register("worker".to_string(), Duration::from_secs(60))

rust/cymbal/src/bin/test_js_resolution.rs

@@ -1,4 +1,4 @@
-use std::sync::Arc;
+use std::{cmp::min, collections::HashMap, sync::Arc};
 
 use cymbal::{
     config::Config,
@@ -15,34 +15,35 @@ use tokio::sync::Mutex;
 /**
 Input data gathered by running the following, then converting to json:
 SELECT
-    symbol_set.ref as filename,
-    contents::json->>'mangled_name' as "function",
-    (contents::json->>'in_app')::boolean as in_app,
-    CASE
-        WHEN contents::json->>'line' IS NOT NULL
-        THEN (contents::json->>'line')::int
-    END as lineno,
-    CASE
-        WHEN contents::json->>'column' IS NOT NULL
-        THEN (contents::json->>'column')::int
-    END as colno
+    contents::json->>'junk_drawer' as junk_drawer
 FROM posthog_errortrackingstackframe frame
 LEFT JOIN posthog_errortrackingsymbolset symbol_set
     ON frame.symbol_set_id = symbol_set.id
-WHERE (contents::json->>'resolved_name') is null
+WHERE (contents::json->>'resolved_name') is n
     AND contents::json->>'lang' = 'javascript'
+    AND contents::json->>'junk_drawer' IS NOT NULL
     AND symbol_set.storage_ptr IS NOT NULL;
 
-This doesn't actually work - we don't have the original line and column number, and
-so can't repeat the original resolution. I couldn't find a way to reverse that mapping
-with sourcemaps, so instead I'm going to temporarily add the raw frame to the resolve
-Frame.
 */
-const NAMELESS_FRAMES_IN_RAW_FMT: &str = include_str!("./nameless_frames_in_raw_format.json");
+const NAMELESS_FRAMES_IN_RAW_FMT: &str = include_str!("./no_resolved_name_raw_frames.json");
 
 #[tokio::main]
 async fn main() {
+    let start_at: usize = std::env::var("START_AT")
+        .unwrap_or("0".to_string())
+        .parse()
+        .expect("START_AT must be an integer");
+    let run_until: Option<usize> = std::env::var("RUN_UNTIL")
+        .ok()
+        .map(|s| s.parse().expect("RUN_UNTIL must be an integer"));
+
+    let early_exit = std::env::var("EARLY_EXIT").is_ok();
+
+    // I want a lot of line context while working on this
+    std::env::set_var("CONTEXT_LINE_COUNT", "1");
+
     let config = Config::init_with_defaults().unwrap();
+
     let provider = SourcemapProvider::new(&config);
     let cache = Arc::new(Mutex::new(SymbolSetCache::new(1_000_000_000)));
     let provider = Caching::new(provider, cache);
@@ -55,32 +56,56 @@ async fn main() {
     let frames: Vec<RawJSFrame> = frames
         .into_iter()
         .map(|f| {
-            let mut f = f;
-            let in_app = f["in_app"].as_str().unwrap() == "true";
-            f["in_app"] = Value::Bool(in_app);
-            let lineno: u32 = f["lineno"]
-                .as_str()
-                .unwrap()
-                .replace(",", "")
-                .parse()
-                .unwrap();
-            let colno: u32 = f["colno"]
-                .as_str()
-                .unwrap()
-                .replace(",", "")
-                .parse()
-                .unwrap();
-            f["lineno"] = Value::Number(lineno.into());
-            f["colno"] = Value::Number(colno.into());
-            serde_json::from_value(f).unwrap()
+            let junk: HashMap<String, Value> =
+                serde_json::from_str(f["junk_drawer"].as_str().unwrap()).unwrap();
+            serde_json::from_value(junk["raw_frame"].clone()).unwrap()
         })
         .collect();
 
-    for frame in frames {
+    let run_until = min(frames.len(), run_until.unwrap_or(frames.len()));
+
+    let mut failures = Vec::new();
+
+    let mut resolved = 0;
+    for (i, frame) in frames
+        .into_iter()
+        .enumerate()
+        .skip(start_at)
+        .take(run_until - start_at)
+    {
         let res = frame.resolve(0, &catalog).await.unwrap();
 
-        if res.resolved_name.is_none() {
-            panic!("Frame name not resolved: {:?}", frame);
+        println!("-------------------");
+        println!("Resolving frame {}", i);
+        println!("Input frame: {:?}", frame);
+        println!("Resolved: {}", res);
+        println!("-------------------");
+
+        if res.resolved_name.is_some() {
+            resolved += 1;
+        } else if early_exit {
+            break;
+        } else {
+            failures.push((frame.clone(), res, i));
         }
     }
+
+    println!("Failures:");
+    for failure in failures {
+        println!("-------------------");
+        println!(
+            "Failed to resolve name for frame {}, {:?}",
+            failure.2, failure.0
+        );
+        println!(
+            "Failure: {}",
+            failure.1.resolve_failure.as_deref().unwrap_or("unknown")
+        )
+    }
+
+    println!(
+        "Resolved {} out of {} frames",
+        resolved,
+        run_until - start_at
+    );
 }

rust/cymbal/src/config.rs

@@ -1,7 +1,12 @@
+use std::sync::atomic::{AtomicUsize, Ordering};
+
 use envconfig::Envconfig;
 
 use crate::hack::kafka::{ConsumerConfig, KafkaConfig};
 
+// TODO - I'm just too lazy to pipe this all the way through the resolve call stack
+pub static FRAME_CONTEXT_LINES: AtomicUsize = AtomicUsize::new(15);
+
 #[derive(Envconfig, Clone)]
 pub struct Config {
     #[envconfig(from = "BIND_HOST", default = "::")]
@@ -69,11 +74,21 @@ pub struct Config {
 
     #[envconfig(default = "600")]
     pub frame_cache_ttl_seconds: u64,
+
+    // Maximum number of lines of pre and post context to get per frame
+    #[envconfig(default = "15")]
+    pub context_line_count: usize,
 }
 
 impl Config {
     pub fn init_with_defaults() -> Result<Self, envconfig::Error> {
         ConsumerConfig::set_defaults("error-tracking-rs", "exception_symbolification_events");
-        Self::init_from_env()
+        let res = Self::init_from_env()?;
+        init_global_state(&res);
+        Ok(res)
     }
 }
+
+pub fn init_global_state(config: &Config) {
+    FRAME_CONTEXT_LINES.store(config.context_line_count, Ordering::Relaxed);
+}

rust/cymbal/src/error.rs

@@ -48,6 +48,9 @@ pub enum JsResolveErr {
     // We failed to parse a found source map
     #[error("Invalid source map: {0}")]
     InvalidSourceMap(String),
+    // We failed to parse a found source map cache
+    #[error("Invalid source map cache: {0}")]
+    InvalidSourceMapCache(String),
     // We found and parsed the source map, but couldn't find our frames token in it
     #[error("Token not found for frame: {0}:{1}:{2}")]
     TokenNotFound(String, u32, u32),

rust/cymbal/src/frames/resolver.rs

@@ -73,6 +73,7 @@ mod test {
     use httpmock::MockServer;
     use mockall::predicate;
     use sqlx::PgPool;
+    use symbolic::sourcemapcache::SourceMapCacheWriter;
 
     use crate::{
         config::Config,
@@ -157,6 +158,18 @@ mod test {
         test_stack.pop().unwrap()
     }
 
+    fn get_sourcemapcache_bytes() -> Vec<u8> {
+        let mut result = Vec::new();
+        let writer = SourceMapCacheWriter::new(
+            core::str::from_utf8(MINIFIED).unwrap(),
+            core::str::from_utf8(MAP).unwrap(),
+        )
+        .unwrap();
+
+        writer.serialize(&mut result).unwrap();
+        result
+    }
+
     fn expect_puts_and_gets(
         config: &Config,
         mut client: S3Client,
@@ -168,7 +181,7 @@ mod test {
             .with(
                 predicate::eq(config.object_storage_bucket.clone()),
                 predicate::str::starts_with(config.ss_prefix.clone()),
-                predicate::eq(Vec::from(MAP)),
+                predicate::always(), // We don't assert on what we store, because who cares
             )
             .returning(|_, _, _| Ok(()))
             .times(puts);
@@ -179,7 +192,7 @@ mod test {
                 predicate::eq(config.object_storage_bucket.clone()),
                 predicate::str::starts_with(config.ss_prefix.clone()),
             )
-            .returning(|_, _| Ok(Vec::from(MAP)))
+            .returning(|_, _| Ok(get_sourcemapcache_bytes()))
             .times(gets);
 
         client