diff --git a/js/__tests__/helper.test.js b/js/__tests__/helper.test.js index 349c6f4b1c..fad7d9957d 100644 --- a/js/__tests__/helper.test.js +++ b/js/__tests__/helper.test.js @@ -195,4 +195,132 @@ describe('SentryCli helper', () => { ]); }); }); + + describe('determineSuccessMessage type coverage', () => { + test('determineSuccessMessage handles supported high-impact commands', () => { + const testCases = [ + { args: ['releases', 'new', 'v1.0.0'], expected: '✓ Release v1.0.0 created' }, + { args: ['releases', 'finalize', 'v1.0.0'], expected: '✓ Release v1.0.0 finalized' }, + { + args: ['releases', 'files', 'v1.0.0', 'upload-sourcemaps'], + expected: '✓ Source maps uploaded', + }, + { args: ['sourcemaps', 'upload'], expected: '✓ Source maps uploaded' }, + { args: ['sourcemaps', 'inject'], expected: '✓ Source maps injected' }, + { args: ['debug-files', 'upload'], expected: '✓ Debug files uploaded' }, + { args: ['upload-proguard'], expected: '✓ ProGuard mappings uploaded' }, + { args: ['upload-dif'], expected: '✓ Debug information files uploaded' }, + { args: ['upload-dsym'], expected: '✓ Debug information files uploaded' }, + { args: ['deploys', 'new'], expected: '✓ Deploy created' }, + { args: ['send-event'], expected: '✓ Event sent' }, + { args: ['send-envelope'], expected: '✓ Envelope sent' }, + ]; + + testCases.forEach(({ args, expected }) => { + const message = helper.determineSuccessMessage(args); + expect(message).toBe(expected); + }); + }); + + test('determineSuccessMessage returns null for info/list operations and utility commands', () => { + const testCases = [ + ['--help'], + ['--version'], + ['info'], + ['login'], + ['organizations', 'list'], + ['projects', 'list'], + ['issues', 'list'], + ['events', 'list'], + ['files', 'list'], + ['deploys', 'list'], + ['monitors', 'list'], + ['releases', 'list'], + ['releases', 'delete', 'v1.0.0'], + ['unknown-command'], + ]; + + testCases.forEach((args) => { + const message = helper.determineSuccessMessage(args); + expect(message).toBe(null); + }); + }); + }); + + describe('Promise resolution scenarios', () => { + let consoleInfoSpy; + + beforeEach(() => { + consoleInfoSpy = jest.spyOn(console, 'info').mockImplementation(); + }); + + afterEach(() => { + consoleInfoSpy.mockRestore(); + }); + + test('execute with live=true, silentLogs=true uses stdio piping and resolves with empty string', async () => { + const result = await helper.execute(['--version'], true, false, true); + + expect(result).toBe(''); + expect(consoleInfoSpy).not.toHaveBeenCalled(); // --version doesn't have success message + }); + + test('execute with live=true, silentLogs=true shows success message for supported commands', async () => { + const result = await helper.execute(['sourcemaps', 'upload'], true, false, true); + + expect(result).toBe(''); + expect(consoleInfoSpy).toHaveBeenCalledWith('✓ Source maps uploaded'); + }); + + test('execute with live=false, silentLogs=true uses callback mode and resolves with empty string', async () => { + const result = await helper.execute(['--version'], false, false, true); + + expect(result).toBe(''); + expect(consoleInfoSpy).not.toHaveBeenCalled(); + }); + + test('execute with silent=true takes precedence over silentLogs=true', async () => { + const result = await helper.execute(['--version'], false, true, true); + + expect(result).toBe(''); + expect(consoleInfoSpy).not.toHaveBeenCalled(); + }); + + test('execute with live=false, silentLogs=true shows success message and returns empty string', async () => { + const result = await helper.execute(['sourcemaps', 'upload'], false, false, true); + + expect(result).toBe(''); + expect(consoleInfoSpy).toHaveBeenCalledWith('✓ Source maps uploaded'); + }); + + test('execute with normal mode (live=false, silent=false, silentLogs=false) returns actual stdout', async () => { + const result = await helper.execute(['--version'], false, false, false); + + expect(result.trim()).toBe('sentry-cli DEV'); + expect(consoleInfoSpy).not.toHaveBeenCalled(); + }); + + test('execute with silent=true suppresses output and messages', async () => { + // Test with live=true - uses stdio piping, resolves with undefined + const result1 = await helper.execute(['--version'], true, true, false); + expect(result1).toBeUndefined(); + + // Test with live=false - uses callback mode, resolves with empty string + const result2 = await helper.execute(['--version'], false, true, false); + expect(result2).toBe(''); + + // Test with silentLogs=true (should be ignored when silent=true) + const result3 = await helper.execute(['sourcemaps', 'upload'], false, true, true); + expect(result3).toBe(''); + + expect(consoleInfoSpy).not.toHaveBeenCalled(); + }); + + test('execute with live=true and normal mode uses stdio inherit and resolves with undefined', async () => { + const result = await helper.execute(['--version'], true, false, false); + + expect(result).toBeUndefined(); + expect(consoleInfoSpy).not.toHaveBeenCalled(); + }); + }); }); diff --git a/js/helper.js b/js/helper.js index ff23054c3e..4fc5d73129 100644 --- a/js/helper.js +++ b/js/helper.js @@ -287,11 +287,17 @@ function getPath() { * - `'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 {boolean} silentLogs Show only errors and success messages, hide verbose logs * @param {string} [configFile] Relative or absolute path to the configuration file. * @param {Object} [config] More configuration to pass to the CLI * @returns {Promise.} A promise that resolves to the standard output. */ -async function execute(args, live, silent, configFile, config = {}) { +async function execute(args, live, silent, silentLogs, configFile, config = {}) { + // Ensure silent takes precedence over silentLogs + if (silent) { + silentLogs = false; + } + const env = { ...process.env }; if (configFile) { env.SENTRY_PROPERTIES = configFile; @@ -329,19 +335,47 @@ async function execute(args, live, silent, configFile, config = {}) { return new Promise((resolve, reject) => { if (live === true || live === 'rejectOnError') { - const output = silent ? 'ignore' : 'inherit'; + // Configure stdio options for child process based on settings + // TypeScript needs explicit typing for stdio options + /** @type {('ignore' | 'pipe' | 'inherit')[]} */ + let stdio; + if (silent) { + // Complete silence + stdio = ['ignore', 'ignore', 'ignore']; + } else if (silentLogs && live !== 'rejectOnError') { + // Capture stdout to filter it, but show stderr for errors + // Not applicable for rejectOnError mode which needs to capture output + stdio = ['ignore', 'pipe', 'inherit']; + } else { + // Show everything (default behavior) + stdio = ['ignore', 'inherit', 'inherit']; + } + const pid = childProcess.spawn(getPath(), args, { env, - // stdin, stdout, stderr - stdio: ['ignore', output, output], + stdio, }); + pid.on('exit', (exitCode) => { if (live === 'rejectOnError') { if (exitCode === 0) { resolve('success (live mode)'); + } else { + reject(new Error(`Command ${args.join(' ')} failed with exit code ${exitCode}`)); } - reject(new Error(`Command ${args.join(' ')} failed with exit code ${exitCode}`)); + return; } + + if (silentLogs && exitCode === 0) { + const successMessage = determineSuccessMessage(args); + if (successMessage) { + console.info(successMessage); + } + resolve(''); + return; + } + + // Default case // 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. @@ -354,7 +388,15 @@ async function execute(args, live, silent, configFile, config = {}) { if (err) { reject(err); } else { - resolve(stdout); + if (silentLogs) { + const successMessage = determineSuccessMessage(args); + if (successMessage) { + console.info(successMessage); + } + resolve(''); + } else { + resolve(silent ? '' : stdout); + } } }); } @@ -365,6 +407,73 @@ function getProjectFlagsFromOptions({ projects = [] } = {}) { return projects.reduce((flags, project) => flags.concat('-p', project), []); } +/** + * Determines an appropriate success message based on the command. + * + * @param {string[]} args Command line arguments passed to sentry-cli + * @returns {string|null} Success message to display or null if no message needed + */ +function determineSuccessMessage(args) { + if (!args || args.length === 0) { + return null; + } + + const command = args[0]; + + // Only show success messages for high-impact operations customers care about + switch (command) { + case 'releases': + if (args[1] === 'new' && args[2]) { + return `✓ Release ${args[2]} created`; + } else if (args[1] === 'finalize' && args[2]) { + return `✓ Release ${args[2]} finalized`; + } else if (args[1] === 'files' && args[3] === 'upload-sourcemaps') { + return `✓ Source maps uploaded`; + } + break; + + case 'sourcemaps': + if (args[1] === 'upload') { + return `✓ Source maps uploaded`; + } else if (args[1] === 'inject') { + return `✓ Source maps injected`; + } + break; + + case 'debug-files': + if (args[1] === 'upload') { + return `✓ Debug files uploaded`; + } + break; + + case 'upload-proguard': + return `✓ ProGuard mappings uploaded`; + + case 'upload-dif': + case 'upload-dsym': + return `✓ Debug information files uploaded`; + + case 'deploys': + if (args[1] === 'new') { + return `✓ Deploy created`; + } + break; + + case 'send-event': + return `✓ Event sent`; + + case 'send-envelope': + return `✓ Envelope sent`; + + // Don't show success messages for info/list operations - they show their own output + // Don't show for --version, --help - the output is the success + default: + return null; + } + + return null; +} + module.exports = { execute, getPath, @@ -375,4 +484,5 @@ module.exports = { getDistributionForThisPlatform, throwUnsupportedPlatformError, getFallbackBinaryPath, + determineSuccessMessage, }; diff --git a/js/index.js b/js/index.js index fd70b0f0c3..8914ec06f6 100644 --- a/js/index.js +++ b/js/index.js @@ -34,7 +34,11 @@ class SentryCli { if (typeof configFile === 'string') { this.configFile = configFile; } - this.options = options || { silent: false }; + this.options = { + silent: false, + silentLogs: false, + ...options, + }; this.releases = new Releases({ ...this.options, configFile }); } @@ -65,7 +69,14 @@ class SentryCli { * @returns {Promise.} A promise that resolves to the standard output. */ execute(args, live) { - return helper.execute(args, live, this.options.silent, this.configFile, this.options); + return helper.execute( + args, + live, + this.options.silent, + this.options.silentLogs, + this.configFile, + this.options + ); } } diff --git a/js/releases/__tests__/index.test.js b/js/releases/__tests__/index.test.js index fae20dd482..287aabf533 100644 --- a/js/releases/__tests__/index.test.js +++ b/js/releases/__tests__/index.test.js @@ -7,7 +7,7 @@ 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', () => { @@ -33,8 +33,9 @@ describe('SentryCli releases', () => { ['releases', 'new', 'my-version'], null, false, + false, undefined, - { silent: false } + { silent: false, silentLogs: false } ); }); test('with projects', async () => { @@ -43,8 +44,9 @@ describe('SentryCli releases', () => { ['releases', 'new', 'my-version', '-p', 'proj-a', '-p', 'proj-b'], null, false, + false, undefined, - { silent: false } + { silent: false, silentLogs: false } ); }); }); @@ -63,8 +65,9 @@ describe('SentryCli releases', () => { ], true, false, + false, undefined, - { silent: false } + { silent: false, silentLogs: false } ); }); test('with projects', async () => { @@ -88,8 +91,9 @@ describe('SentryCli releases', () => { ], true, false, + false, undefined, - { silent: false } + { silent: false, silentLogs: false } ); }); @@ -100,7 +104,7 @@ 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', @@ -113,8 +117,9 @@ describe('SentryCli releases', () => { ], true, false, + false, undefined, - { silent: false } + { silent: false, silentLogs: false } ) ); }); @@ -139,8 +144,9 @@ describe('SentryCli releases', () => { ], true, false, + false, undefined, - { silent: false } + { silent: false, silentLogs: false } ); expect(mockExecute).toHaveBeenCalledWith( @@ -155,8 +161,9 @@ describe('SentryCli releases', () => { ], true, false, + false, undefined, - { silent: false } + { silent: false, silentLogs: false } ); }); @@ -174,8 +181,9 @@ describe('SentryCli releases', () => { ], live, false, + false, undefined, - { silent: false } + { silent: false, silentLogs: false } ); }); }); diff --git a/js/releases/index.js b/js/releases/index.js index c3fa261d66..cf8fa24ca7 100644 --- a/js/releases/index.js +++ b/js/releases/index.js @@ -267,7 +267,14 @@ class Releases { * @returns {Promise.} A promise that resolves to the standard output. */ async execute(args, live) { - return helper.execute(args, live, this.options.silent, this.configFile, this.options); + return helper.execute( + args, + live, + this.options.silent, + this.options.silentLogs, + this.configFile, + this.options + ); } }