From 07c4383b966f37f8b483dce5219df55b4e247745 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Mon, 10 Nov 2025 12:37:38 +0100 Subject: [PATCH] feat(proguard): Upload ProGuard with chunked uploading (#2918) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Description ⚠️ **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. ### Issues - 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. --- CHANGELOG.md | 4 + src/commands/upload_proguard.rs | 96 +++++-------------- src/utils/proguard/mapping.rs | 5 - .../upload_proguard-no-upload.trycmd | 1 - .../upload_proguard/upload_proguard.trycmd | 9 -- .../debug_files/get-chunk-upload.json | 2 +- tests/integration/test_utils/test_manager.rs | 6 -- tests/integration/upload_proguard.rs | 11 +-- 8 files changed, 28 insertions(+), 106 deletions(-) delete mode 100644 tests/integration/_cases/upload_proguard/upload_proguard.trycmd diff --git a/CHANGELOG.md b/CHANGELOG.md index 65fe9962c9..5ac9fe0f9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ we should rename this section to "Unreleased" --> - 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)). diff --git a/src/commands/upload_proguard.rs b/src/commands/upload_proguard.rs index 2fa6ba4470..e7dbb71d0b 100644 --- a/src/commands/upload_proguard.rs +++ b/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::("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::("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(()) } diff --git a/src/utils/proguard/mapping.rs b/src/utils/proguard/mapping.rs index bfd86dea4f..40117cad5e 100644 --- a/src/utils/proguard/mapping.rs +++ b/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 diff --git a/tests/integration/_cases/upload_proguard/upload_proguard-no-upload.trycmd b/tests/integration/_cases/upload_proguard/upload_proguard-no-upload.trycmd index 62f5f693f0..cb30fac691 100644 --- a/tests/integration/_cases/upload_proguard/upload_proguard-no-upload.trycmd +++ b/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. ``` diff --git a/tests/integration/_cases/upload_proguard/upload_proguard.trycmd b/tests/integration/_cases/upload_proguard/upload_proguard.trycmd deleted file mode 100644 index ac28898ceb..0000000000 --- a/tests/integration/_cases/upload_proguard/upload_proguard.trycmd +++ /dev/null @@ -1,9 +0,0 @@ -``` -$ sentry-cli upload-proguard tests/integration/_fixtures/proguard.txt -? success -warning: ignoring proguard mapping 'tests/integration/_fixtures/proguard.txt': Proguard mapping does not contain line information -> compressing mappings -> uploading mappings -> Uploaded a total of 0 new mapping files - -``` diff --git a/tests/integration/_responses/debug_files/get-chunk-upload.json b/tests/integration/_responses/debug_files/get-chunk-upload.json index 98a8aa5dac..0b56be5b03 100644 --- a/tests/integration/_responses/debug_files/get-chunk-upload.json +++ b/tests/integration/_responses/debug_files/get-chunk-upload.json @@ -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"] } diff --git a/tests/integration/test_utils/test_manager.rs b/tests/integration/test_utils/test_manager.rs index 6aec668f0d..bbec5385d1 100644 --- a/tests/integration/test_utils/test_manager.rs +++ b/tests/integration/test_utils/test_manager.rs @@ -201,12 +201,6 @@ impl AssertCmdTestManager { self } - /// Set a custom environment variable for the test. - pub fn env(mut self, variable: impl AsRef, value: impl AsRef) -> Self { - self.command.env(variable, value); - self - } - /// Run the command and perform assertions. /// /// This function asserts both the mocks and the command result. diff --git a/tests/integration/upload_proguard.rs b/tests/integration/upload_proguard.rs index 97d9f0f2fb..d6c148bc80 100644 --- a/tests/integration/upload_proguard.rs +++ b/tests/integration/upload_proguard.rs @@ -8,13 +8,7 @@ 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(); + TestManager::new().register_trycmd_test("upload_proguard/*.trycmd"); } #[test] @@ -72,7 +66,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 +159,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 +281,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) }