feat(proguard): Upload ProGuard with chunked uploading (#2918)

⚠️ **Breaking change:** Do not merge until ready to release in a major.

Use chunked uploading for `sentry-cli upload-proguard`, as chunked uploads are generally more reliable than non-chunked uploads, and chunked uploading is the uploading method used by all other file uploading commands in Sentry CLI.

Users who previously had set the `SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD` environment variable to opt into the chunk uploading behavior no longer need to set this variable, as all ProGuard files are always uploaded with chunked uploading.

- Resolves #2328
- Resolves [CLI-13](https://linear.app/getsentry/issue/CLI-13/make-chunk-uploads-the-default-for-proguard-files)

_______

BREAKING CHANGE: The `sentry-cli upload-proguard` file can no longer upload to Sentry servers which lack support for receiving ProGuard mappings via chunked upload.

Author: szokeasaurusrex

CHANGELOG.md

@@ -10,6 +10,10 @@
 
 - Removed the `upload-proguard` subcommand's `--app-id`, `--version`, and `--version-code` arguments ([#2876](https://github.com/getsentry/sentry-cli/pull/2876)). Users using these arguments should stop using them, as they are unnecessary. The information passed to these arguments is no longer visible in Sentry.
 
+### Improvements
+
+- The `sentry-cli upload-proguard` command now uses chunked uploading by default ([#2918](https://github.com/getsentry/sentry-cli/pull/2918)). Users who previously set the `SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD` environment variable to opt into this behavior no longer need to set the variable.
+
 ### Fixes
 
 - Fixed misleading error message claiming the server doesn't support chunk uploading when the actual error was a non-existent organization ([#2930](https://github.com/getsentry/sentry-cli/pull/2930)).

src/commands/upload_proguard.rs

@@ -1,4 +1,3 @@
-use std::env;
 use std::io;
 
 use anyhow::{bail, Error, Result};
@@ -9,16 +8,13 @@ use symbolic::common::ByteView;
 use uuid::Uuid;
 
 use crate::api::Api;
+use crate::api::ChunkUploadCapability;
 use crate::config::Config;
 use crate::utils::android::dump_proguard_uuids_as_properties;
 use crate::utils::args::ArgExt as _;
-use crate::utils::fs::TempFile;
 use crate::utils::proguard;
 use crate::utils::proguard::ProguardMapping;
 use crate::utils::system::QuietExit;
-use crate::utils::ui::{copy_with_progress, make_byte_progress_bar};
-
-const CHUNK_UPLOAD_ENV_VAR: &str = "SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD";
 
 pub fn make_command(command: Command) -> Command {
     command
@@ -167,79 +163,31 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
     let api = Api::current();
     let config = Config::current();
 
-    // Don't initialize these until we confirm the user did not pass the --no-upload flag,
-    // or if we are using chunked uploading. This is because auth token, org, and project
-    // are not needed for the no-upload case.
-    let authenticated_api;
-    let (org, project);
-
-    if env::var(CHUNK_UPLOAD_ENV_VAR) == Ok("1".into()) {
-        log::warn!(
-            "EXPERIMENTAL FEATURE: Uploading proguard mappings using chunked uploading. \
-             Some functionality may be unavailable when using chunked uploading. Please unset \
-             the {CHUNK_UPLOAD_ENV_VAR} variable if you encounter any \
-             problems."
-        );
-
-        authenticated_api = api.authenticated()?;
-        (org, project) = config.get_org_and_project(matches)?;
-
-        let chunk_upload_options = authenticated_api.get_chunk_upload_options(&org)?;
-
-        proguard::chunk_upload(&mappings, chunk_upload_options, &org, &project)?;
-    } else {
-        if mappings.is_empty() && matches.get_flag("require_one") {
-            println!();
-            eprintln!("{}", style("error: found no mapping files to upload").red());
-            return Err(QuietExit(1).into());
-        }
-
-        println!("{} compressing mappings", style(">").dim());
-        let tf = TempFile::create()?;
-        {
-            let mut zip = zip::ZipWriter::new(tf.open()?);
-            for mapping in &mappings {
-                let pb = make_byte_progress_bar(mapping.len() as u64);
-                zip.start_file(
-                    format!("proguard/{}.txt", mapping.uuid()),
-                    zip::write::SimpleFileOptions::default(),
-                )?;
-                copy_with_progress(&pb, &mut mapping.as_ref(), &mut zip)?;
-                pb.finish_and_clear();
-            }
-        }
+    if mappings.is_empty() && matches.get_flag("require_one") {
+        println!();
+        eprintln!("{}", style("error: found no mapping files to upload").red());
+        return Err(QuietExit(1).into());
+    }
 
-        // write UUIDs into the mapping file.
-        if let Some(p) = matches.get_one::<String>("write_properties") {
-            let uuids: Vec<_> = mappings.iter().map(|x| x.uuid()).collect();
-            dump_proguard_uuids_as_properties(p, &uuids)?;
-        }
+    // write UUIDs into the mapping file.
+    if let Some(p) = matches.get_one::<String>("write_properties") {
+        let uuids: Vec<_> = mappings.iter().map(|x| x.uuid()).collect();
+        dump_proguard_uuids_as_properties(p, &uuids)?;
+    }
 
-        if matches.get_flag("no_upload") {
-            println!("{} skipping upload.", style(">").dim());
-            return Ok(());
-        }
+    if matches.get_flag("no_upload") {
+        println!("{} skipping upload.", style(">").dim());
+        return Ok(());
+    }
 
-        println!("{} uploading mappings", style(">").dim());
-        (org, project) = config.get_org_and_project(matches)?;
+    let authenticated_api = api.authenticated()?;
+    let (org, project) = config.get_org_and_project(matches)?;
 
-        authenticated_api = api.authenticated()?;
+    let chunk_upload_options = authenticated_api.get_chunk_upload_options(&org)?;
 
-        let rv = authenticated_api
-            .region_specific(&org)
-            .upload_dif_archive(&project, tf.path())?;
-        println!(
-            "{} Uploaded a total of {} new mapping files",
-            style(">").dim(),
-            style(rv.len()).yellow()
-        );
-        if !rv.is_empty() {
-            println!("Newly uploaded debug symbols:");
-            for df in rv {
-                println!("  {}", style(&df.id()).dim());
-            }
-        }
+    if chunk_upload_options.supports(ChunkUploadCapability::Proguard) {
+        proguard::chunk_upload(&mappings, chunk_upload_options, &org, &project)
+    } else {
+        Err(anyhow::anyhow!("Server does not support uploading ProGuard mappings via chunked upload. Please update your Sentry server."))
     }
-
-    Ok(())
 }

src/utils/proguard/mapping.rs

@@ -19,11 +19,6 @@ pub struct ProguardMapping<'a> {
 }
 
 impl<'a> ProguardMapping<'a> {
-    /// Get the length of the mapping in bytes.
-    pub fn len(&self) -> usize {
-        self.bytes.len()
-    }
-
     /// Get the UUID of the mapping.
     pub fn uuid(&self) -> Uuid {
         self.uuid

tests/integration/_cases/upload_proguard/upload_proguard-no-upload.trycmd

@@ -2,7 +2,6 @@
 $ sentry-cli upload-proguard tests/integration/_fixtures/proguard.txt --no-upload
 ? success
 warning: ignoring proguard mapping 'tests/integration/_fixtures/proguard.txt': Proguard mapping does not contain line information
-> compressing mappings
 > skipping upload.
 
 ```

tests/integration/_cases/upload_proguard/upload_proguard.trycmd

@@ -7,5 +7,5 @@
   "concurrency": 8,
   "hashAlgorithm": "sha1",
   "compression": ["gzip"],
-  "accept": ["debug_files", "release_files", "pdbs", "portablepdbs", "sources", "bcsymbolmaps"]
+  "accept": ["debug_files", "release_files", "pdbs", "portablepdbs", "sources", "bcsymbolmaps", "proguard"]
 }

tests/integration/_responses/debug_files/get-chunk-upload.json

@@ -201,12 +201,6 @@ impl AssertCmdTestManager {
         self
     }
 
-    /// Set a custom environment variable for the test.
-    pub fn env(mut self, variable: impl AsRef<OsStr>, value: impl AsRef<OsStr>) -> Self {
-        self.command.env(variable, value);
-        self
-    }
-
     /// Run the command and perform assertions.
     ///
     /// This function asserts both the mocks and the command result.

tests/integration/test_utils/test_manager.rs

@@ -9,10 +9,6 @@ use crate::integration::{MockEndpointBuilder, TestManager};
 #[test]
 fn command_upload_proguard() {
     TestManager::new()
-        .mock_endpoint(
-            MockEndpointBuilder::new("POST", "/api/0/projects/wat-org/wat-project/files/dsyms/")
-                .with_response_body("[]"),
-        )
         .register_trycmd_test("upload_proguard/*.trycmd")
         .with_default_token();
 }
@@ -72,7 +68,6 @@ fn chunk_upload_already_there() {
             "tests/integration/_fixtures/upload_proguard/mapping.txt",
         ])
         .with_default_token()
-        .env("SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD", "1")
         .run_and_assert(AssertCommand::Success)
 }
 
@@ -166,7 +161,6 @@ fn chunk_upload_needs_upload() {
             "tests/integration/_fixtures/upload_proguard/mapping.txt",
         ])
         .with_default_token()
-        .env("SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD", "1")
         .run_and_assert(AssertCommand::Success)
 }
 
@@ -289,6 +283,5 @@ fn chunk_upload_two_files() {
             "tests/integration/_fixtures/upload_proguard/mapping-2.txt",
         ])
         .with_default_token()
-        .env("SENTRY_EXPERIMENTAL_PROGUARD_CHUNK_UPLOAD", "1")
         .run_and_assert(AssertCommand::Success)
 }