Skip to content

Commit

Permalink
fix(xcode): Only parse Plist when required during RN source maps uplo…
Browse files Browse the repository at this point in the history
…ad (#1940)
  • Loading branch information
krystofwoldrich authored Feb 28, 2024
1 parent 26da328 commit 5657d86
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 16 deletions.
39 changes: 23 additions & 16 deletions src/commands/react_native/xcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -371,21 +365,34 @@ 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 (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");
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))
},
_ => {
bail!("Info.plist was not found or an parsing error occurred");
}
}
}
// 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::<String>("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,
Expand All @@ -398,7 +405,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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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: 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.

```
Original file line number Diff line number Diff line change
@@ -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)

```

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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"
2 changes: 2 additions & 0 deletions tests/integration/react_native/mod.rs
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down
30 changes: 30 additions & 0 deletions tests/integration/react_native/xcode.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#[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")]
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);
}

#[cfg(target_os = "macos")]
pub fn assert_endpoints(mocks: &[Mock]) {
for mock in mocks {
mock.assert();
}
}

0 comments on commit 5657d86

Please sign in to comment.