Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show Sentry CLI output when uploading artifacts #11100

Merged
merged 2 commits into from
May 19, 2021

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented May 14, 2021

The function we were using to run shell commands during the sentry:publish script were swallowing the CLI output. We also weren't correctly detecting the process exit in some cases.

The run-command module originally written for auto-changelog (introduced in #10782 and replaced in #10993) has been resurrected for running commands where we don't care about the output, or where we want to use the output for something. A second function (runInShell) has been added for running commands with the same STDOUT and STDERR streams, so that the output is sent directly to the CLI. This ensures that the console output from the shell script we run gets correctly output to the CLI.

Manual testing steps for release artifact publishing:

  • Create a free Sentry account and log in, and create a project.
  • Navigate to "Settings => Developer Settings => "New Internal Integration"
  • Assign a name to this integration, and grant it a "Release" permission of "Admin".
  • After saving this new integration, take note of the token that should be listed on the integration page.
  • Run the command yarn sentry:publish with the environment variable SENTRY_AUTH_TOKEN set to the token from that integration, and with SENTRY_ORG set to your account name and SENTRY_PROJECT set to the project name you created.
  • It should complete successfully. You should see the release under "Releases" with the correct project selected. It should have 22 artifacts.
  • You should see the upload process output via the CLI, including the "all done!" message when it completes.

You can also try testing with either of the three required environment variables missing, to see that the error cases behave correctly.

@metamaskbot
Copy link
Collaborator

Builds ready [0f84685]
Page Load Metrics (645 ± 57 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint51736473
domContentLoaded38079864411957
load38179964511957
domInteractive38079764411957

@Gudahtt
Copy link
Member Author

Gudahtt commented May 14, 2021

This depends upon #11085

@Gudahtt Gudahtt force-pushed the use-run-command-for-sentry-cli branch from 0f84685 to 7a86e1d Compare May 14, 2021 21:47
@metamaskbot
Copy link
Collaborator

Builds ready [7a86e1d]
Page Load Metrics (660 ± 49 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint46816584
domContentLoaded41781365810149
load41981466010149
domInteractive41781265810149

@Gudahtt Gudahtt force-pushed the use-run-command-for-sentry-cli branch from 7a86e1d to 9c3df97 Compare May 15, 2021 17:21
@metamaskbot
Copy link
Collaborator

Builds ready [9c3df97]
Page Load Metrics (603 ± 41 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint45856194
domContentLoaded3987016028641
load3997026038641
domInteractive3977016018641

Base automatically changed from extract-sentry-account-details-to-environment-variables to develop May 18, 2021 16:26
@Gudahtt Gudahtt force-pushed the use-run-command-for-sentry-cli branch from 9c3df97 to 4b81ed7 Compare May 18, 2021 16:26
@metamaskbot
Copy link
Collaborator

Builds ready [4b81ed7]
Page Load Metrics (571 ± 40 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint458062105
domContentLoaded3796465708541
load3846475718440
domInteractive3796465708541

@Gudahtt Gudahtt marked this pull request as ready for review May 18, 2021 16:49
@Gudahtt Gudahtt requested review from kumavis and a team as code owners May 18, 2021 16:49
@Gudahtt Gudahtt requested a review from darkwing May 18, 2021 16:49
The function we were using to run shell commands during the
`sentry:publish` script were swallowing the CLI output. We also weren't
correctly detecting the process exit in some cases.

The `run-command` module originally written for `auto-changelog`
(introduced in #10782 and replaced in #10993) has been resurrected for
running commands where we don't care about the output, or where we want
to use the output for something. A second function (`runInShell`) has
been added for running commands with the same STDOUT and STDERR
streams, so that the output is sent directly to the CLI. This ensures
that the console output from the shell script we run gets correctly
output to the CLI.
@Gudahtt Gudahtt force-pushed the use-run-command-for-sentry-cli branch from 4b81ed7 to 46d01ca Compare May 19, 2021 17:02
@metamaskbot
Copy link
Collaborator

Builds ready [46d01ca]
Page Load Metrics (645 ± 39 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint448361105
domContentLoaded4228076438239
load4318096458139
domInteractive4218066438239

@darkwing
Copy link
Contributor

Awesome, this worked perfectly as described. Well done @Gudahtt !

@Gudahtt Gudahtt merged commit 3540a5b into develop May 19, 2021
@Gudahtt Gudahtt deleted the use-run-command-for-sentry-cli branch May 19, 2021 17:24
@github-actions github-actions bot locked and limited conversation to collaborators May 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants