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

### Description
We added the `live = 'rejectOnError'` option in #2605 with the intention to make the behavior provided by setting `live = 'rejectOnError'` the default behavior of setting `live = true` in the next major.

### Issues
- Resolves #2606
- Resolves [CLI-138](https://linear.app/getsentry/issue/CLI-138/v3-fix-js-interface-execute-promise-resolve-values)

Author: szokeasaurusrex

CHANGELOG.md

@@ -25,6 +25,8 @@ we should rename this section to "Unreleased" -->
 
 The following changes only apply when using `sentry-cli` via the npm package [`@sentry/cli`](https://www.npmjs.com/package/@sentry/cli):
 
+- The `SentryCli.execute` method's `live` parameter now only takes boolean values ([#2971](https://github.com/getsentry/sentry-cli/pull/2971)). 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.
+- The `option` parameter to `Releases.uploadSourceMaps` no longer takes a `live` property ([#2971](https://github.com/getsentry/sentry-cli/pull/2971)). We now always execute the command with `live` set to `true`.
 - Removed the `apiKey` option from `SentryCliOptions` ([#2935](https://github.com/getsentry/sentry-cli/pull/2935)). If you are using `apiKey`, you need to generate and use an [Auth Token](https://docs.sentry.io/account/auth-tokens/) via the `authToken` option, instead.
 
 ### 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/__tests__/index.test.js

@@ -7,14 +7,14 @@ describe('SentryCli releases', () => {
   test('call sentry-cli releases propose-version', () => {
     expect.assertions(1);
     const cli = new SentryCli();
-    return cli.releases.proposeVersion().then(version => expect(version).toBeTruthy());
+    return cli.releases.proposeVersion().then((version) => expect(version).toBeTruthy());
   });
 
   describe('with mock', () => {
     let cli;
     let mockExecute;
     beforeAll(() => {
-      mockExecute = jest.fn(async () => { });
+      mockExecute = jest.fn(async () => {});
       jest.doMock('../../helper', () => ({
         ...jest.requireActual('../../helper'),
         execute: mockExecute,
@@ -52,15 +52,7 @@ describe('SentryCli releases', () => {
       test('without projects', async () => {
         await cli.releases.uploadSourceMaps('my-version', { include: ['path'] });
         expect(mockExecute).toHaveBeenCalledWith(
-          [
-            'sourcemaps',
-            'upload',
-            '--release',
-            'my-version',
-            'path',
-            '--ignore',
-            'node_modules',
-          ],
+          ['sourcemaps', 'upload', '--release', 'my-version', 'path', '--ignore', 'node_modules'],
           true,
           false,
           undefined,
@@ -100,17 +92,9 @@ describe('SentryCli releases', () => {
         await cli.releases.uploadSourceMaps('my-version', { include: paths });
 
         expect(mockExecute).toHaveBeenCalledTimes(2);
-        paths.forEach(path =>
+        paths.forEach((path) =>
           expect(mockExecute).toHaveBeenCalledWith(
-            [
-              'sourcemaps',
-              'upload',
-              '--release',
-              'my-version',
-              path,
-              '--ignore',
-              'node_modules',
-            ],
+            ['sourcemaps', 'upload', '--release', 'my-version', path, '--ignore', 'node_modules'],
             true,
             false,
             undefined,
@@ -159,25 +143,6 @@ 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(
-          [
-            'sourcemaps',
-            'upload',
-            '--release',
-            'my-version',
-            'path',
-            '--ignore',
-            'node_modules',
-          ],
-          live,
-          false,
-          undefined,
-          { silent: false }
-        );
-      });
     });
   });
 });

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
    */
@@ -193,10 +192,7 @@ 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),
-          options.live != null ? options.live : true
-        )
+        this.execute(helper.prepareCommand([...args, path], SOURCEMAPS_SCHEMA, newOptions), true)
       );
     });
 
@@ -252,11 +248,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>;
 }