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 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 6b12318 to 40b77a1 Compare November 19, 2025 13:40
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! (cc @JPeer264 since we just talked about this)

### 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 40b77a1 to bd678cf Compare November 24, 2025 15:15
@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').

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