Skip to content

Conversation

@szokeasaurusrex
Copy link
Member

@szokeasaurusrex szokeasaurusrex commented Nov 19, 2025

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

Copy link
Member Author

szokeasaurusrex commented Nov 19, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@linear
Copy link

linear bot commented Nov 19, 2025

szokeasaurusrex added a commit that referenced this pull request Nov 19, 2025
### 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)
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-live-rejectOnError branch from abf71f3 to 6b12318 Compare November 19, 2025 13:20
@szokeasaurusrex szokeasaurusrex marked this pull request as ready for review November 19, 2025 13:22
@szokeasaurusrex szokeasaurusrex requested review from a team as code owners November 19, 2025 13:22
@szokeasaurusrex szokeasaurusrex requested review from AbhiPrasad and s1gr1d and removed request for a team, AbhiPrasad and s1gr1d November 19, 2025 13:22
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-release-bundle-uploads branch from 3d01060 to 002c166 Compare November 24, 2025 15:15
// 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}`));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing else causes reject after successful resolve

The exit handler unconditionally calls reject() after conditionally calling resolve() for zero exit codes. When exitCode === 0, both resolve('success (live mode)') and reject(new Error(...)) execute sequentially. While promises only settle once, this creates an unhandled rejection and indicates incorrect control flow logic. An else clause or return statement is needed to prevent rejection after successful resolution.

Fix in Cursor Fix in Web

@szokeasaurusrex szokeasaurusrex added the v3.0 Breaking changes to include in version 3.0.0 of Sentry CLI label Nov 25, 2025
*
* @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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szokeasaurusrex Here the live option is entirely removed, but in types.ts the live option is just adapted. What is now true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly, this removal is intentional. For sourcemaps upload, we always use live = true, and there is no way to override this (this is actually what we had prior to introducing the 'rejectOnError' option; we only made live configurable here to allow us to use 'rejectOnError').

@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-release-bundle-uploads branch from 002c166 to 63f645f Compare December 3, 2025 09:24
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-live-rejectOnError branch from bd678cf to 2127f3c Compare December 3, 2025 09:24
szokeasaurusrex added a commit that referenced this pull request Dec 3, 2025
### 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)
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-live-rejectOnError branch from 2127f3c to d62403a Compare December 3, 2025 09:58
Comment on lines +335 to +338
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}`));

This comment was marked as outdated.

@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-release-bundle-uploads branch from 63f645f to 7911a86 Compare December 11, 2025 09:59
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-live-rejectOnError branch from d62403a to 45c37fb Compare December 11, 2025 09:59
szokeasaurusrex added a commit that referenced this pull request Dec 11, 2025
### 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)
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-release-bundle-uploads branch from 7911a86 to 741cd33 Compare December 11, 2025 16:10
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-live-rejectOnError branch from 45c37fb to 2d7ff71 Compare December 11, 2025 16:10
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-release-bundle-uploads branch from 741cd33 to c13f61c Compare December 11, 2025 16:25
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-live-rejectOnError branch from 2d7ff71 to 487d55a Compare December 11, 2025 16:25
### 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)
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-live-rejectOnError branch from 487d55a to 58399c6 Compare December 12, 2025 12:04
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/no-release-bundle-uploads branch from c13f61c to 2601832 Compare December 12, 2025 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v3.0 Breaking changes to include in version 3.0.0 of Sentry CLI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants