diff --git a/js/__tests__/helper.test.js b/js/__tests__/helper.test.js index ae2fd30461..fa3e4c52f3 100644 --- a/js/__tests__/helper.test.js +++ b/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']; diff --git a/js/helper.js b/js/helper.js index 5f154c85c2..ff23054c3e 100644 --- a/js/helper.js +++ b/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 { diff --git a/js/releases/__tests__/index.test.js b/js/releases/__tests__/index.test.js index 5c992d8852..1e7a74ee10 100644 --- a/js/releases/__tests__/index.test.js +++ b/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 } + ); + }); }); }); }); diff --git a/js/releases/index.js b/js/releases/index.js index 4a1b66231a..5a58bc7901 100644 --- a/js/releases/index.js +++ b/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.} A promise that resolves to the standard output. */ async execute(args, live) {