fix: allow optional abs_path and lineno for JsFrame

crates/symbolicator-js/src/interface.rs

@@ -229,9 +229,11 @@ pub struct JsFrame {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub module: Option<String>,
 
-    pub abs_path: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub abs_path: Option<String>,
 
-    pub lineno: u32,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub lineno: Option<u32>,
 
     #[serde(skip_serializing_if = "Option::is_none")]
     pub colno: Option<u32>,

crates/symbolicator-js/src/lookup.rs

@@ -195,16 +195,18 @@ impl SourceMapLookup {
             for frame in &mut stacktrace.frames {
                 // NOTE: some older JS SDK versions did not correctly strip a leading `async `
                 // prefix from the `abs_path`, which we will work around here.
-                if let Some(abs_path) = frame.abs_path.strip_prefix("async ") {
-                    frame.abs_path = abs_path.to_owned();
+                if let Some(abs_path) = frame.abs_path.as_ref().and_then(|s| s.strip_prefix("async ")) {
+                    frame.abs_path = Some(abs_path.to_owned());
                 }
-                let abs_path = &frame.abs_path;
-                if self.modules_by_abs_path.contains_key(abs_path) {
-                    continue;
+
+                if let Some(abs_path) = &frame.abs_path {
+                    if self.modules_by_abs_path.contains_key(abs_path) {
+                        continue;
+                    }
+                    let cached_module = SourceMapModule::new(abs_path, None);
+                    self.modules_by_abs_path
+                        .insert(abs_path.to_owned(), cached_module);
                 }
-                let cached_module = SourceMapModule::new(abs_path, None);
-                self.modules_by_abs_path
-                    .insert(abs_path.to_owned(), cached_module);
             }
         }
     }

crates/symbolicator-js/src/symbolication.rs

@@ -64,7 +64,7 @@ impl SourceMapService {
                             .entry(raw_frame.platform.clone())
                             .or_default() += 1;
                         errors.insert(JsModuleError {
-                            abs_path: raw_frame.abs_path.clone(),
+                            abs_path: raw_frame.abs_path.clone().unwrap_or_default(),
                             kind: err,
                         });
                         symbolicated_frames.push(raw_frame.clone());
@@ -102,24 +102,29 @@ async fn symbolicate_js_frame(
     should_apply_source_context: bool,
     stats: &mut SymbolicationStats,
 ) -> Result<JsFrame, JsModuleErrorKind> {
+    let Some(abs_path) = &raw_frame.abs_path else {
+        return Err(JsModuleErrorKind::InvalidAbsPath);
+    };
+
     // we check for a valid line (i.e. >= 1) first, as we want to avoid resolving / scraping the minified
     // file in that case. we frequently saw 0 line/col values in combination with non-js files,
     // and we want to avoid scraping a bunch of html files in that case.
-    let line = if raw_frame.lineno > 0 {
-        raw_frame.lineno
-    } else {
-        return Err(JsModuleErrorKind::InvalidLocation {
-            line: raw_frame.lineno,
-            col: raw_frame.colno,
-        });
+    let line = match raw_frame.lineno {
+        Some(lineno) if lineno > 0 => lineno,
+        lineno => {
+            return Err(JsModuleErrorKind::InvalidLocation {
+                line: lineno.unwrap_or(0),
+                col: raw_frame.colno,
+            });
+        }
     };
 
     let col = raw_frame.colno.unwrap_or_default();
 
-    let module = lookup.get_module(&raw_frame.abs_path).await;
+    let module = lookup.get_module(&abs_path).await;
 
     tracing::trace!(
-        abs_path = &raw_frame.abs_path,
+        abs_path = &abs_path,
         ?module,
         "Module for `abs_path`"
     );
@@ -154,7 +159,7 @@ async fn symbolicate_js_frame(
         .map(|entry| entry.sourcemap_url())
         .ok()
         .flatten()
-        .unwrap_or_else(|| raw_frame.abs_path.clone());
+        .unwrap_or_else(|| raw_frame.abs_path.clone().unwrap_or_default());
 
     let (smcache, resolved_with, sourcemap_origin) = match &module.smcache {
         Some(smcache) => match &smcache.entry {
@@ -229,31 +234,32 @@ async fn symbolicate_js_frame(
 
     if let Some(filename) = token.file_name() {
         let mut filename = filename.to_string();
-        frame.abs_path = module
+        frame.abs_path = Some(module
             .source_file_base()
             .map(|base| join_paths(base, &filename))
-            .unwrap_or_else(|| filename.clone());
+            .unwrap_or_else(|| filename.clone()));
 
         if filename.starts_with("webpack:") {
             filename = fixup_webpack_filename(&filename);
             frame.module = Some(generate_module(&filename));
         }
 
-        frame.in_app = is_in_app(&frame.abs_path, &filename);
+        let abs_path = frame.abs_path.clone().unwrap_or_default();
+        frame.in_app = is_in_app(&abs_path, &filename);
 
         if frame.module.is_none()
-            && (frame.abs_path.starts_with("http:")
-                || frame.abs_path.starts_with("https:")
-                || frame.abs_path.starts_with("webpack:")
-                || frame.abs_path.starts_with("app:"))
+            && (abs_path.starts_with("http:")
+                || abs_path.starts_with("https:")
+                || abs_path.starts_with("webpack:")
+                || abs_path.starts_with("app:"))
         {
-            frame.module = Some(generate_module(&frame.abs_path));
+            frame.module = Some(generate_module(&abs_path));
         }
 
         frame.filename = Some(filename);
     }
 
-    frame.lineno = token.line().saturating_add(1);
+    frame.lineno = Some(token.line().saturating_add(1));
     frame.colno = Some(token.column().saturating_add(1));
 
     if !should_apply_source_context {
@@ -264,7 +270,7 @@ async fn symbolicate_js_frame(
         if let Some(file_source) = file.source() {
             if let Err(err) = apply_source_context(&mut frame, file_source) {
                 errors.insert(JsModuleError {
-                    abs_path: raw_frame.abs_path.clone(),
+                    abs_path: raw_frame.abs_path.clone().unwrap_or_default(),
                     kind: err,
                 });
             }
@@ -291,7 +297,7 @@ async fn symbolicate_js_frame(
                 // It's arguable whether we should collect it, but this is what monolith does now,
                 // and it might be useful to indicate incorrect sentry-cli rewrite behavior.
                 errors.insert(JsModuleError {
-                    abs_path: raw_frame.abs_path.clone(),
+                    abs_path: raw_frame.abs_path.clone().unwrap_or_default(),
                     kind: JsModuleErrorKind::MissingSourceContent {
                         source: file_key
                             .and_then(|key| key.abs_path().map(|path| path.to_string()))
@@ -307,11 +313,14 @@ async fn symbolicate_js_frame(
 }
 
 fn apply_source_context(frame: &mut JsFrame, source: &str) -> Result<(), JsModuleErrorKind> {
-    let lineno = frame.lineno as usize;
+    let Some(lineno) = frame.lineno else {
+        return Ok(());
+    };
+
     let column = frame.colno.map(|col| col as usize).unwrap_or_default();
 
     if let Some((pre_context, context_line, post_context)) =
-        get_context_lines(source, lineno, column, None)
+        get_context_lines(source, lineno as usize, column, None)
     {
         frame.pre_context = pre_context;
         frame.context_line = Some(context_line);

crates/symbolicator-js/tests/integration/snapshots/integration__sourcemap__invalid_abs_path.snap

@@ -0,0 +1,22 @@
+---
+source: crates/symbolicator-js/tests/integration/sourcemap.rs
+assertion_line: 380
+expression: response
+---
+stacktraces:
+  - frames:
+      - filename: test.js
+        lineno: 1
+        colno: 1
+        data:
+          symbolicated: false
+raw_stacktraces:
+  - frames:
+      - filename: test.js
+        lineno: 1
+        colno: 1
+        data:
+          symbolicated: false
+errors:
+  - abs_path: ""
+    type: invalid_abs_path

crates/symbolicator-js/tests/integration/snapshots/integration__sourcemap__invalid_lineno.snap

@@ -0,0 +1,22 @@
+---
+source: crates/symbolicator-js/tests/integration/sourcemap.rs
+assertion_line: 397
+expression: response
+---
+stacktraces:
+  - frames:
+      - filename: test.js
+        abs_path: http//example.com/test.min.js
+        data:
+          symbolicated: false
+raw_stacktraces:
+  - frames:
+      - filename: test.js
+        abs_path: http//example.com/test.min.js
+        data:
+          symbolicated: false
+errors:
+  - abs_path: http//example.com/test.min.js
+    type: invalid_location
+    line: 0
+    col: ~

crates/symbolicator-js/tests/integration/sourcemap.rs

@@ -363,6 +363,40 @@ async fn test_malformed_abs_path() {
     assert_snapshot!(response);
 }
 
+#[tokio::test]
+async fn test_invalid_abs_path() {
+    let (symbolication, _cache_dir) = setup_service(|_| ());
+    let (_srv, source) = sourcemap_server("", |_url, _query| json!([]));
+
+    let frames = r#"[{
+        "filename": "test.js",
+        "lineno": 1,
+        "colno": 1
+    }]"#;
+
+    let request = make_js_request(source, frames, "[]", String::from("release"), None);
+    let response = symbolication.symbolicate_js(request).await;
+
+    assert_snapshot!(response);
+}
+
+
+#[tokio::test]
+async fn test_invalid_lineno() {
+    let (symbolication, _cache_dir) = setup_service(|_| ());
+    let (_srv, source) = sourcemap_server("", |_url, _query| json!([]));
+
+    let frames = r#"[{
+        "abs_path": "http//example.com/test.min.js",
+        "filename": "test.js"
+    }]"#;
+
+    let request = make_js_request(source, frames, "[]", String::from("release"), None);
+    let response = symbolication.symbolicate_js(request).await;
+
+    assert_snapshot!(response);
+}
+
 #[tokio::test]
 async fn test_fetch_error() {
     let (symbolication, _cache_dir) = setup_service(|_| ());

crates/symbolicli/src/main.rs

@@ -727,8 +727,8 @@ mod event {
             function: value.function,
             filename: value.filename,
             module: value.module,
-            abs_path: value.abs_path?,
-            lineno: value.lineno?,
+            abs_path: value.abs_path,
+            lineno: value.lineno,
             colno: value.colno,
             pre_context: value.pre_context,
             context_line: value.context_line,

crates/symbolicli/src/output.rs

@@ -240,7 +240,8 @@ fn print_compact_js(mut response: CompletedJsSymbolicationResponse) {
         let filename = filename.unwrap_or_default();
         row.add_cell(cell!(filename));
 
-        row.add_cell(cell!(lineno));
+        let line = lineno.map(|line| line.to_string()).unwrap_or_default();
+        row.add_cell(cell!(line));
 
         let col = colno.map(|col| col.to_string()).unwrap_or_default();
         row.add_cell(cell!(col));
@@ -290,10 +291,19 @@ fn print_pretty_js(mut response: CompletedJsSymbolicationResponse) {
             table.add_row(row![r->"  File:", name]);
         }
 
-        if let Some(col) = *colno {
-            table.add_row(row![r->"  Line/Column:", format!("{lineno}:{col}")]);
-        } else {
-            table.add_row(row![r->"  Line:", format!("{lineno}")]);
+        match (colno, lineno) {
+            (Some(col), Some(line)) => {
+                table.add_row(row![r->"  Line/Column:", format!("{}:{}", line, col)]);
+            }
+            (Some(col), None) => {
+                table.add_row(row![r->"  Column:", format!("{}", col)]);
+            }
+            (None, Some(line)) => {
+                table.add_row(row![r->"  Line:", format!("{}", line)]);
+            }
+            (None, None) => {
+                table.add_row(row![r->"  Line/Column:", "N/A"]);
+            }
         }
 
         table.add_empty_row();