Merge branch 'master' into cursor/update-mobile-app-command-for-ipa-uploads-e699

Author: szokeasaurusrex

.github/workflows/test_node.yml

@@ -20,9 +20,11 @@ jobs:
           node-version-file: package.json
 
       # We need to skip the fallback download because downloading will fail on release branches because the new version isn't available yet.
-      - run: SENTRYCLI_SKIP_DOWNLOAD=1 yarn install --frozen-lockfile
+      # We have to use npm here because yarn fails on the non-existing existing optionalDependency version:
+      # https://github.com/yarnpkg/berry/issues/2425#issuecomment-1627807326
+      - run: SENTRYCLI_SKIP_DOWNLOAD=1 npm install
 
-      - run: yarn check:types
+      - run: npm run check:types
 
   test_node:
     strategy:
@@ -42,13 +44,12 @@ jobs:
           node-version: ${{ matrix.node-version }}
 
       # We need to skip the fallback download because downloading will fail on release branches because the new version isn't available yet.
-      - run: SENTRYCLI_SKIP_DOWNLOAD=1 yarn install --frozen-lockfile
-        if: matrix.node-version != '10.x' && matrix.node-version != '12.x'
+      # We have to use npm here because yarn fails on the non-existing existing optionalDependency version:
+      # https://github.com/yarnpkg/berry/issues/2425#issuecomment-1627807326
+      - run: SENTRYCLI_SKIP_DOWNLOAD=1 npm install
 
       # older node versions need an older nft
-      - run: |
-          SENTRYCLI_SKIP_DOWNLOAD=1 yarn install --ignore-engines --frozen-lockfile
-          SENTRYCLI_SKIP_DOWNLOAD=1 yarn add @vercel/nft@0.22.1 --ignore-engines
+      - run: SENTRYCLI_SKIP_DOWNLOAD=1 npm install @vercel/nft@0.22.1
         if: matrix.node-version == '10.x' || matrix.node-version == '12.x'
 
-      - run: yarn test
+      - run: npm test

apple-catalog-parsing/build.rs

@@ -2,6 +2,9 @@ use std::env;
 use std::process::Command;
 
 fn main() {
+    /// Environment variable to disable Swift sandboxing.
+    const SWIFT_DISABLE_SANDBOX: &str = "SWIFT_DISABLE_SANDBOX";
+
     let target = env::var("TARGET").expect("TARGET is set for build scripts");
     let mut target_bits = target.split('-');
 
@@ -20,21 +23,34 @@ fn main() {
 
     println!("cargo:rerun-if-changed=native/swift/AssetCatalogParser");
 
+    // Allow swift to be run with `--disable-sandbox` in case cargo has been invoked inside a
+    // sandbox already. Nested sandboxes are not allowed on Darwin.
+    println!("cargo:rerun-if-env-changed={SWIFT_DISABLE_SANDBOX}");
+
     let out_dir = env::var("OUT_DIR").expect("OUT_DIR is set for build scripts");
 
     // Compile Swift code
     let status = Command::new("swift")
-        .args([
-            "build",
-            "-c",
-            "release",
-            "--package-path",
-            "native/swift/AssetCatalogParser",
-            "--scratch-path",
-            &format!("{out_dir}/swift-scratch"),
-            "--triple",
-            &format!("{arch}-apple-macosx10.12"),
-        ])
+        .args(
+            [
+                "build",
+                "-c",
+                "release",
+                "--package-path",
+                "native/swift/AssetCatalogParser",
+                "--scratch-path",
+                &format!("{out_dir}/swift-scratch"),
+                "--triple",
+                &format!("{arch}-apple-macosx10.12"),
+            ]
+            .into_iter()
+            .chain(
+                env::var(SWIFT_DISABLE_SANDBOX)
+                    .ok()
+                    .filter(|s| s == "1")
+                    .map(|_| "--disable-sandbox"),
+            ),
+        )
         .status()
         .expect("Failed to compile SPM");
 

js/__tests__/helper.test.js

@@ -22,6 +22,30 @@ describe('SentryCli helper', () => {
     expect(helper.getPath()).toMatch(pattern);
   });
 
+  describe('execute', () => {
+    test('execute with live=false returns stdout', async () => {
+      const output = await helper.execute(['--version'], false);
+      expect(output.trim()).toBe('sentry-cli DEV');
+    });
+
+    test('execute with live=true resolves without output', async () => {
+      // TODO (v3): This should resolve with a string, not undefined/void
+      const result = await helper.execute(['--version'], true);
+      expect(result).toBeUndefined();
+    });
+
+    test('execute with live=rejectOnError resolves on success', async () => {
+      const result = await helper.execute(['--version'], 'rejectOnError');
+      expect(result).toBe('success (live mode)');
+    });
+
+    test('execute with live=rejectOnError rejects on failure', async () => {
+      await expect(helper.execute(['fail'], 'rejectOnError')).rejects.toThrow(
+        'Command fail failed with exit code 1'
+      );
+    });
+  });
+
   describe('`prepare` command', () => {
     test('call prepare command add default ignore', () => {
       const command = ['releases', 'files', 'release', 'upload-sourcemaps', '/dev/null'];

js/helper.js

@@ -281,7 +281,11 @@ function getPath() {
  * expect(output.trim()).toBe('sentry-cli x.y.z');
  *
  * @param {string[]} args Command line arguments passed to `sentry-cli`.
- * @param {boolean} live We inherit stdio to display `sentry-cli` output directly.
+ * @param {boolean | 'rejectOnError'} live can be set to:
+ *  - `true` to inherit stdio to display `sentry-cli` output directly.
+ *  - `false` to not inherit stdio and return the output as a string.
+ *  - `'rejectOnError'` to inherit stdio and reject the promise if the command
+ *    exits with a non-zero exit code.
  * @param {boolean} silent Disable stdout for silents build (CI/Webpack Stats, ...)
  * @param {string} [configFile] Relative or absolute path to the configuration file.
  * @param {Object} [config] More configuration to pass to the CLI
@@ -322,16 +326,27 @@ async function execute(args, live, silent, configFile, config = {}) {
     ]);
     args = [...headers, ...args];
   }
+
   return new Promise((resolve, reject) => {
-    if (live === true) {
+    if (live === true || live === 'rejectOnError') {
       const output = silent ? 'ignore' : 'inherit';
       const pid = childProcess.spawn(getPath(), args, {
         env,
         // stdin, stdout, stderr
         stdio: ['ignore', output, output],
       });
-      pid.on('exit', () => {
-        // @ts-expect-error - this is a TODO (v3) to fix and resolve a string here
+      pid.on('exit', (exitCode) => {
+        if (live === 'rejectOnError') {
+          if (exitCode === 0) {
+            resolve('success (live mode)');
+          }
+          reject(new Error(`Command ${args.join(' ')} failed with exit code ${exitCode}`));
+        }
+        // According to the type definition, resolving with void is not allowed.
+        // However, for backwards compatibility, we resolve void here to
+        // avoid a behaviour-breaking change.
+        // TODO (v3): Clean this up and always resolve a string (or change the type definition)
+        // @ts-expect-error - see comment above
         resolve();
       });
     } else {

js/releases/__tests__/index.test.js

@@ -159,6 +159,25 @@ describe('SentryCli releases', () => {
           { silent: false }
         );
       });
+
+      test.each([true, false, 'rejectOnError'])('handles live mode %s', async (live) => {
+        await cli.releases.uploadSourceMaps('my-version', { include: ['path'], live });
+        expect(mockExecute).toHaveBeenCalledWith(
+          [
+            'releases',
+            'files',
+            'my-version',
+            'upload-sourcemaps',
+            'path',
+            '--ignore',
+            'node_modules',
+          ],
+          live,
+          false,
+          undefined,
+          { silent: false }
+        );
+      });
     });
   });
 });

js/releases/index.js

@@ -199,7 +199,10 @@ class Releases {
 
       return uploadPaths.map((path) =>
         // `execute()` is async and thus we're returning a promise here
-        this.execute(helper.prepareCommand([...args, path], SOURCEMAPS_SCHEMA, newOptions), true)
+        this.execute(
+          helper.prepareCommand([...args, path], SOURCEMAPS_SCHEMA, newOptions),
+          options.live != null ? options.live : true
+        )
       );
     });
 
@@ -255,7 +258,11 @@ class Releases {
   /**
    * See {helper.execute} docs.
    * @param {string[]} args Command line arguments passed to `sentry-cli`.
-   * @param {boolean} live We inherit stdio to display `sentry-cli` output directly.
+   * @param {boolean | 'rejectOnError'} live can be set to:
+   *  - `true` to inherit stdio to display `sentry-cli` output directly.
+   *  - `false` to not inherit stdio and return the output as a string.
+   *  - `'rejectOnError'` to inherit stdio and reject the promise if the command
+   *    exits with a non-zero exit code.
    * @returns {Promise.<string>} A promise that resolves to the standard output.
    */
   async execute(args, live) {

src/commands/mobile_app/upload.rs

@@ -1,10 +1,10 @@
 use std::borrow::Cow;
-use std::io::Write;
-use std::path::Path;
 #[cfg(not(windows))]
 use std::fs;
+use std::io::Write;
 #[cfg(not(windows))]
 use std::os::unix::fs::PermissionsExt;
+use std::path::Path;
 
 use anyhow::{anyhow, bail, Context as _, Result};
 use clap::{Arg, ArgAction, ArgMatches, Command};
@@ -343,9 +343,14 @@ fn normalize_directory(path: &Path) -> Result<TempFile> {
         .filter(|entry| entry.path().is_file())
         .map(|entry| {
             let entry_path = entry.into_path();
-            let relative_path = entry_path.strip_prefix(
-                path.parent().ok_or_else(|| anyhow!("Cannot determine parent directory for path: {}", path.display()))?
-            )?.to_owned();
+            let relative_path = entry_path
+                .strip_prefix(path.parent().ok_or_else(|| {
+                    anyhow!(
+                        "Cannot determine parent directory for path: {}",
+                        path.display()
+                    )
+                })?)?
+                .to_owned();
             Ok((entry_path, relative_path))
         })
         .collect::<Result<Vec<_>>>()?

src/commands/mod.rs

@@ -19,7 +19,33 @@ use crate::utils::system::{load_dotenv, print_error, set_panic_hook, QuietExit};
 use crate::utils::update::run_sentrycli_update_nagger;
 use crate::utils::value_parsers::auth_token_parser;
 
+mod bash_hook;
+mod debug_files;
+mod deploys;
 mod derive_parser;
+mod events;
+mod files;
+mod info;
+mod issues;
+mod login;
+mod mobile_app;
+mod monitors;
+mod organizations;
+mod projects;
+mod react_native;
+mod releases;
+mod repos;
+mod send_envelope;
+mod send_event;
+mod send_metric;
+mod sourcemaps;
+#[cfg(not(feature = "managed"))]
+mod uninstall;
+#[cfg(not(feature = "managed"))]
+mod update;
+mod upload_dif;
+mod upload_dsym;
+mod upload_proguard;
 
 macro_rules! each_subcommand {
     ($mac:ident) => {
@@ -53,14 +79,6 @@ macro_rules! each_subcommand {
     };
 }
 
-macro_rules! import_subcommand {
-    ($name:ident) => {
-        pub mod $name;
-    };
-}
-
-each_subcommand!(import_subcommand);
-
 const ABOUT: &str = "
 Command line utility for Sentry.
 

src/commands/react_native/gradle.rs

@@ -119,9 +119,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
 
     if let Some(version) = version {
         for dist in matches.get_many::<String>("dist").unwrap() {
-            println!(
-                "Uploading sourcemaps for release {version} distribution {dist}"
-            );
+            println!("Uploading sourcemaps for release {version} distribution {dist}");
 
             processor.upload(&UploadContext {
                 org: &org,

src/commands/send_metric/mod.rs

@@ -38,7 +38,7 @@ This command will validate input parameters and attempt to send a metric to \
 Sentry. Due to network errors and rate limits, the metric is not guaranteed to \
 arrive. Check the debug output for transmission errors by passing --log-level=\
 debug or setting `SENTRY_LOG_LEVEL=debug`."))]
-#[command(hide=true)]
+#[command(hide = true)]
 enum SendMetricSubcommand {
     #[command(about = format!("[DEPRECATED] {INCREMENT_ABOUT}"))]
     #[command(long_about = format!("{DEPRECATION_MESSAGE}{{n}}{{n}}{INCREMENT_ABOUT}"))]