fix(js): Don't apply minified source context to symbolicated frame (#…

…1449)

We clone the minified frame before applying source context to it, because otherwise the unminified frame might end up with nonsensical source context.

crates/symbolicator-js/src/symbolication.rs

@@ -118,17 +118,26 @@ async fn symbolicate_js_frame(
 
     // Apply source context to the raw frame. If it fails, we bail early, as it's not possible
     // to construct a `SourceMapCache` without the minified source anyway.
-    match &module.minified_source.entry {
+    let mut frame = match &module.minified_source.entry {
         Ok(minified_source) => {
+            // Clone the frame before applying source context to the raw frame.
+            // If we apply source context first, this can lead to the following confusing situation:
+            // 1. Apply source context to the minified frame
+            // 2. Clone it for the unminified frame
+            // 3. We don't have unminified source
+            // 4. The unminified frame contains the minified source context
+            let frame = raw_frame.clone();
             if should_apply_source_context {
                 apply_source_context(raw_frame, minified_source.contents())?
             }
+
+            frame
         }
         Err(CacheError::DownloadError(msg)) if msg == "Scraping disabled" => {
             return Err(JsModuleErrorKind::ScrapingDisabled);
         }
         Err(_) => return Err(JsModuleErrorKind::MissingSource),
-    }
+    };
 
     let sourcemap_label = &module
         .minified_source
@@ -159,7 +168,6 @@ async fn symbolicate_js_frame(
         None => return Ok(raw_frame.clone()),
     };
 
-    let mut frame = raw_frame.clone();
     frame.data.sourcemap = Some(sourcemap_label.clone());
     frame.data.resolved_with = Some(resolved_with);
 

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

@@ -0,0 +1,56 @@
+---
+source: crates/symbolicator-js/tests/integration/sourcemap.rs
+assertion_line: 552
+expression: response
+---
+stacktraces:
+  - frames:
+      - function: "function: \"HTMLDocument.<anonymous>\""
+        filename: index.html
+        abs_path: "http://example.com/index.html"
+        lineno: 283
+        colno: 17
+        in_app: false
+      - function: add
+        filename: file1.js
+        module: file1
+        abs_path: "http://example.com/file1.js"
+        lineno: 3
+        colno: 9
+        data:
+          sourcemap: "http://example.com/embedded.js.map"
+          resolved_with: release
+          symbolicated: true
+raw_stacktraces:
+  - frames:
+      - function: "function: \"HTMLDocument.<anonymous>\""
+        filename: index.html
+        abs_path: "http://example.com/index.html"
+        lineno: 283
+        colno: 17
+        in_app: false
+      - filename: embedded.js
+        abs_path: "http://example.com/embedded.js"
+        lineno: 1
+        colno: 39
+        context_line: "function add(a,b){\"use strict\";return a+b}function multiply(a,b){\"use strict\";return a*b}function divide(a,b){\"use strict\";try{return multip {snip}"
+        post_context:
+          - "//# sourceMappingURL=embedded.js.map"
+errors:
+  - abs_path: "http://example.com/embedded.js"
+    type: missing_source_content
+    source: "http://example.com/file1.js"
+    sourcemap: "http://example.com/embedded.js.map"
+  - abs_path: "http://example.com/index.html"
+    type: scraping_disabled
+scraping_attempts:
+  - url: "http://example.com/index.html"
+    status: failure
+    reason: disabled
+  - url: "http://example.com/embedded.js"
+    status: not_attempted
+  - url: "http://example.com/embedded.js.map"
+    status: not_attempted
+  - url: "http://example.com/file1.js"
+    status: failure
+    reason: disabled

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

@@ -513,6 +513,46 @@ async fn test_dart_async_name() {
     );
 }
 
+#[tokio::test]
+async fn test_no_source_contents() {
+    let (symbolication, _cache_dir) = setup_service(|_| ());
+    let (_srv, source) = sourcemap_server("11_no_source_contents", |url, _query| {
+        json!([{
+            "type": "file",
+            "id": "1",
+            "url": format!("{url}/embedded.js"),
+            "abs_path": "~/embedded.js",
+            "resolved_with": "release",
+        }, {
+            "type": "file",
+            "id": "2",
+            "url": format!("{url}/embedded.js.map"),
+            "abs_path": "~/embedded.js.map",
+            "resolved_with": "release",
+        }])
+    });
+
+    let frames = r#"[{
+        "function": "function: \"HTMLDocument.<anonymous>\"",
+        "abs_path": "http://example.com/index.html",
+        "filename": "index.html",
+        "lineno": 283,
+        "colno": 17,
+        "in_app": false
+    }, {
+        "abs_path": "http://example.com/embedded.js",
+        "filename": "embedded.js",
+        "lineno": 1,
+        "colno": 39
+    }]"#;
+
+    let request = make_js_request(source, frames, "[]", String::from("release"), None);
+    let response = symbolication.symbolicate_js(request).await;
+
+    // The sourcemap doesn't contain source context, so the symbolicated frame shouldn't have source context.
+    assert_snapshot!(response);
+}
+
 #[tokio::test]
 async fn e2e_node_debugid() {
     let (symbolication, _cache_dir) = setup_service(|_| ());