diff --git a/src/api/data_types/chunking/mobile_app.rs b/src/api/data_types/chunking/mobile_app.rs index b29f3a18b1..6223e78c30 100644 --- a/src/api/data_types/chunking/mobile_app.rs +++ b/src/api/data_types/chunking/mobile_app.rs @@ -9,7 +9,7 @@ pub struct ChunkedMobileAppRequest<'a> { pub checksum: Digest, pub chunks: &'a [Digest], #[serde(skip_serializing_if = "Option::is_none")] - pub git_sha: Option<&'a str>, + pub head_sha: Option<&'a str>, #[serde(skip_serializing_if = "Option::is_none")] pub build_configuration: Option<&'a str>, } @@ -20,4 +20,5 @@ pub struct AssembleMobileAppResponse { pub state: ChunkedFileState, pub missing_chunks: Vec, pub detail: Option, + pub artifact_id: Option, } diff --git a/src/api/mod.rs b/src/api/mod.rs index 41ac6f822c..33c175bfd2 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -1036,7 +1036,7 @@ impl<'a> AuthenticatedApi<'a> { project: &str, checksum: Digest, chunks: &[Digest], - git_sha: Option<&str>, + head_sha: Option<&str>, build_configuration: Option<&str>, ) -> ApiResult { let url = format!( @@ -1049,7 +1049,7 @@ impl<'a> AuthenticatedApi<'a> { .with_json_body(&ChunkedMobileAppRequest { checksum, chunks, - git_sha, + head_sha, build_configuration, })? .send()? diff --git a/src/commands/mobile_app/upload.rs b/src/commands/mobile_app/upload.rs index 25beb9fffa..b6e0757372 100644 --- a/src/commands/mobile_app/upload.rs +++ b/src/commands/mobile_app/upload.rs @@ -11,15 +11,14 @@ use clap::{Arg, ArgAction, ArgMatches, Command}; use indicatif::ProgressStyle; use itertools::Itertools as _; use log::{debug, info, warn}; -use sha1_smol::Digest; use symbolic::common::ByteView; use zip::write::SimpleFileOptions; use zip::{DateTime, ZipWriter}; -use crate::api::{Api, AuthenticatedApi, ChunkUploadCapability}; +use crate::api::{Api, AuthenticatedApi, ChunkUploadCapability, ChunkedFileState}; use crate::config::Config; use crate::utils::args::ArgExt as _; -use crate::utils::chunks::{upload_chunks, Chunk, ASSEMBLE_POLL_INTERVAL}; +use crate::utils::chunks::{upload_chunks, Chunk}; use crate::utils::fs::get_sha1_checksums; #[cfg(all(target_os = "macos", target_arch = "aarch64"))] use crate::utils::fs::TempDir; @@ -129,9 +128,10 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { let config = Config::current(); let (org, project) = config.get_org_and_project(matches)?; + let base_url = config.get_base_url()?; - let mut uploaded_paths = vec![]; - let mut errored_paths = vec![]; + let mut uploaded_paths_and_ids = vec![]; + let mut errored_paths_and_reasons = vec![]; for (path, zip) in normalized_zips { info!("Uploading file: {}", path.display()); let bytes = ByteView::open(zip.path())?; @@ -143,41 +143,50 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { sha.as_deref(), build_configuration, ) { - Ok(_) => { + Ok(artifact_id) => { info!("Successfully uploaded file: {}", path.display()); - uploaded_paths.push(path.to_path_buf()); + uploaded_paths_and_ids.push((path.to_path_buf(), artifact_id)); } Err(e) => { debug!("Failed to upload file at path {}: {}", path.display(), e); - errored_paths.push(path.to_path_buf()); + errored_paths_and_reasons.push((path.to_path_buf(), e)); } } } - if !errored_paths.is_empty() { + if !errored_paths_and_reasons.is_empty() { warn!( "Failed to upload {} file{}:", - errored_paths.len(), - if errored_paths.len() == 1 { "" } else { "s" } + errored_paths_and_reasons.len(), + if errored_paths_and_reasons.len() == 1 { + "" + } else { + "s" + } ); - for path in errored_paths { - warn!(" - {}", path.display()); - } - } - - println!( - "Successfully uploaded {} file{} to Sentry", - uploaded_paths.len(), - if uploaded_paths.len() == 1 { "" } else { "s" } - ); - if uploaded_paths.len() < 3 { - for path in &uploaded_paths { - println!(" - {}", path.display()); + for (path, reason) in errored_paths_and_reasons { + warn!(" - {} ({})", path.display(), reason); } } - if uploaded_paths.is_empty() { + if uploaded_paths_and_ids.is_empty() { bail!("Failed to upload any files"); + } else { + println!( + "Successfully uploaded {} file{} to Sentry", + uploaded_paths_and_ids.len(), + if uploaded_paths_and_ids.len() == 1 { + "" + } else { + "s" + } + ); + if uploaded_paths_and_ids.len() < 3 { + for (path, artifact_id) in &uploaded_paths_and_ids { + let url = format!("{base_url}/{org}/preprod/{project}/{artifact_id}"); + println!(" - {} ({url})", path.display()); + } + } } Ok(()) } @@ -337,6 +346,7 @@ fn normalize_directory(path: &Path) -> Result { Ok(temp_file) } +/// Returns artifact id if upload was successful. fn upload_file( api: &AuthenticatedApi, bytes: &[u8], @@ -344,7 +354,7 @@ fn upload_file( project: &str, sha: Option<&str>, build_configuration: Option<&str>, -) -> Result<()> { +) -> Result { const SELF_HOSTED_ERROR_HINT: &str = "If you are using a self-hosted Sentry server, \ update to the latest version of Sentry to use the mobile-app upload command."; @@ -371,7 +381,7 @@ fn upload_file( } let progress_style = - ProgressStyle::default_spinner().template("{spinner} Optimizing bundle for upload..."); + ProgressStyle::default_spinner().template("{spinner} Preparing for upload..."); let pb = ProgressBar::new_spinner(); pb.enable_steady_tick(100); pb.set_style(progress_style); @@ -384,76 +394,52 @@ fn upload_file( .map(|(data, checksum)| Chunk((*checksum, data))) .collect::>(); - pb.finish_with_duration("Finishing upload"); - - let response = - api.assemble_mobile_app(org, project, checksum, &checksums, sha, build_configuration)?; - chunks.retain(|Chunk((digest, _))| response.missing_chunks.contains(digest)); - - if !chunks.is_empty() { - let upload_progress_style = ProgressStyle::default_bar().template( - "{prefix:.dim} Uploading files...\ - \n{wide_bar} {bytes}/{total_bytes} ({eta})", - ); - upload_chunks(&chunks, &chunk_upload_options, upload_progress_style)?; - } else { - println!("Nothing to upload, all files are on the server"); - } - - poll_assemble( - api, - checksum, - &checksums, - org, - project, - sha, - build_configuration, - )?; - Ok(()) -} - -fn poll_assemble( - api: &AuthenticatedApi, - checksum: Digest, - chunks: &[Digest], - org: &str, - project: &str, - sha: Option<&str>, - build_configuration: Option<&str>, -) -> Result<()> { - debug!("Polling assemble for checksum: {}", checksum); - - let progress_style = ProgressStyle::default_spinner().template("{spinner} Processing files..."); - let pb = ProgressBar::new_spinner(); + pb.finish_with_duration("Preparing for upload"); + + // In the normal case we go through this loop exactly twice: + // 1. state=not_found + // server tells us the we need to send every chunk and we do so + // 2. artifact_id set so we're done (likely state=created) + // + // In the case where all the chunks are already on the server we go + // through only once: + // 1. state=ok, artifact_id set + // + // In the case where something went wrong (which could be on either + // iteration of the loop) we get: + // n. state=err, artifact_id unset + let result = loop { + let response = + api.assemble_mobile_app(org, project, checksum, &checksums, sha, build_configuration)?; + chunks.retain(|Chunk((digest, _))| response.missing_chunks.contains(digest)); + + if !chunks.is_empty() { + let upload_progress_style = ProgressStyle::default_bar().template( + "{prefix:.dim} Uploading files...\ + \n{wide_bar} {bytes}/{total_bytes} ({eta})", + ); + upload_chunks(&chunks, &chunk_upload_options, upload_progress_style)?; + } - pb.enable_steady_tick(100); - pb.set_style(progress_style); + // state.is_err() is not the same as this since it also returns + // true for ChunkedFileState::NotFound. + if response.state == ChunkedFileState::Error { + let message = response.detail.as_deref().unwrap_or("unknown error"); + bail!("Failed to process uploaded files: {}", message); + } - let response = loop { - let response = - api.assemble_mobile_app(org, project, checksum, chunks, sha, build_configuration)?; + if let Some(artifact_id) = response.artifact_id { + break Ok(artifact_id); + } if response.state.is_finished() { - break response; + bail!( + "File upload is_finished() but did not succeeded (returning artifact_id) or error" + ); } - - std::thread::sleep(ASSEMBLE_POLL_INTERVAL); }; - pb.finish_with_duration("Processing"); - - if response.state.is_err() { - let message = response.detail.as_deref().unwrap_or("unknown error"); - bail!("Failed to process uploaded files: {}", message); - } - - if response.state.is_pending() { - info!("File upload complete (processing pending on server)"); - } else { - info!("File processing complete"); - } - - Ok(()) + result } #[cfg(not(windows))] diff --git a/tests/integration/_cases/mobile_app/mobile_app-upload-apk-all-uploaded.trycmd b/tests/integration/_cases/mobile_app/mobile_app-upload-apk-all-uploaded.trycmd index 7da13e2bca..0e309d7f1c 100644 --- a/tests/integration/_cases/mobile_app/mobile_app-upload-apk-all-uploaded.trycmd +++ b/tests/integration/_cases/mobile_app/mobile_app-upload-apk-all-uploaded.trycmd @@ -2,8 +2,7 @@ $ sentry-cli mobile-app upload tests/integration/_fixtures/mobile_app/apk.apk ? success [..]WARN[..]EXPERIMENTAL: The mobile-app subcommand is experimental. The command is subject to breaking changes and may be removed without notice in any release. -Nothing to upload, all files are on the server Successfully uploaded 1 file to Sentry - - tests/integration/_fixtures/mobile_app/apk.apk + - tests/integration/_fixtures/mobile_app/apk.apk (http[..]/wat-org/preprod/wat-project/42) ``` diff --git a/tests/integration/_cases/mobile_app/mobile_app-upload-apk.trycmd b/tests/integration/_cases/mobile_app/mobile_app-upload-apk.trycmd index f202e997c1..1f2259717b 100644 --- a/tests/integration/_cases/mobile_app/mobile_app-upload-apk.trycmd +++ b/tests/integration/_cases/mobile_app/mobile_app-upload-apk.trycmd @@ -3,6 +3,6 @@ $ sentry-cli mobile-app upload tests/integration/_fixtures/mobile_app/apk.apk -- ? success [..]WARN[..]EXPERIMENTAL: The mobile-app subcommand is experimental. The command is subject to breaking changes and may be removed without notice in any release. Successfully uploaded 1 file to Sentry - - tests/integration/_fixtures/mobile_app/apk.apk + - tests/integration/_fixtures/mobile_app/apk.apk (http[..]/wat-org/preprod/wat-project/42) ``` diff --git a/tests/integration/_cases/mobile_app/mobile_app-upload-ipa.trycmd b/tests/integration/_cases/mobile_app/mobile_app-upload-ipa.trycmd index ffddcfcdc6..aafd34cafb 100644 --- a/tests/integration/_cases/mobile_app/mobile_app-upload-ipa.trycmd +++ b/tests/integration/_cases/mobile_app/mobile_app-upload-ipa.trycmd @@ -3,6 +3,6 @@ $ sentry-cli mobile-app upload tests/integration/_fixtures/mobile_app/ipa.ipa -- ? success [..]WARN[..]EXPERIMENTAL: The mobile-app subcommand is experimental. The command is subject to breaking changes and may be removed without notice in any release. Successfully uploaded 1 file to Sentry - - tests/integration/_fixtures/mobile_app/ipa.ipa + - tests/integration/_fixtures/mobile_app/ipa.ipa (http[..]/wat-org/preprod/wat-project/some-text-id) ``` diff --git a/tests/integration/mobile_app/upload.rs b/tests/integration/mobile_app/upload.rs index 7eb63e3d5a..613be3b42d 100644 --- a/tests/integration/mobile_app/upload.rs +++ b/tests/integration/mobile_app/upload.rs @@ -86,7 +86,7 @@ fn command_mobile_app_upload_apk_all_uploaded() { "POST", "/api/0/projects/wat-org/wat-project/files/preprodartifacts/assemble/", ) - .with_response_body(r#"{"state":"ok","missingChunks":[]}"#), + .with_response_body(r#"{"state":"ok","missingChunks":[],"artifactId":"42"}"#), ) .register_trycmd_test("mobile_app/mobile_app-upload-apk-all-uploaded.trycmd") .with_default_token(); @@ -159,7 +159,7 @@ fn command_mobile_app_upload_apk_chunked() { "/api/0/projects/wat-org/wat-project/files/preprodartifacts/assemble/", ) .with_header_matcher("content-type", "application/json") - .with_matcher(r#"{"checksum":"18e40e6e932d0b622d631e887be454cc2003dbb5","chunks":["18e40e6e932d0b622d631e887be454cc2003dbb5"],"git_sha":"test_sha"}"#) + .with_matcher(r#"{"checksum":"18e40e6e932d0b622d631e887be454cc2003dbb5","chunks":["18e40e6e932d0b622d631e887be454cc2003dbb5"],"head_sha":"test_sha"}"#) .with_response_fn(move |_| { if is_first_assemble_call.swap(false, Ordering::Relaxed) { r#"{ @@ -169,7 +169,8 @@ fn command_mobile_app_upload_apk_chunked() { } else { r#"{ "state": "ok", - "missingChunks": [] + "missingChunks": [], + "artifactId": "42" }"# } .into() @@ -213,7 +214,7 @@ fn command_mobile_app_upload_ipa_chunked() { "/api/0/projects/wat-org/wat-project/files/preprodartifacts/assemble/", ) .with_header_matcher("content-type", "application/json") - .with_matcher(r#"{"checksum":"ed9da71e3688261875db21b266da84ffe004a8a4","chunks":["ed9da71e3688261875db21b266da84ffe004a8a4"],"git_sha":"test_sha"}"#) + .with_matcher(r#"{"checksum":"ed9da71e3688261875db21b266da84ffe004a8a4","chunks":["ed9da71e3688261875db21b266da84ffe004a8a4"],"head_sha":"test_sha"}"#) .with_response_fn(move |_| { if is_first_assemble_call.swap(false, Ordering::Relaxed) { r#"{ @@ -223,7 +224,8 @@ fn command_mobile_app_upload_ipa_chunked() { } else { r#"{ "state": "ok", - "missingChunks": [] + "missingChunks": [], + "artifactId": "some-text-id" }"# } .into()