Skip to content

fix(gatsby): Don't crash build when auth token is missing #7858

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

Merged
merged 2 commits into from
Apr 14, 2023

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Apr 14, 2023

Our Gatsby SDK ships with the webpack plugin to automatically upload source maps (TIL).

When building for production, the webpack causes a crash if no auth token is specified. We decided that builds shouldn't crash because of a misconfiguration of our plugin, so this PR fixes that (see #7427 and #7846). Instead, we'll log a warning in this case.
Note: I opted to not log a warning if ORG or PROJECT were not specified because in this case one could argue that users don't want to upload at all. This is identical to the previous behaviour. Happy to change that though, if reviewers think we should notify users anyway.

closes #7852

@Lms24 Lms24 requested review from a team, mydea and AbhiPrasad and removed request for a team April 14, 2023 09:02
@Lms24 Lms24 self-assigned this Apr 14, 2023
if (message.includes('organization slug is required') || message.includes('project slug is required')) {
return;
}
if (message.includes('authentication credentials were not provided')) {
// eslint-disable-next-line no-console
console.warn('Sentry Logger [Warn]: Cannot upload source maps due to missing SENTRY_AUTH_TOKEN env variable.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.warn('Sentry Logger [Warn]: Cannot upload source maps due to missing SENTRY_AUTH_TOKEN env variable.')
console.warn('Sentry [Warn]: Cannot upload source maps due to missing SENTRY_AUTH_TOKEN env variable.')

logaf giga l

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair, a bit further below, we had Sentry Logger [] so I just duplicated it but we can also remove it in both places

@@ -46,10 +46,15 @@ exports.onCreateWebpackConfig = ({ plugins, getConfig, actions }) => {
// Handle sentry-cli configuration errors when the user has not done it not to break
// the build.
errorHandler(err, invokeErr) {
const { message } = err;
const message = err.message && err.message.toLowerCase() || '';
if (message.includes('organization slug is required') || message.includes('project slug is required')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

l: Do we maybe wanna add a similar error message right here as a quick win?

Copy link
Member Author

Choose a reason for hiding this comment

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

As I wrote in the PR, there's an argument to be made that if these env vars are not specified, users don't want to upload source maps and hence there should be no warning. But let's just display a "info" level message here that we're not gonna upload source maps in case these variables aren't set.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sorry! I skipped over that. I think printing a message is good here since having source maps is such a productivity win - I think we wanna tell users about it :D

@Lms24 Lms24 merged commit e424a32 into develop Apr 14, 2023
@Lms24 Lms24 deleted the lms/gatsby-fix-crash-no-auth-token branch April 14, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gatsby build fails when run locally without SENTRY_AUTH_TOKEN
2 participants