feat(inject): Make sourcemap discovery smarter

src/utils/sourcemaps.rs

@@ -766,6 +766,13 @@ impl SourceMapProcessor {
 
         let mut report = InjectReport::default();
 
+        let mut sourcemaps = self
+            .sources
+            .values()
+            .filter_map(|s| (s.ty == SourceFileType::SourceMap).then_some(s.url.clone()))
+            .collect::<Vec<_>>();
+        sourcemaps.sort();
+
         for (source_url, sourcemap_url) in self.sourcemap_references.iter_mut() {
             // We only allow injection into files that match the extension
             if !url_matches_extension(source_url, js_extensions) {
@@ -826,8 +833,22 @@ impl SourceMapProcessor {
                         } else {
                             // Handle external sourcemaps
 
-                            let sourcemap_url =
+                            let normalized =
                                 inject::normalize_sourcemap_url(source_url, sourcemap_url);
+                            let matches = inject::find_matching_paths(&sourcemaps, &normalized);
+
+                            let sourcemap_url = match &matches[..] {
+                                [] => normalized,
+                                [x] => x.to_string(),
+                                _ => {
+                                    warn!("Ambiguous matches for sourcemap path {normalized}:");
+                                    for path in matches {
+                                        warn!("{path}");
+                                    }
+                                    normalized
+                                }
+                            };
+
                             match self.sources.get_mut(&sourcemap_url) {
                                 None => {
                                     debug!("Sourcemap file {} not found", sourcemap_url);
@@ -979,54 +1000,59 @@ impl Default for SourceMapProcessor {
     }
 }
 
-#[test]
-fn test_split_url() {
-    assert_eq!(split_url("/foo.js"), (Some(""), "foo", Some("js")));
-    assert_eq!(split_url("foo.js"), (None, "foo", Some("js")));
-    assert_eq!(split_url("foo"), (None, "foo", None));
-    assert_eq!(split_url("/foo"), (Some(""), "foo", None));
-    assert_eq!(
-        split_url("/foo.deadbeef0123.js"),
-        (Some(""), "foo", Some("deadbeef0123.js"))
-    );
-    assert_eq!(
-        split_url("/foo/bar/baz.js"),
-        (Some("/foo/bar"), "baz", Some("js"))
-    );
-}
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_split_url() {
+        assert_eq!(split_url("/foo.js"), (Some(""), "foo", Some("js")));
+        assert_eq!(split_url("foo.js"), (None, "foo", Some("js")));
+        assert_eq!(split_url("foo"), (None, "foo", None));
+        assert_eq!(split_url("/foo"), (Some(""), "foo", None));
+        assert_eq!(
+            split_url("/foo.deadbeef0123.js"),
+            (Some(""), "foo", Some("deadbeef0123.js"))
+        );
+        assert_eq!(
+            split_url("/foo/bar/baz.js"),
+            (Some("/foo/bar"), "baz", Some("js"))
+        );
+    }
 
-#[test]
-fn test_unsplit_url() {
-    assert_eq!(&unsplit_url(Some(""), "foo", Some("js")), "/foo.js");
-    assert_eq!(&unsplit_url(None, "foo", Some("js")), "foo.js");
-    assert_eq!(&unsplit_url(None, "foo", None), "foo");
-    assert_eq!(&unsplit_url(Some(""), "foo", None), "/foo");
-}
+    #[test]
+    fn test_unsplit_url() {
+        assert_eq!(&unsplit_url(Some(""), "foo", Some("js")), "/foo.js");
+        assert_eq!(&unsplit_url(None, "foo", Some("js")), "foo.js");
+        assert_eq!(&unsplit_url(None, "foo", None), "foo");
+        assert_eq!(&unsplit_url(Some(""), "foo", None), "/foo");
+    }
 
-#[test]
-fn test_join() {
-    assert_eq!(&join_url("app:///", "foo.html").unwrap(), "app:///foo.html");
-    assert_eq!(&join_url("app://", "foo.html").unwrap(), "app:///foo.html");
-    assert_eq!(&join_url("~/", "foo.html").unwrap(), "~/foo.html");
-    assert_eq!(
-        &join_url("app:///", "/foo.html").unwrap(),
-        "app:///foo.html"
-    );
-    assert_eq!(&join_url("app://", "/foo.html").unwrap(), "app:///foo.html");
-    assert_eq!(
-        &join_url("https:///example.com/", "foo.html").unwrap(),
-        "https://example.com/foo.html"
-    );
-    assert_eq!(
-        &join_url("https://example.com/", "foo.html").unwrap(),
-        "https://example.com/foo.html"
-    );
-}
+    #[test]
+    fn test_join() {
+        assert_eq!(&join_url("app:///", "foo.html").unwrap(), "app:///foo.html");
+        assert_eq!(&join_url("app://", "foo.html").unwrap(), "app:///foo.html");
+        assert_eq!(&join_url("~/", "foo.html").unwrap(), "~/foo.html");
+        assert_eq!(
+            &join_url("app:///", "/foo.html").unwrap(),
+            "app:///foo.html"
+        );
+        assert_eq!(&join_url("app://", "/foo.html").unwrap(), "app:///foo.html");
+        assert_eq!(
+            &join_url("https:///example.com/", "foo.html").unwrap(),
+            "https://example.com/foo.html"
+        );
+        assert_eq!(
+            &join_url("https://example.com/", "foo.html").unwrap(),
+            "https://example.com/foo.html"
+        );
+    }
 
-#[test]
-fn test_url_matches_extension() {
-    assert!(url_matches_extension("foo.js", &["js"][..]));
-    assert!(!url_matches_extension("foo.mjs", &["js"][..]));
-    assert!(url_matches_extension("foo.mjs", &["js", "mjs"][..]));
-    assert!(!url_matches_extension("js", &["js"][..]));
+    #[test]
+    fn test_url_matches_extension() {
+        assert!(url_matches_extension("foo.js", &["js"][..]));
+        assert!(!url_matches_extension("foo.mjs", &["js"][..]));
+        assert!(url_matches_extension("foo.mjs", &["js", "mjs"][..]));
+        assert!(!url_matches_extension("js", &["js"][..]));
+    }
 }

src/utils/sourcemaps/inject.rs

@@ -378,6 +378,60 @@ pub fn normalize_sourcemap_url(source_url: &str, sourcemap_url: &str) -> String
     format!("{}{}", &joined[..cutoff], clean_path(&joined[cutoff..]))
 }
 
+/// Returns a list of those paths among `candidate_paths` that differ from `expected_path` in
+/// at most one segment (modulo `.` segments).
+///
+/// The differing segment cannot be the last one (i.e., the filename).
+///
+/// If `expected_path` occurs among the `candidate_paths`, no other paths will be returned since
+/// that is considered a unique best match.
+///
+/// The intended usecase is finding sourcemaps even if they reside in a different directory; see
+/// the `test_find_matching_paths_sourcemaps` test for a minimal example.
+pub fn find_matching_paths(candidate_paths: &[String], expected_path: &str) -> Vec<String> {
+    let mut matches = Vec::new();
+    for candidate in candidate_paths {
+        let mut expected_segments = expected_path
+            .split('/')
+            .filter(|&segment| segment != ".")
+            .peekable();
+        let mut candidate_segments = candidate
+            .split('/')
+            .filter(|&segment| segment != ".")
+            .peekable();
+
+        // If there is a candidate that is exactly equal to the goal path,
+        // return only that one.
+        if Iterator::eq(candidate_segments.clone(), expected_segments.clone()) {
+            return vec![candidate.clone()];
+        }
+
+        // Iterate through both paths and discard segments so long as they are equal.
+        while candidate_segments
+            .peek()
+            .zip(expected_segments.peek())
+            .map_or(false, |(x, y)| x == y)
+        {
+            candidate_segments.next();
+            expected_segments.next();
+        }
+
+        // The next segments (if there are any left) must be where the paths disagree.
+        candidate_segments.next();
+        expected_segments.next();
+
+        // The rest of both paths must agree and be nonempty, so at least the filenames definitely
+        // must agree.
+        if candidate_segments.peek().is_some()
+            && Iterator::eq(candidate_segments, expected_segments)
+        {
+            matches.push(candidate.clone());
+        }
+    }
+
+    matches
+}
+
 #[cfg(test)]
 mod tests {
     use std::io::Write;
@@ -386,7 +440,7 @@ mod tests {
 
     use crate::utils::fs::TempFile;
 
-    use super::{fixup_js_file, fixup_sourcemap, normalize_sourcemap_url, replace_sourcemap_url};
+    use super::*;
 
     #[test]
     fn test_fixup_sourcemap() {
@@ -601,4 +655,69 @@ more text
 "#;
         assert_eq!(std::str::from_utf8(&js_contents).unwrap(), expected);
     }
+
+    #[test]
+    fn test_find_matching_paths_unique() {
+        let expected = "./foo/bar/baz/quux";
+        let candidates = &[
+            "./foo/baz/quux".to_string(),
+            "foo/baar/baz/quux".to_string(),
+        ];
+
+        assert_eq!(
+            find_matching_paths(candidates, expected),
+            vec!["foo/baar/baz/quux"]
+        );
+
+        let candidates = &[
+            "./foo/baz/quux".to_string(),
+            "foo/baar/baz/quux".to_string(),
+            "./foo/bar/baz/quux".to_string(),
+        ];
+
+        assert_eq!(find_matching_paths(candidates, expected), vec![expected]);
+    }
+
+    #[test]
+    fn test_find_matching_paths_ambiguous() {
+        let expected = "./foo/bar/baz/quux";
+        let candidates = &[
+            "./foo/bar/baaz/quux".to_string(),
+            "foo/baar/baz/quux".to_string(),
+        ];
+
+        assert_eq!(find_matching_paths(candidates, expected), candidates,);
+    }
+
+    #[test]
+    fn test_find_matching_paths_filename() {
+        let expected = "./foo/bar/baz/quux";
+        let candidates = &[
+            "./foo/bar/baz/nop".to_string(),
+            "foo/baar/baz/quux".to_string(),
+        ];
+
+        assert_eq!(
+            find_matching_paths(candidates, expected),
+            ["foo/baar/baz/quux".to_string()]
+        );
+    }
+
+    #[test]
+    fn test_find_matching_paths_sourcemaps() {
+        let candidates = &[
+            "./project/maps/index.js.map".to_string(),
+            "./project/maps/page/index.js.map".to_string(),
+        ];
+
+        assert_eq!(
+            find_matching_paths(candidates, "project/code/index.js.map"),
+            &["./project/maps/index.js.map"]
+        );
+
+        assert_eq!(
+            find_matching_paths(candidates, "project/code/page/index.js.map"),
+            &["./project/maps/page/index.js.map"]
+        );
+    }
 }

tests/integration/_cases/sourcemaps/sourcemaps-inject-split-ambiguous.trycmd

@@ -0,0 +1,24 @@
+```
+$ sentry-cli sourcemaps inject ./code ./maps ./maps2 --log-level warn
+? success
+> Searching ./code
+> Found 2 files
+> Searching ./maps
+> Found 2 files
+> Searching ./maps2
+> Found 1 file
+> Analyzing 5 sources
+> Injecting debug ids
+  WARN    [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] Ambiguous matches for sourcemap path ./code/foo/index.js.map:
+  WARN    [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] ./maps/foo/index.js.map
+  WARN    [..]-[..]-[..] [..]:[..]:[..].[..] +[..]:[..] ./maps2/foo/index.js.map
+
+Source Map Debug ID Injection Report
+  Modified: The following source files have been modified to have debug ids
+    [..]-[..]-[..]-[..]-[..] - ./code/foo/index.js
+    [..]-[..]-[..]-[..]-[..] - ./code/index.js
+  Ignored: The following sourcemap files already have debug ids
+    [..]-[..]-[..]-[..]-[..] - ./maps/index.js.map
+
+
+```

tests/integration/_cases/sourcemaps/sourcemaps-inject-split.trycmd

@@ -0,0 +1,21 @@
+```
+$ sentry-cli sourcemaps inject ./code ./maps
+? success
+> Searching ./code
+> Found 2 files
+> Searching ./maps
+> Found 2 files
+> Analyzing 4 sources
+> Injecting debug ids
+
+Source Map Debug ID Injection Report
+  Modified: The following source files have been modified to have debug ids
+    [..]-[..]-[..]-[..]-[..] - ./code/foo/index.js
+    [..]-[..]-[..]-[..]-[..] - ./code/index.js
+  Modified: The following sourcemap files have been modified to have debug ids
+    [..]-[..]-[..]-[..]-[..] - ./maps/foo/index.js.map
+  Ignored: The following sourcemap files already have debug ids
+    [..]-[..]-[..]-[..]-[..] - ./maps/index.js.map
+
+
+```

tests/integration/sourcemaps/inject.rs

@@ -86,3 +86,34 @@ fn command_sourcemaps_inject_output_embedded() {
     let parsed: serde_json::Value = serde_json::from_slice(&decoded).unwrap();
     assert_eq!(parsed["mappings"], ";;;;");
 }
+
+#[test]
+fn command_sourcemaps_inject_output_split() {
+    let testcase_cwd_path = "tests/integration/_cases/sourcemaps/sourcemaps-inject-split.in/";
+    if std::path::Path::new(testcase_cwd_path).exists() {
+        remove_dir_all(testcase_cwd_path).unwrap();
+    }
+    copy_recursively(
+        "tests/integration/_fixtures/inject_split/",
+        testcase_cwd_path,
+    )
+    .unwrap();
+
+    register_test("sourcemaps/sourcemaps-inject-split.trycmd");
+}
+
+#[test]
+fn command_sourcemaps_inject_output_split_ambiguous() {
+    let testcase_cwd_path =
+        "tests/integration/_cases/sourcemaps/sourcemaps-inject-split-ambiguous.in/";
+    if std::path::Path::new(testcase_cwd_path).exists() {
+        remove_dir_all(testcase_cwd_path).unwrap();
+    }
+    copy_recursively(
+        "tests/integration/_fixtures/inject_split_ambiguous/",
+        testcase_cwd_path,
+    )
+    .unwrap();
+
+    register_test("sourcemaps/sourcemaps-inject-split-ambiguous.trycmd");
+}