fix: Only pass `urlPrefix` to sentry-cli if it's not empty #275

andreiborza · 2025-03-19T07:08:32Z

Please see 511c3f3 for the relevant changes.

I also reworked how projects are passed to the CLI. Previously the CLI was invoked once per project, but I saw that sentry-cli does actually support passing multiple projects so I reworked that.

Here's a screenshot from the report email showing the two projects I specified in a private repo.

Closes: #137

Lms24 · 2025-03-19T08:10:17Z

action.yml

        INPUT_DISABLE_TELEMETRY: ${{ inputs.disable_telemetry }}
        INPUT_DISABLE_SAFE_DIRECTORY: ${{ inputs.disable_safe_directory }}
-      uses: docker://ghcr.io/getsentry/action-release-image:master
+      uses: docker://ghcr.io/getsentry/action-release-image:ab-avoid-sending-empty-urlPrefix


l: I guess this will be reverted before merging?

+1, seems like this should not get merged

There's a github action that will revert it on master after merging.

ah okay, makes sense. Thank you for clarifying!

Lms24 · 2025-03-19T08:12:52Z

src/main.ts

-              return getCLI().uploadSourceMaps(release, sourceMapOptions);
-            })
-          );
+          const sourceMapsOptions: SentryCliUploadSourceMapsOptions & {


that's a nice change! I guess this improves upload speeds a bit in multi-project scenarios (?)

I'd guess it does, not sure by how much tho but there are over 200 orgs that use multi-project scenarios so hopefully they'll see some kind of impact.

Actually, turns out this does not work. It only uploads source maps for the first project supplied :(

I changed it back.

szokeasaurusrex

sentry-cli sourcemaps upload indeed only supports uploading one project at a time. I think it is possible to pass multiple projects but all except the first one are ignored. I realize this is a bad user experience (it was already this way before I took over maintaining Sentry CLI), and I would like to add support for uploading multiple projects at once, but for now, the action needs to only pass one project at a time.

szokeasaurusrex · 2025-03-19T08:17:49Z

action.yml

        INPUT_DISABLE_TELEMETRY: ${{ inputs.disable_telemetry }}
        INPUT_DISABLE_SAFE_DIRECTORY: ${{ inputs.disable_safe_directory }}
-      uses: docker://ghcr.io/getsentry/action-release-image:master
+      uses: docker://ghcr.io/getsentry/action-release-image:ab-avoid-sending-empty-urlPrefix


+1, seems like this should not get merged

szokeasaurusrex · 2025-03-19T08:19:08Z

src/main.ts

-              return getCLI().uploadSourceMaps(release, sourceMapOptions);
-            })
-          );
+          const sourceMapsOptions: SentryCliUploadSourceMapsOptions & {


Just wondering since I don't know much about typescript: what does this & do here?

It widens the type to also include projects as a key. SentryCliUploadSourceMapsOptions does not have a projects key, I assumed that's because it's not really an upload option but rather an option to sentry-cli itself so I just widen here.

szokeasaurusrex · 2025-03-19T08:20:30Z

src/main.ts

          core.debug(`Adding sourcemaps`);
-          await Promise.all(
-            projects.map(async project => {
-              // upload source maps can only do one project at a time


This is still accurate

Thanks, will revert.

I had a look at both projects and saw that both had 4 artifacts uploaded, but upon clicking the artifacts link only one of them actually had source maps uploaded.

Project that has the uploads: https://sentry-sdks.sentry.io/explore/releases/3da6213c7f9ebf252ae055dea015b9cae688f1c9/?notification_uuid=6b722c0d-04be-4cf1-bd24-9fbd19ef7c13&project=4508602366230528

Project that shows it has 4 artifacts but when clicking on them no artifacts are to be found: https://sentry-sdks.sentry.io/explore/releases/3da6213c7f9ebf252ae055dea015b9cae688f1c9/?notification_uuid=6b722c0d-04be-4cf1-bd24-9fbd19ef7c13&project=4505545558327296

szokeasaurusrex

@andreiborza Could you please remove da1a031 from this PR? Now that there are multiple relevant commits, it's quite difficult to review this PR with da1a031 included here

andreiborza · 2025-03-20T01:19:35Z

@szokeasaurusrex removed and reapplied. Relevant code to review is in 140c6f0 reformatting in 1f07990.

Also lets sentry-cli handle uploading source maps for multiple projects.

szokeasaurusrex

Looks good from CLI perspective, but since I am not super experienced in JS/TS please have a second approval before merging, from @Lms24 or someone else with JS/TS experience.

andreiborza requested review from Lms24, chargome and szokeasaurusrex and removed request for Lms24 and chargome March 19, 2025 07:08

andreiborza changed the title ~~fix: Only passes urlPrefix to sentry-cli if it's not empty~~ fix: Only passes urlPrefix to sentry-cli if it's not empty Mar 19, 2025

andreiborza force-pushed the ab/avoid-sending-empty-urlPrefix branch from 48ae124 to da1a031 Compare March 19, 2025 07:12

andreiborza requested review from Lms24 and chargome March 19, 2025 08:00

andreiborza changed the title ~~fix: Only passes urlPrefix to sentry-cli if it's not empty~~ fix: Only pass urlPrefix to sentry-cli if it's not empty Mar 19, 2025

Lms24 approved these changes Mar 19, 2025

View reviewed changes

szokeasaurusrex requested changes Mar 19, 2025

View reviewed changes

andreiborza requested review from Lms24 and szokeasaurusrex March 19, 2025 08:54

szokeasaurusrex reviewed Mar 19, 2025

View reviewed changes

andreiborza force-pushed the ab/avoid-sending-empty-urlPrefix branch 2 times, most recently from 6b547a6 to 6ec3868 Compare March 20, 2025 01:27

andreiborza added 2 commits March 20, 2025 10:34

fix: Only passes urlPrefix to sentry-cli if it's not empty

140c6f0

Also lets sentry-cli handle uploading source maps for multiple projects.

Update prettier settings and reformat

1f07990

andreiborza force-pushed the ab/avoid-sending-empty-urlPrefix branch from 6ec3868 to 1f07990 Compare March 20, 2025 01:35

Merge branch 'master' into ab/avoid-sending-empty-urlPrefix

e149779

szokeasaurusrex approved these changes Mar 20, 2025

View reviewed changes

andreiborza merged commit a154094 into master Mar 21, 2025
19 checks passed

andreiborza deleted the ab/avoid-sending-empty-urlPrefix branch March 21, 2025 02:35

fix: Only pass urlPrefix to sentry-cli if it's not empty #275

fix: Only pass urlPrefix to sentry-cli if it's not empty #275

Uh oh!

Conversation

andreiborza commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szokeasaurusrex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szokeasaurusrex left a comment

Choose a reason for hiding this comment

Uh oh!

andreiborza commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szokeasaurusrex left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix: Only pass `urlPrefix` to sentry-cli if it's not empty #275

fix: Only pass `urlPrefix` to sentry-cli if it's not empty #275

andreiborza commented Mar 19, 2025 •

edited

Loading

andreiborza commented Mar 20, 2025 •

edited

Loading