Merge branch 'master' into fix/goblin-logging-perf

Author: supervacuus

.github/CODEOWNERS

@@ -13,6 +13,7 @@
 
 # Files co-owned by Emerge Tools team
 /apple-catalog-parsing @getsentry/emerge-tools @getsentry/owners-sentry-cli
+/src/api/data_types/snapshots.rs @getsentry/emerge-tools @getsentry/owners-sentry-cli
 /src/commands/build @getsentry/emerge-tools @getsentry/owners-sentry-cli
 /src/utils/build @getsentry/emerge-tools @getsentry/owners-sentry-cli
 /tests/integration/build @getsentry/emerge-tools @getsentry/owners-sentry-cli

CHANGELOG.md

@@ -2,9 +2,17 @@
 
 ## Unreleased
 
+### Fixes
+
+- (sourcemaps) Skip non-base64 embedded sourcemaps during injection ([#3243](https://github.com/getsentry/sentry-cli/pull/3243))
+
 ### New Features ✨
 
 - Add `sentry-cli build download` command to download installable builds (IPA/APK) by build ID ([#3221](https://github.com/getsentry/sentry-cli/pull/3221)).
+- Add `sentry-cli code-mappings upload` command to bulk upload code mappings from a JSON file ([#3207](https://github.com/getsentry/sentry-cli/pull/3207), [#3208](https://github.com/getsentry/sentry-cli/pull/3208), [#3209](https://github.com/getsentry/sentry-cli/pull/3209), [#3210](https://github.com/getsentry/sentry-cli/pull/3210)).
+  - Code mappings link stack trace paths (e.g. `com/example/module`) to source paths in your repository (e.g. `src/main/java/com/example/module`), enabling Sentry to display source context and link directly to your code from error stack traces.
+  - Repository name and default branch are automatically inferred from your local git remotes, or can be specified explicitly with `--repo` and `--default-branch`.
+  - Large mapping files are automatically split into batches for upload.
 
 ## 3.3.3
 

package-lock.json

@@ -7,7 +7,6 @@ use serde_json::Value;
 
 use super::VcsInfo;
 
-const IMAGE_FILE_NAME_FIELD: &str = "image_file_name";
 const WIDTH_FIELD: &str = "width";
 const HEIGHT_FIELD: &str = "height";
 
@@ -42,16 +41,7 @@ pub struct ImageMetadata {
 }
 
 impl ImageMetadata {
-    pub fn new(
-        image_file_name: String,
-        width: u32,
-        height: u32,
-        mut extra: HashMap<String, Value>,
-    ) -> Self {
-        extra.insert(
-            IMAGE_FILE_NAME_FIELD.to_owned(),
-            Value::String(image_file_name),
-        );
+    pub fn new(width: u32, height: u32, mut extra: HashMap<String, Value>) -> Self {
         extra.insert(WIDTH_FIELD.to_owned(), Value::from(width));
         extra.insert(HEIGHT_FIELD.to_owned(), Value::from(height));
 
@@ -68,18 +58,16 @@ mod tests {
     #[test]
     fn cli_managed_fields_override_sidecar_fields() {
         let extra = serde_json::from_value(json!({
-            (IMAGE_FILE_NAME_FIELD): "from-sidecar.png",
             (WIDTH_FIELD): 1,
             (HEIGHT_FIELD): 2,
             "custom": "keep-me"
         }))
         .unwrap();
 
-        let metadata = ImageMetadata::new("from-cli.png".to_owned(), 100, 200, extra);
+        let metadata = ImageMetadata::new(100, 200, extra);
         let serialized = serde_json::to_value(metadata).unwrap();
 
         let expected = json!({
-            (IMAGE_FILE_NAME_FIELD): "from-cli.png",
             (WIDTH_FIELD): 100,
             (HEIGHT_FIELD): 200,
             "custom": "keep-me"

src/api/data_types/snapshots.rs

@@ -331,11 +331,28 @@ fn upload_images(
 
     let mut many_builder = session.many();
     let mut manifest_entries = HashMap::new();
-    let image_count = images.len();
-
+    let mut collisions: HashMap<String, Vec<String>> = HashMap::new();
+    let mut kept_paths: HashMap<String, String> = HashMap::new();
     for image in images {
         debug!("Processing image: {}", image.path.display());
 
+        let image_file_name = image
+            .relative_path
+            .file_name()
+            .unwrap_or_default()
+            .to_string_lossy()
+            .into_owned();
+
+        let relative_path = crate::utils::fs::path_as_url(&image.relative_path);
+
+        if manifest_entries.contains_key(&image_file_name) {
+            collisions
+                .entry(image_file_name)
+                .or_default()
+                .push(relative_path);
+            continue;
+        }
+
         let hash = compute_sha256_hash(&image.path)?;
         let file = runtime
             .block_on(tokio::fs::File::open(&image.path))
@@ -353,33 +370,40 @@ fn upload_images(
                 .expiration_policy(expiration),
         );
 
-        let image_file_name = image
-            .relative_path
-            .file_name()
-            .unwrap_or_default()
-            .to_string_lossy()
-            .into_owned();
-
-        let extra = read_sidecar_metadata(&image.path).unwrap_or_else(|err| {
+        let mut extra = read_sidecar_metadata(&image.path).unwrap_or_else(|err| {
             warn!("Error reading sidecar metadata, ignoring it instead: {err:#}");
             HashMap::new()
         });
+        extra.insert("content_hash".to_owned(), serde_json::Value::String(hash));
 
+        kept_paths.insert(image_file_name.clone(), relative_path);
         manifest_entries.insert(
-            hash,
-            ImageMetadata::new(image_file_name, image.width, image.height, extra),
+            image_file_name,
+            ImageMetadata::new(image.width, image.height, extra),
         );
     }
 
+    if !collisions.is_empty() {
+        let mut details = String::new();
+        for (name, excluded_paths) in &collisions {
+            let mut all_paths = vec![kept_paths[name].as_str()];
+            all_paths.extend(excluded_paths.iter().map(|s| s.as_str()));
+            details.push_str(&format!("\n  {name}: {}", all_paths.join(", ")));
+        }
+        warn!("Some images share identical file names. Only the first occurrence of each is included:{details}");
+    }
+
     let result = runtime.block_on(async { many_builder.send().error_for_failures().await });
 
+    let uploaded_count = manifest_entries.len();
+
     match result {
         Ok(()) => {
             println!(
                 "{} Uploaded {} image {}",
                 style(">").dim(),
-                style(image_count).yellow(),
-                if image_count == 1 { "file" } else { "files" }
+                style(uploaded_count).yellow(),
+                if uploaded_count == 1 { "file" } else { "files" }
             );
             Ok(manifest_entries)
         }
@@ -391,7 +415,7 @@ fn upload_images(
                 eprintln!("  {}", style(format!("{error:#}")).red());
                 error_count += 1;
             }
-            anyhow::bail!("Failed to upload {error_count} out of {image_count} images")
+            anyhow::bail!("Failed to upload {error_count} out of {uploaded_count} images")
         }
     }
 }

src/commands/build/snapshots.rs

@@ -4,11 +4,16 @@ use anyhow::{bail, Context as _, Result};
 use clap::{Arg, ArgMatches, Command};
 use log::debug;
 
-use crate::api::{Api, BulkCodeMapping, BulkCodeMappingResult, BulkCodeMappingsRequest};
+use crate::api::{
+    Api, BulkCodeMapping, BulkCodeMappingResult, BulkCodeMappingsRequest, BulkCodeMappingsResponse,
+};
 use crate::config::Config;
 use crate::utils::formatting::Table;
 use crate::utils::vcs;
 
+/// Maximum number of mappings the backend accepts per request.
+const BATCH_SIZE: usize = 300;
+
 pub fn make_command(command: Command) -> Command {
     command
         .about("Upload code mappings for a project from a JSON file. Each mapping pairs a stack trace root (e.g. com/example/module) with the corresponding source path in your repository (e.g. modules/module/src/main/java/com/example/module).")
@@ -72,31 +77,47 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
     )?;
 
     let mapping_count = mappings.len();
-    let request = BulkCodeMappingsRequest {
-        project: &project,
-        repository: &repo_name,
-        default_branch: &default_branch,
-        mappings: &mappings,
-    };
+    let total_batches = mapping_count.div_ceil(BATCH_SIZE);
 
     println!("Uploading {mapping_count} code mapping(s)...");
 
     let api = Api::current();
-    let response = api
-        .authenticated()?
-        .bulk_upload_code_mappings(&org, &request)?;
+    let authenticated = api.authenticated()?;
+
+    let merged: MergedResponse = mappings
+        .chunks(BATCH_SIZE)
+        .enumerate()
+        .map(|(i, batch)| {
+            if total_batches > 1 {
+                println!("Sending batch {}/{total_batches}...", i + 1);
+            }
+            let request = BulkCodeMappingsRequest {
+                project: &project,
+                repository: &repo_name,
+                default_branch: &default_branch,
+                mappings: batch,
+            };
+            authenticated
+                .bulk_upload_code_mappings(&org, &request)
+                .map_err(|err| format!("Batch {}/{total_batches} failed: {err}", i + 1))
+        })
+        .collect();
+
+    // Display error details (successful mappings are summarized in counts only).
+    print_error_table(&merged.mappings);
 
-    print_results_table(response.mappings);
+    for err in &merged.batch_errors {
+        println!("{err}");
+    }
+
+    let total_errors = merged.errors + merged.batch_errors.len() as u64;
     println!(
-        "Created: {}, Updated: {}, Errors: {}",
-        response.created, response.updated, response.errors
+        "Created: {}, Updated: {}, Errors: {total_errors}",
+        merged.created, merged.updated
     );
 
-    if response.errors > 0 {
-        bail!(
-            "{} mapping(s) failed to upload. See errors above.",
-            response.errors
-        );
+    if total_errors > 0 {
+        bail!("{total_errors} error(s) during upload. See details above.");
     }
 
     Ok(())
@@ -196,30 +217,61 @@ fn infer_default_branch(git_repo: Option<&git2::Repository>, remote_name: Option
         })
 }
 
-fn print_results_table(mappings: Vec<BulkCodeMappingResult>) {
+fn print_error_table(mappings: &[BulkCodeMappingResult]) {
+    if !mappings.iter().any(|r| r.status == "error") {
+        return;
+    }
+
     let mut table = Table::new();
     table
         .title_row()
         .add("Stack Root")
         .add("Source Root")
-        .add("Status");
+        .add("Detail");
 
-    for result in mappings {
-        let status = match result.detail {
-            Some(detail) if result.status == "error" => format!("error: {detail}"),
-            _ => result.status,
-        };
+    for result in mappings.iter().filter(|r| r.status == "error") {
+        let detail = result.detail.as_deref().unwrap_or("unknown error");
         table
             .add_row()
             .add(&result.stack_root)
             .add(&result.source_root)
-            .add(&status);
+            .add(detail);
     }
 
     table.print();
     println!();
 }
 
+#[derive(Default)]
+struct MergedResponse {
+    created: u64,
+    updated: u64,
+    errors: u64,
+    mappings: Vec<BulkCodeMappingResult>,
+    batch_errors: Vec<String>,
+}
+
+impl FromIterator<Result<BulkCodeMappingsResponse, String>> for MergedResponse {
+    fn from_iter<I>(iter: I) -> Self
+    where
+        I: IntoIterator<Item = Result<BulkCodeMappingsResponse, String>>,
+    {
+        let mut merged = Self::default();
+        for result in iter {
+            match result {
+                Ok(response) => {
+                    merged.created += response.created;
+                    merged.updated += response.updated;
+                    merged.errors += response.errors;
+                    merged.mappings.extend(response.mappings);
+                }
+                Err(err) => merged.batch_errors.push(err),
+            }
+        }
+        merged
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/commands/code_mappings/upload.rs

@@ -790,7 +790,12 @@ impl SourceMapProcessor {
 
                         let Ok(mut decoded) = data_encoding::BASE64.decode(encoded.as_bytes())
                         else {
-                            bail!("Invalid embedded sourcemap in source file {source_url}");
+                            // The data URL isn't valid base64, so we can't use it.
+                            // This commonly happens when minified bundles contain
+                            // sourceMappingURL strings inside template literals
+                            // (e.g from bundled terser or babel)
+                            warn!("Skipping {source_url}: embedded sourcemap is not valid base64");
+                            continue;
                         };
 
                         let mut sourcemap =