From 7f213f335106f4c52051474c6efc07d0ec9d265d Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Feb 2024 15:33:14 +0100 Subject: [PATCH 1/6] fix(xcode): Only parse Plist when required during RN source maps upload --- src/commands/react_native/xcode.rs | 48 ++++++++++++------- ...de-upload-source-maps-invalid-plist.trycmd | 15 ++++++ ...urce-maps-release_and_dist_from_env.trycmd | 26 ++++++++++ .../react_native/react-native-xcode-bundle.js | 3 ++ .../react-native-xcode-bundle.js.map | 1 + .../react_native/react-native-xcode.sh | 8 ++++ tests/integration/react_native/mod.rs | 2 + tests/integration/react_native/xcode.rs | 29 +++++++++++ 8 files changed, 116 insertions(+), 16 deletions(-) create mode 100644 tests/integration/_cases/react_native/xcode-upload-source-maps-invalid-plist.trycmd create mode 100644 tests/integration/_cases/react_native/xcode-upload-source-maps-release_and_dist_from_env.trycmd create mode 100644 tests/integration/_fixtures/react_native/react-native-xcode-bundle.js create mode 100644 tests/integration/_fixtures/react_native/react-native-xcode-bundle.js.map create mode 100755 tests/integration/_fixtures/react_native/react-native-xcode.sh create mode 100644 tests/integration/react_native/xcode.rs diff --git a/src/commands/react_native/xcode.rs b/src/commands/react_native/xcode.rs index 4617834503..2d1294fea8 100644 --- a/src/commands/react_native/xcode.rs +++ b/src/commands/react_native/xcode.rs @@ -201,12 +201,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { return Ok(()); } - info!("Parsing Info.plist"); - let plist = match InfoPlist::discover_from_env()? { - Some(plist) => plist, - None => bail!("Could not find info.plist"), - }; - info!("Parse result from Info.plist: {:?}", &plist); let report_file = TempFile::create()?; let node = find_node(); info!("Using node interpreter '{}'", &node); @@ -371,21 +365,43 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { chunk_upload_options: chunk_upload_options.as_ref(), })?; } else { - let dist = dist_from_env.unwrap_or_else(|_| plist.build().to_string()); - let release_name = release_from_env.unwrap_or(format!( - "{}@{}+{}", - plist.bundle_id(), - plist.version(), - dist - )); + let mut dist: Option = None; + let mut release_name: Option = None; + + if dist_from_env.is_err() && release_from_env.is_err() { + info!("Parsing Info.plist"); + let plist = match InfoPlist::discover_from_env()? { + Some(plist) => plist, + None => bail!("Could not find info.plist"), + }; + info!("Parse result from Info.plist: {:?}", &plist); + + let dist_string = plist.build().to_string(); + dist = Some(dist_string.clone()); + release_name = Some(format!( + "{}@{}+{}", + plist.bundle_id(), + plist.version(), + dist_string + )); + } + + if let Ok(_dist) = dist_from_env { + info!("Using dist from `SENTRY_DIST` env"); + dist = Some(_dist); + } + if let Ok(_release) = release_from_env { + info!("Using release from `SENTRY_RELEASE` env"); + release_name = Some(_release); + } match matches.get_many::("dist") { None => { processor.upload(&UploadContext { org: &org, project: Some(&project), - release: Some(&release_name), - dist: Some(&dist), + release: release_name.as_deref(), + dist: dist.as_deref(), note: None, wait, max_wait, @@ -398,7 +414,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { processor.upload(&UploadContext { org: &org, project: Some(&project), - release: Some(&release_name), + release: release_name.as_deref(), dist: Some(dist), note: None, wait, diff --git a/tests/integration/_cases/react_native/xcode-upload-source-maps-invalid-plist.trycmd b/tests/integration/_cases/react_native/xcode-upload-source-maps-invalid-plist.trycmd new file mode 100644 index 0000000000..68be8e9a7e --- /dev/null +++ b/tests/integration/_cases/react_native/xcode-upload-source-maps-invalid-plist.trycmd @@ -0,0 +1,15 @@ +``` +$ CONFIGURATION=Release sentry-cli react-native xcode tests/integration/_fixtures/react_native/react-native-xcode.sh --force-foreground +? failed +react-native-xcode.sh called with args: +Using React Native Packager bundle and source map. +Processing react-native sourcemaps for Sentry upload. +> Analyzing 2 sources +> Rewriting sources +> Adding source map references +error: Could not find info.plist + +Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output. +Please attach the full debug log to all bug reports. + +``` diff --git a/tests/integration/_cases/react_native/xcode-upload-source-maps-release_and_dist_from_env.trycmd b/tests/integration/_cases/react_native/xcode-upload-source-maps-release_and_dist_from_env.trycmd new file mode 100644 index 0000000000..951d393d50 --- /dev/null +++ b/tests/integration/_cases/react_native/xcode-upload-source-maps-release_and_dist_from_env.trycmd @@ -0,0 +1,26 @@ +``` +$ CONFIGURATION=Release SENTRY_RELEASE=test-release SENTRY_DIST=test-dist sentry-cli react-native xcode tests/integration/_fixtures/react_native/react-native-xcode.sh --force-foreground +? success +react-native-xcode.sh called with args: +Using React Native Packager bundle and source map. +Processing react-native sourcemaps for Sentry upload. +> Analyzing 2 sources +> Rewriting sources +> Adding source map references +> Bundled 2 files for upload +> Bundle ID: [..]-[..]-[..]-[..]-[..] +> Uploaded files to Sentry +> File upload complete (processing pending on server) +> Organization: wat-org +> Project: wat-project +> Release: test-release +> Dist: test-dist +> Upload type: artifact bundle + +Source Map Upload Report + Scripts + ~/react-native-xcode-bundle.js.map + Minified Scripts + ~/react-native-xcode-bundle.js (sourcemap at react-native-xcode-bundle.map) + +``` diff --git a/tests/integration/_fixtures/react_native/react-native-xcode-bundle.js b/tests/integration/_fixtures/react_native/react-native-xcode-bundle.js new file mode 100644 index 0000000000..a9da8b02ea --- /dev/null +++ b/tests/integration/_fixtures/react_native/react-native-xcode-bundle.js @@ -0,0 +1,3 @@ +console.log("Fake bundle!"); + +//# sourceMappingURL=react-native-xcode-bundle.map diff --git a/tests/integration/_fixtures/react_native/react-native-xcode-bundle.js.map b/tests/integration/_fixtures/react_native/react-native-xcode-bundle.js.map new file mode 100644 index 0000000000..09e76ff015 --- /dev/null +++ b/tests/integration/_fixtures/react_native/react-native-xcode-bundle.js.map @@ -0,0 +1 @@ +{"version":3,"file":"react-native-xcode-bundle.js"} diff --git a/tests/integration/_fixtures/react_native/react-native-xcode.sh b/tests/integration/_fixtures/react_native/react-native-xcode.sh new file mode 100755 index 0000000000..432d67cfe6 --- /dev/null +++ b/tests/integration/_fixtures/react_native/react-native-xcode.sh @@ -0,0 +1,8 @@ +#!/bin/sh + +echo "react-native-xcode.sh called with args: $@" + +echo '{ + "packager_bundle_path": "tests/integration/_fixtures/react_native/react-native-xcode-bundle.js", + "packager_sourcemap_path": "tests/integration/_fixtures/react_native/react-native-xcode-bundle.js.map" +}' > "$SENTRY_RN_SOURCEMAP_REPORT" diff --git a/tests/integration/react_native/mod.rs b/tests/integration/react_native/mod.rs index c99208524d..668650f5b9 100644 --- a/tests/integration/react_native/mod.rs +++ b/tests/integration/react_native/mod.rs @@ -1,6 +1,8 @@ #[cfg(target_os = "macos")] use crate::integration::register_test; +mod xcode; + #[test] #[cfg(target_os = "macos")] fn xcode_wrap_call_minimum() { diff --git a/tests/integration/react_native/xcode.rs b/tests/integration/react_native/xcode.rs new file mode 100644 index 0000000000..f017519c1f --- /dev/null +++ b/tests/integration/react_native/xcode.rs @@ -0,0 +1,29 @@ +use mockito::Mock; + +#[cfg(target_os = "macos")] +use crate::integration::register_test; +#[cfg(target_os = "macos")] +use crate::integration::{mock_common_upload_endpoints, ChunkOptions, ServerBehavior}; + +#[test] +#[cfg(target_os = "macos")] +fn xcode_upload_source_maps_missing_plist() { + let _upload_endpoints = + mock_common_upload_endpoints(ServerBehavior::Modern, ChunkOptions::default()); + register_test("react_native/xcode-upload-source-maps-invalid-plist.trycmd"); +} + +#[test] +#[cfg(target_os = "macos")] +fn xcode_upload_source_maps_release_and_dist_from_env() { + let upload_endpoints = + mock_common_upload_endpoints(ServerBehavior::Modern, ChunkOptions::default()); + register_test("react_native/xcode-upload-source-maps-release_and_dist_from_env.trycmd"); + assert_endpoints(&upload_endpoints); +} + +pub fn assert_endpoints(mocks: &[Mock]) { + for mock in mocks { + mock.assert(); + } +} From fe7431a119cc58af854684854a83dd88f41f978c Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Feb 2024 15:39:54 +0100 Subject: [PATCH 2/6] fix clippy on linux --- tests/integration/react_native/xcode.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/react_native/xcode.rs b/tests/integration/react_native/xcode.rs index f017519c1f..b3fe6cf6e0 100644 --- a/tests/integration/react_native/xcode.rs +++ b/tests/integration/react_native/xcode.rs @@ -1,5 +1,5 @@ +#[cfg(target_os = "macos")] use mockito::Mock; - #[cfg(target_os = "macos")] use crate::integration::register_test; #[cfg(target_os = "macos")] @@ -22,6 +22,7 @@ fn xcode_upload_source_maps_release_and_dist_from_env() { assert_endpoints(&upload_endpoints); } +#[cfg(target_os = "macos")] pub fn assert_endpoints(mocks: &[Mock]) { for mock in mocks { mock.assert(); From af83d618597023d80e960a408080cd58d6c63254 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Feb 2024 17:36:23 +0100 Subject: [PATCH 3/6] simplify dist release login use match --- src/commands/react_native/xcode.rs | 58 ++++++++++++------------- tests/integration/react_native/xcode.rs | 4 +- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/commands/react_native/xcode.rs b/src/commands/react_native/xcode.rs index 2d1294fea8..c57232614a 100644 --- a/src/commands/react_native/xcode.rs +++ b/src/commands/react_native/xcode.rs @@ -365,35 +365,35 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { chunk_upload_options: chunk_upload_options.as_ref(), })?; } else { - let mut dist: Option = None; - let mut release_name: Option = None; - - if dist_from_env.is_err() && release_from_env.is_err() { - info!("Parsing Info.plist"); - let plist = match InfoPlist::discover_from_env()? { - Some(plist) => plist, - None => bail!("Could not find info.plist"), - }; - info!("Parse result from Info.plist: {:?}", &plist); - - let dist_string = plist.build().to_string(); - dist = Some(dist_string.clone()); - release_name = Some(format!( - "{}@{}+{}", - plist.bundle_id(), - plist.version(), - dist_string - )); - } - - if let Ok(_dist) = dist_from_env { - info!("Using dist from `SENTRY_DIST` env"); - dist = Some(_dist); - } - if let Ok(_release) = release_from_env { - info!("Using release from `SENTRY_RELEASE` env"); - release_name = Some(_release); - } + let (dist, release_name) = match (dist_from_env, release_from_env) { + (Ok(dist_env), Ok(release_env)) => { + // Both environment variables are present + (Some(dist_env), Some(release_env)) + }, + (Ok(dist_env), Err(_)) => { + // Only dist environment variable is present + (Some(dist_env), None) + }, + (Err(_), Ok(release_env)) => { + // Only release environment variable is present + (None, Some(release_env)) + }, + (Err(_), Err(_)) => { + // Neither environment variable is present, attempt to parse Info.plist + match InfoPlist::discover_from_env() { + Ok(Some(plist)) => { + // Successfully discovered and parsed Info.plist + let dist_string = plist.build().to_string(); + let release_string = format!("{}@{}+{}", plist.bundle_id(), plist.version(), dist_string); + (Some(dist_string), Some(release_string)) + }, + _ => { + // Info.plist was not found or an error occurred + (None, None) // Handle the error or absence as needed + } + } + } + }; match matches.get_many::("dist") { None => { diff --git a/tests/integration/react_native/xcode.rs b/tests/integration/react_native/xcode.rs index b3fe6cf6e0..d1d5598d4d 100644 --- a/tests/integration/react_native/xcode.rs +++ b/tests/integration/react_native/xcode.rs @@ -1,9 +1,9 @@ #[cfg(target_os = "macos")] -use mockito::Mock; -#[cfg(target_os = "macos")] use crate::integration::register_test; #[cfg(target_os = "macos")] use crate::integration::{mock_common_upload_endpoints, ChunkOptions, ServerBehavior}; +#[cfg(target_os = "macos")] +use mockito::Mock; #[test] #[cfg(target_os = "macos")] From 2593590275402d1b0aacb6be8ffc3bf20988ad3a Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Feb 2024 17:41:16 +0100 Subject: [PATCH 4/6] return debug logs --- src/commands/react_native/xcode.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/commands/react_native/xcode.rs b/src/commands/react_native/xcode.rs index c57232614a..a43fcc1be4 100644 --- a/src/commands/react_native/xcode.rs +++ b/src/commands/react_native/xcode.rs @@ -380,16 +380,17 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { }, (Err(_), Err(_)) => { // Neither environment variable is present, attempt to parse Info.plist + info!("Parsing Info.plist"); match InfoPlist::discover_from_env() { Ok(Some(plist)) => { // Successfully discovered and parsed Info.plist let dist_string = plist.build().to_string(); let release_string = format!("{}@{}+{}", plist.bundle_id(), plist.version(), dist_string); + info!("Parse result from Info.plist: {:?}", &plist); (Some(dist_string), Some(release_string)) }, _ => { - // Info.plist was not found or an error occurred - (None, None) // Handle the error or absence as needed + bail!("Info.plist was not found or an parsing error occurred"); } } } From 6377a5d1767e0d89b6c40b31d1f071cc8325ddcd Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Feb 2024 17:43:17 +0100 Subject: [PATCH 5/6] fix test --- .../react_native/xcode-upload-source-maps-invalid-plist.trycmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/_cases/react_native/xcode-upload-source-maps-invalid-plist.trycmd b/tests/integration/_cases/react_native/xcode-upload-source-maps-invalid-plist.trycmd index 68be8e9a7e..faf097c98a 100644 --- a/tests/integration/_cases/react_native/xcode-upload-source-maps-invalid-plist.trycmd +++ b/tests/integration/_cases/react_native/xcode-upload-source-maps-invalid-plist.trycmd @@ -7,7 +7,7 @@ Processing react-native sourcemaps for Sentry upload. > Analyzing 2 sources > Rewriting sources > Adding source map references -error: Could not find info.plist +error: Info.plist was not found or an parsing error occurred Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output. Please attach the full debug log to all bug reports. From a2a4d543f4d191db553f7b1b3385c5cd0c9cfefd Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Feb 2024 18:03:44 +0100 Subject: [PATCH 6/6] simple match --- src/commands/react_native/xcode.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/commands/react_native/xcode.rs b/src/commands/react_native/xcode.rs index a43fcc1be4..534d78d949 100644 --- a/src/commands/react_native/xcode.rs +++ b/src/commands/react_native/xcode.rs @@ -365,19 +365,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { chunk_upload_options: chunk_upload_options.as_ref(), })?; } else { - let (dist, release_name) = match (dist_from_env, release_from_env) { - (Ok(dist_env), Ok(release_env)) => { - // Both environment variables are present - (Some(dist_env), Some(release_env)) - }, - (Ok(dist_env), Err(_)) => { - // Only dist environment variable is present - (Some(dist_env), None) - }, - (Err(_), Ok(release_env)) => { - // Only release environment variable is present - (None, Some(release_env)) - }, + let (dist, release_name) = match (&dist_from_env, &release_from_env) { (Err(_), Err(_)) => { // Neither environment variable is present, attempt to parse Info.plist info!("Parsing Info.plist"); @@ -394,6 +382,8 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { } } } + // At least one environment variable is present, use the values from the environment + _ => (dist_from_env.ok(), release_from_env.ok()), }; match matches.get_many::("dist") {