Skip to content

feat(gatsby): Upload source maps automatically when sentry-cli is configured #4109

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.

Sign up for GitHub

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
iker-barriocanal merged 5 commits into from
Nov 8, 2021
Merged

feat(gatsby): Upload source maps automatically when sentry-cli is configured #4109

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
447de7c
feat(gatsby): Upload source maps automatically when sentry-cli is con…
iker-barriocanal Nov 2, 2021
1eb7455
fix linting
iker-barriocanal Nov 2, 2021
9c5ab34
Add comments of included and ignored files
iker-barriocanal Nov 3, 2021
815ea88
Merge branch 'master' into iker/ref/gatsby-webpack-plugin
iker-barriocanal Nov 4, 2021
1684bdd
Merge branch 'master' into iker/ref/gatsby-webpack-plugin
iker-barriocanal Nov 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions packages/gatsby/gatsby-node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
const fs = require('fs');

const SentryWebpackPlugin = require('@sentry/webpack-plugin');

const sentryRelease = JSON.stringify(
// Always read first as Sentry takes this as precedence
process.env.SENTRY_RELEASE ||
Expand Down Expand Up @@ -29,6 +31,26 @@ exports.onCreateWebpackConfig = ({ plugins, getConfig, actions }) => {
],
});

if (process.env.NODE_ENV === 'production') {
actions.setWebpackConfig({
plugins: [
new SentryWebpackPlugin({
include: 'public',
ignore: ['app-*', 'polyfill-*', 'framework-*', 'webpack-runtime-*'],
// Handle sentry-cli configuration errors when the user has not done it not to break
// the build.
errorHandler(err, invokeErr) {
const { message } = err;
if (message.includes('organization slug is required') || message.includes('project slug is required')) {
return;
Copy link
Member

@AbhiPrasad AbhiPrasad Nov 2, 2021

Choose a reason for hiding this comment

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

Does this still log the error if we do an early return?

Copy link
Contributor

@kamilogorek kamilogorek Nov 2, 2021

Choose a reason for hiding this comment

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

It won't

Copy link
Member

@AbhiPrasad AbhiPrasad Nov 4, 2021

Choose a reason for hiding this comment

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

@iker-barriocanal Feel like we need a way to tell users about misconfigured sentry-cli without it being blocking. Can we take advantage of the debug flag somehow?

Copy link
Contributor Author

@iker-barriocanal iker-barriocanal Nov 4, 2021

Choose a reason for hiding this comment

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

What do you mean exactly? Telling users when they haven't correctly configured sentry-cli? If this is the case:

  • Enabling debug will generate too much noise, and they can always customize this by setting SENTRY_LOG_LEVEL env var, see configuration values.
  • The code to check whether the file and its values are correct would be too complex and too hard to maintain for the value it provides. Are we planning to check if a sentry-cli config file exists? Or, checking the format is correct? Or, checking if the values are correct (need to ask Sentry whether the org exists)?

Considering sentry-cli has proper docs, failing doesn't have major consequences (not uploading source maps for a misconfiguration, which they can read the docs of, or try to have their own config, can retry as many times as they wish at no cost (not a production issue)...), and we can update Gatsby docs if the process is clear enough, I don't think it's worth it to provide additional checks. I'd rather remove this UX improvement over adding it and potentially provide a worse UX.

Note that I might have misunderstood what you meant.

Copy link
Member

@AbhiPrasad AbhiPrasad Nov 5, 2021

Choose a reason for hiding this comment

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

I just wanted to avoid situations where someone forgets to set an org slug, and doesn't understand why source maps uploading isn't working for gatsby (since the error is being swallowed).

I think we should add to docs how user's can define their own custom config with the webpack plugin.

Copy link
Contributor Author

@iker-barriocanal iker-barriocanal Nov 8, 2021

Choose a reason for hiding this comment

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

It's a good point, but I don't think it's worth covering it in this context. As for the docs, we link to the Sentry Webpack docs here; although these sections could be improved, is there anything specific you have in mind?

}
invokeErr(err);
},
}),
],
});
}

// To configure the SDK, SENTRY_USER_CONFIG is prioritized over `gatsby-config.js`,
// since it isn't possible to set non-serializable parameters in the latter.
// Prioritization here means what `init` is run.
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
},
"dependencies": {
"@sentry/react": "6.13.3",
"@sentry/tracing": "6.13.3"
"@sentry/tracing": "6.13.3",
"@sentry/webpack-plugin": "1.18.3"
},
"peerDependencies": {
"gatsby": "^2.0.0 || ^3.0.0"
Expand Down
19 changes: 19 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2832,13 +2832,32 @@
progress "^2.0.3"
proxy-from-env "^1.1.0"

"@sentry/cli@^1.70.1":
version "1.70.1"
resolved "https://registry.yarnpkg.com/@sentry/cli/-/cli-1.70.1.tgz#908517b699c0714eff88bedb68c6ea72e94945e8"
integrity sha512-pab3nU3rNsq1HKKmJP2ITDl5cGi+QbQ5eLX6ELaeAbN2eAzfndHu2rTqlnjJAKcYQg6l9gFBn8vvY2xAeRJb6Q==
dependencies:
https-proxy-agent "^5.0.0"
mkdirp "^0.5.5"
node-fetch "^2.6.0"
npmlog "^4.1.2"
progress "^2.0.3"
proxy-from-env "^1.1.0"

"@sentry/[email protected]":
version "1.18.1"
resolved "https://registry.yarnpkg.com/@sentry/webpack-plugin/-/webpack-plugin-1.18.1.tgz#0fa24297043305057111d85a7154d4b8b24d43a6"
integrity sha512-maEnHC0nxRnVgAz0qvKvhTGy+SxneR8MFjpgNMvh9CyAB6GEM9VQI1hzxTcAd7Qk90qGW8W4eUmB+ZX8nMrM1w==
dependencies:
"@sentry/cli" "^1.68.0"

"@sentry/[email protected]":
version "1.18.3"
resolved "https://registry.yarnpkg.com/@sentry/webpack-plugin/-/webpack-plugin-1.18.3.tgz#1cd3401f84f561b4a451dac5f42465ee5102f5d6"
integrity sha512-Qk3Jevislc5DZK0X/WwRVcOtO7iatnWARsEgTV/TuXvDN+fUDDpD/2MytAWAbpLaLy3xEB/cXGeLsbv6d1XNkQ==
dependencies:
"@sentry/cli" "^1.70.1"

"@simple-dom/interface@^1.4.0":
version "1.4.0"
resolved "https://registry.yarnpkg.com/@simple-dom/interface/-/interface-1.4.0.tgz#e8feea579232017f89b0138e2726facda6fbb71f"
Expand Down