Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/api/data_types/chunking/mobile_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ pub struct AssembleMobileAppResponse {
pub state: ChunkedFileState,
pub missing_chunks: Vec<Digest>,
pub detail: Option<String>,
pub artifact_id: Option<String>,
pub artifact_url: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah wait, this produces a behavior change. We now are trying to deserialize a field named artifact_url instead of artifact_id

}
34 changes: 16 additions & 18 deletions src/commands/mobile_app/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,8 @@ 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_and_ids = vec![];
let mut uploaded_paths_and_urls = vec![];
let mut errored_paths_and_reasons = vec![];
for (path, zip) in normalized_zips {
info!("Uploading file: {}", path.display());
Expand All @@ -132,9 +131,9 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
sha.as_deref(),
build_configuration,
) {
Ok(artifact_id) => {
Ok(artifact_url) => {
info!("Successfully uploaded file: {}", path.display());
uploaded_paths_and_ids.push((path.to_path_buf(), artifact_id));
uploaded_paths_and_urls.push((path.to_path_buf(), artifact_url));
}
Err(e) => {
debug!("Failed to upload file at path {}: {}", path.display(), e);
Expand All @@ -158,22 +157,21 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
}
}

if uploaded_paths_and_ids.is_empty() {
if uploaded_paths_and_urls.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 {
uploaded_paths_and_urls.len(),
if uploaded_paths_and_urls.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());
if uploaded_paths_and_urls.len() < 3 {
for (path, artifact_url) in &uploaded_paths_and_urls {
println!(" - {} {artifact_url}", path.display());
}
}
}
Expand Down Expand Up @@ -283,7 +281,7 @@ fn handle_directory(path: &Path) -> Result<TempFile> {
normalize_directory(path, temp_dir.path())
}

/// Returns artifact id if upload was successful.
/// Returns artifact url if upload was successful.
fn upload_file(
api: &AuthenticatedApi,
bytes: &[u8],
Expand Down Expand Up @@ -336,15 +334,16 @@ fn upload_file(
// 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)
// 2. artifact_url 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
// 1. state=created, artifact_url 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
// n. state=error, artifact_url unset

let result = loop {
let response =
api.assemble_mobile_app(org, project, checksum, &checksums, sha, build_configuration)?;
Expand All @@ -357,16 +356,15 @@ fn upload_file(
);
upload_chunks(&chunks, &chunk_upload_options, upload_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);
}

if let Some(artifact_id) = response.artifact_id {
break Ok(artifact_id);
if let Some(artifact_url) = response.artifact_url {
break Ok(artifact_url);
}

if response.state.is_finished() {
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/mobile_app/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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":[],"artifactId":"42"}"#),
.with_response_body(r#"{"state":"ok","missingChunks":[],"artifactUrl":"https://sentry.io/wat-org/preprod/wat-project/42"}"#),
)
.register_trycmd_test("mobile_app/mobile_app-upload-apk-all-uploaded.trycmd")
.with_default_token();
Expand Down Expand Up @@ -170,7 +170,7 @@ fn command_mobile_app_upload_apk_chunked() {
r#"{
"state": "ok",
"missingChunks": [],
"artifactId": "42"
"artifactUrl": "http://sentry.io/wat-org/preprod/wat-project/42"
}"#
}
.into()
Expand Down Expand Up @@ -225,7 +225,7 @@ fn command_mobile_app_upload_ipa_chunked() {
r#"{
"state": "ok",
"missingChunks": [],
"artifactId": "some-text-id"
"artifactUrl": "http://sentry.io/wat-org/preprod/wat-project/some-text-id"
}"#
}
.into()
Expand Down
Loading