From bd678cfb4cad03954f34366fdb3c2083e3c55374 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Wed, 19 Nov 2025 14:13:17 +0100 Subject: [PATCH] chore(js): Remove `live = 'rejectOnError'` option (#2971) ### Description We added the `live = 'rejectOnError'` option in https://github.com/getsentry/sentry-cli/pull/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) --- CHANGELOG.md | 2 ++ lib/__tests__/helper.test.js | 14 +++++++-- lib/helper.js | 23 +++++--------- lib/index.js | 7 ++--- lib/releases/__tests__/index.test.js | 45 ++++------------------------ lib/releases/index.js | 15 ++++------ lib/types.ts | 4 +-- 7 files changed, 36 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a54828372e..b43ae84369 100644 --- a/CHANGELOG.md +++ b/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 ([#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. +- In the JS API, 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`. ### Improvements diff --git a/lib/__tests__/helper.test.js b/lib/__tests__/helper.test.js index 349c6f4b1c..b5988ca66b 100644 --- a/lib/__tests__/helper.test.js +++ b/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' diff --git a/lib/helper.js b/lib/helper.js index f2784b9989..d0387ebd43 100644 --- a/lib/helper.js +++ b/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,7 +324,7 @@ 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, @@ -333,18 +332,10 @@ async function execute(args, live, silent, configFile, config = {}) { 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) => { diff --git a/lib/index.js b/lib/index.js index 288ad49e59..be7bcefaa2 100644 --- a/lib/index.js +++ b/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} A promise that resolves to the standard output. */ execute(args, live) { diff --git a/lib/releases/__tests__/index.test.js b/lib/releases/__tests__/index.test.js index fae20dd482..25ed4ccec5 100644 --- a/lib/releases/__tests__/index.test.js +++ b/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 } - ); - }); }); }); }); diff --git a/lib/releases/index.js b/lib/releases/index.js index f03ea05130..8b01f2d769 100644 --- a/lib/releases/index.js +++ b/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} 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} A promise that resolves to the standard output. */ async execute(args, live) { diff --git a/lib/types.ts b/lib/types.ts index 26d92214fb..d966e65b67 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -224,12 +224,12 @@ export interface SentryCliReleases { uploadSourceMaps( release: string, - options: SentryCliUploadSourceMapsOptions & { live?: boolean | 'rejectOnError' } + options: SentryCliUploadSourceMapsOptions & { live?: boolean } ): Promise; listDeploys(release: string): Promise; newDeploy(release: string, options: SentryCliNewDeployOptions): Promise; - execute(args: string[], live: boolean | 'rejectOnError'): Promise; + execute(args: string[], live: boolean): Promise; }