fix(sourcemaps): Remove distinction between Source and `MinifiedSou…

…rce` for `sourcemaps inject` (#1958)

Until now, the sentry-cli sourcemaps inject command had only been injecting debug IDs into files that we had identified as being minified. However, the intended behavior for this command is for the command to inject debug IDs into all JS source files at the user-specified path; we should not distinguish between minified and non-minified files at all.

This PR implements the intended behavior.

Users who currently call the sourcemap inject command with a path containing files that should not be injected with debug IDs (such as a path containing both transpiled and source JS files) should update the path passed to only include transpiled JS files, or should use other command line arguments (e.g. --ignore) to exclude the source files from having debug IDs injected.

This PR is intended to provide a quick fix to implement the behavior. In the future, we plan to completely refactor and simplify the logic powering the sourcemaps inject command.

Fixes GH-1955

src/utils/sourcemaps.rs

@@ -21,7 +21,6 @@ use symbolic::debuginfo::sourcebundle::SourceFileType;
 use url::Url;
 
 use crate::api::Api;
-use crate::utils::enc::decode_unknown_string;
 use crate::utils::file_search::ReleaseFileMatch;
 use crate::utils::file_upload::{
  initialize_legacy_release_upload, FileUpload, SourceFile, SourceFiles, UploadContext,
@@ -37,20 +36,6 @@ pub mod inject;
 /// Data URLs are used to embed sourcemaps directly in javascript source files.
 const DATA_PREAMBLE: &str = "data:application/json;base64,";
 
-fn is_likely_minified_js(code: &[u8]) -> bool {
- // if we have a debug id or source maps location reference, this is a minified file
- if let Ok(code) = std::str::from_utf8(code) {
- if discover_debug_id(code).is_some() || discover_sourcemaps_location(code).is_some() {
- return true;
- }
- }
- if let Ok(code_str) = decode_unknown_string(code) {
- might_be_minified::analyze_str(&code_str).is_likely_minified()
- } else {
- false
- }
-}
-
 fn join_url(base_url: &str, url: &str) -> Result<String> {
  if base_url.starts_with("~/") {
  match Url::parse(&format!("http://{base_url}"))?.join(url) {
@@ -300,20 +285,6 @@ impl SourceMapProcessor {
  && sourcemap::ram_bundle::is_ram_bundle_slice(&file.contents)
  {
  (SourceFileType::IndexedRamBundle, None)
- } else if file
- .path
- .file_name()
- .and_then(OsStr::to_str)
- .map(|x| x.contains(".min."))
- .unwrap_or(false)
- || is_likely_minified_js(&file.contents)
- {
- (
- SourceFileType::MinifiedSource,
- std::str::from_utf8(&file.contents)
- .ok()
- .and_then(discover_debug_id),
- )
  } else if is_hermes_bytecode(&file.contents) {
  // This is actually a big hack:
  // For the react-native Hermes case, we skip uploading the bytecode bundle,
@@ -323,7 +294,24 @@ impl SourceMapProcessor {
  file.contents.clear();
  (SourceFileType::MinifiedSource, None)
  } else {
- (SourceFileType::Source, None)
+ // Here, we use MinifiedSource for historical reasons. We used to guess whether
+ // a JS file was a minified file or a source file, and we would treat these files
+ // differently when uploading or injecting them. However, the desired behavior is
+ // and has always been to treat all JS files the same, since users should be
+ // responsible for providing the file paths for only files they would like to have
+ // uploaded or injected. The minified file guessing furthermore was not reliable,
+ // since minification is not a necessary step in the JS build process.
+ //
+ // We use MinifiedSource here rather than Source because we want to treat all JS
+ // files the way we used to treat minified files only. To use Source, we would need
+ // to analyze all possible code paths that check this value, and update those as
+ // well. To keep the change minimal, we use MinifiedSource here.
+ (
+ SourceFileType::MinifiedSource,
+ std::str::from_utf8(&file.contents)
+ .ok()
+ .and_then(discover_debug_id),
+ )
  };
 
  let mut source_file = SourceFile {

tests/integration/_cases/react_native/xcode-upload-source-maps-release_and_dist_from_env.trycmd

@@ -22,7 +22,8 @@ Processing react-native sourcemaps for Sentry upload.
 
 Source Map Upload Report
  Scripts
- ~/react-native-xcode-bundle.js.map (no sourcemap ref)
  ~/react-native-xcode-bundle.js (sourcemap at react-native-xcode-bundle.map)
+ ~/react-native-xcode-bundle.js.map (no sourcemap ref)
+ - warning: could not determine a source map reference (Could not auto-detect referenced sourcemap for ~/react-native-xcode-bundle.js.map)
 
 ```

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

@@ -12,6 +12,7 @@ Source Map Debug ID Injection Report
  Modified: The following source files have been modified to have debug ids
  [..]-[..]-[..]-[..]-[..] - ./server/app/page.min.js
  [..]-[..]-[..]-[..]-[..] - ./server/chunks/1.min.js
+ [..]-[..]-[..]-[..]-[..] - ./server/chunks/flight-server-css-manifest.js
  [..]-[..]-[..]-[..]-[..] - ./server/flight-manifest.js
  [..]-[..]-[..]-[..]-[..] - ./server/pages/_document.min.js
  [..]-[..]-[..]-[..]-[..] - ./server/pages/api/hello.min.js

tests/integration/_cases/sourcemaps/sourcemaps-inject-not-compiled.trycmd

@@ -0,0 +1,5 @@
+```
+$ sentry-cli sourcemaps inject .
+? success
+...
+```

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

@@ -12,6 +12,7 @@ Source Map Debug ID Injection Report
  Modified: The following source files have been modified to have debug ids
  [..]-[..]-[..]-[..]-[..] - ./server/app/page.js
  [..]-[..]-[..]-[..]-[..] - ./server/chunks/1.js
+ [..]-[..]-[..]-[..]-[..] - ./server/chunks/flight-server-css-manifest.js
  [..]-[..]-[..]-[..]-[..] - ./server/dummy_embedded.js
  [..]-[..]-[..]-[..]-[..] - ./server/dummy_hosted.js
  [..]-[..]-[..]-[..]-[..] - ./server/flight-manifest.js

tests/integration/_cases/sourcemaps/sourcemaps-upload-some-debugids.trycmd

@@ -25,9 +25,10 @@ $ sentry-cli sourcemaps upload tests/integration/_fixtures/upload_some_debugids
 
 Source Map Upload Report
  Scripts
- ~/server/chunks/flight-server-css-manifest.js (no sourcemap ref)
  ~/server/app/page.js (sourcemap at page.js.map)
  ~/server/chunks/1.js (sourcemap at 1.js.map)
+ ~/server/chunks/flight-server-css-manifest.js (no sourcemap ref)
+ - warning: could not determine a source map reference (Could not auto-detect referenced sourcemap for ~/server/chunks/flight-server-css-manifest.js)
  ~/server/dummy_embedded.js (embedded sourcemap)
  ~/server/edge-runtime-webpack.js (sourcemap at edge-runtime-webpack.js.map, debug id 2297b93d-928d-421e-8910-127c786382dd)
  ~/server/flight-manifest.js (no sourcemap ref)

tests/integration/_fixtures/inject-not-compiled/not-compiled.js

@@ -0,0 +1,5 @@
+function helloWorld() {
+ return 'Hello, World!';
+}
+
+helloWorld();

tests/integration/sourcemaps/inject.rs

@@ -145,3 +145,23 @@ fn command_sourcemaps_inject_bundlers() {
  assert_eq!(actual_map, expected_map, "CJS, bundler: {bundler}");
  }
 }
+
+#[test]
+fn command_sourcemaps_inject_not_compiled() {
+ let testcase_cwd_path =
+ "tests/integration/_cases/sourcemaps/sourcemaps-inject-not-compiled.in/";
+ if std::path::Path::new(testcase_cwd_path).exists() {
+ remove_dir_all(testcase_cwd_path).unwrap();
+ }
+
+ copy_recursively(
+ "tests/integration/_fixtures/inject-not-compiled",
+ testcase_cwd_path,
+ )
+ .unwrap();
+
+ register_test("sourcemaps/sourcemaps-inject-not-compiled.trycmd");
+
+ let file_contents = fs::read_to_string(format!("{testcase_cwd_path}not-compiled.js")).unwrap();
+ assert!(file_contents.contains("//# debugId="));
+}