chore(js): Remove live = 'rejectOnError' option

Author: szokeasaurusrex

CHANGELOG.md

@@ -20,6 +20,8 @@ we should rename this section to "Unreleased" -->
   - `api_key` configuration file field
   - `apiKey` option in the JavaScript API
 - Removed the `upload-proguard` subcommand's `--app-id`, `--version`, `--version-code`, `--android-manifest`, and `--platform` arguments ([#2876](https://github.com/getsentry/sentry-cli/pull/2876), [#2940](https://github.com/getsentry/sentry-cli/pull/2940), [#2948](https://github.com/getsentry/sentry-cli/pull/2948)). Users using these arguments should stop using them, as they are unnecessary. The information passed to these arguments is no longer visible in Sentry.
+- In the JS API, the `SentryCli.execute` method's `live` parameter now only takes boolean values ([#XXXX](TODO)). Setting `live` to `true` now behaves like `'rejectOnError'` did previously, with a zero exit status resolving the returned promise with `"success (live mode)"` and a non-zero status rejecting the promise with an error message.
+- In the JS API, the `option` parameter to `Releases.uploadSourceMaps` no longer takes a `live` property ([#XXXX](TODO)). We now always execute the command with `live` set to `true`.
 
 ### Improvements
 

lib/__tests__/helper.test.js

@@ -28,17 +28,27 @@ describe('SentryCli helper', () => {
       expect(output.trim()).toBe('sentry-cli DEV');
     });
 
-    test('execute with live=true resolves without output', async () => {
+    test('execute with live=true resolves on success', async () => {
       // TODO (v3): This should resolve with a string, not undefined/void
       const result = await helper.execute(['--version'], true);
-      expect(result).toBeUndefined();
+      expect(result).toBe('success (live mode)');
+    });
+
+    test('execute with live=true rejects on failure', async () => {
+      await expect(helper.execute(['fail'], true)).rejects.toThrow(
+        'Command fail failed with exit code 1'
+      );
     });
 
+    // live=rejectOnError is not supported per the type declarations, but we should still aim
+    // to support it for backwards compatibility.
     test('execute with live=rejectOnError resolves on success', async () => {
       const result = await helper.execute(['--version'], 'rejectOnError');
       expect(result).toBe('success (live mode)');
     });
 
+    // live=rejectOnError is not supported per the type declarations, but we should still aim
+    // to support it for backwards compatibility.
     test('execute with live=rejectOnError rejects on failure', async () => {
       await expect(helper.execute(['fail'], 'rejectOnError')).rejects.toThrow(
         'Command fail failed with exit code 1'

lib/helper.js

@@ -281,11 +281,10 @@ function getPath() {
  * expect(output.trim()).toBe('sentry-cli x.y.z');
  *
  * @param {string[]} args Command line arguments passed to `sentry-cli`.
- * @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
+ * @param {boolean} live can be set to:
+ *  - `true` to inherit stdio and reject the promise if the command
  *    exits with a non-zero exit code.
+ *  - `false` to not inherit stdio and return the output as a string.
  * @param {boolean} silent Disable stdout for silents build (CI/Webpack Stats, ...)
  * @param {string} [configFile] Relative or absolute path to the configuration file.
  * @param {import('./index').SentryCliOptions} [config] More configuration to pass to the CLI
@@ -325,26 +324,18 @@ async function execute(args, live, silent, configFile, config = {}) {
   }
 
   return new Promise((resolve, reject) => {
-    if (live === true || live === 'rejectOnError') {
+    if (live) {
       const output = silent ? 'ignore' : 'inherit';
       const pid = childProcess.spawn(getPath(), args, {
         env,
         // stdin, stdout, stderr
         stdio: ['ignore', output, output],
       });
       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}`));
+        if (exitCode === 0) {
+          resolve('success (live mode)');
         }
-        // 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();
+        reject(new Error(`Command ${args.join(' ')} failed with exit code ${exitCode}`));
       });
     } else {
       childProcess.execFile(getPath(), args, { env }, (err, stdout) => {

lib/index.js

@@ -68,11 +68,10 @@ class SentryCli {
   /**
    * See {helper.execute} docs.
    * @param {string[]} args Command line arguments passed to `sentry-cli`.
-   * @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
+   * @param {boolean} live can be set to:
+   *  - `true` to inherit stdio and reject the promise if the command
    *    exits with a non-zero exit code.
+   *  - `false` to not inherit stdio and return the output as a string.
    * @returns {Promise<string>} A promise that resolves to the standard output.
    */
   execute(args, live) {

lib/releases/index.js

@@ -142,11 +142,10 @@ class Releases {
    *   ext: ['js', 'map', 'jsbundle', 'bundle'],  // override file extensions to scan for
    *   projects: ['node'],        // provide a list of projects
    *   decompress: false          // decompress gzip files before uploading
-   *   live: true                 // whether to inherit stdio to display `sentry-cli` output directly.
    * });
    *
    * @param {string} release Unique name of the release.
-   * @param {SentryCliUploadSourceMapsOptions & {live?: boolean | 'rejectOnError'}} options Options to configure the source map upload.
+   * @param {SentryCliUploadSourceMapsOptions} options Options to configure the source map upload.
    * @returns {Promise<string[]>} A promise that resolves when the upload has completed successfully.
    * @memberof SentryReleases
    */
@@ -195,7 +194,7 @@ class Releases {
         // `execute()` is async and thus we're returning a promise here
         this.execute(
           helper.prepareCommand([...args, path], SOURCEMAPS_SCHEMA, newOptions),
-          options.live != null ? options.live : true
+          true
         )
       );
     });
@@ -252,11 +251,10 @@ class Releases {
   /**
    * See {helper.execute} docs.
    * @param {string[]} args Command line arguments passed to `sentry-cli`.
-   * @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
+   * @param {boolean} live can be set to:
+   *  - `true` to inherit stdio and reject the promise if the command
    *    exits with a non-zero exit code.
+   *  - `false` to not inherit stdio and return the output as a string.
    * @returns {Promise<string>} A promise that resolves to the standard output.
    */
   async execute(args, live) {

lib/types.ts

@@ -224,12 +224,12 @@ export interface SentryCliReleases {
 
   uploadSourceMaps(
     release: string,
-    options: SentryCliUploadSourceMapsOptions & { live?: boolean | 'rejectOnError' }
+    options: SentryCliUploadSourceMapsOptions & { live?: boolean }
   ): Promise<string[]>;
 
   listDeploys(release: string): Promise<string>;
 
   newDeploy(release: string, options: SentryCliNewDeployOptions): Promise<string>;
 
-  execute(args: string[], live: boolean | 'rejectOnError'): Promise<string>;
+  execute(args: string[], live: boolean): Promise<string>;
 }