Skip to content

Add sentry#981

Merged
shcheklein merged 1 commit into
masterfrom
sentry-integration
Feb 7, 2020
Merged

Add sentry#981
shcheklein merged 1 commit into
masterfrom
sentry-integration

Conversation

@iAdramelk
Copy link
Copy Markdown
Contributor

Test integration with Sentry. To make it work we need to add env variable SENTRY_DSN to Heroku.

@shcheklein shcheklein temporarily deployed to dvc-landing-sentry-inte-rzvicf February 7, 2020 14:02 Inactive
@iAdramelk iAdramelk temporarily deployed to dvc-landing-sentry-inte-rzvicf February 7, 2020 14:04 Inactive
@iAdramelk iAdramelk temporarily deployed to dvc-landing-sentry-inte-rzvicf February 7, 2020 14:05 Inactive
@shcheklein shcheklein merged commit 4d2194e into master Feb 7, 2020
@jorgeorpinel
Copy link
Copy Markdown
Contributor

Interesting. So Sentry is a SRE/APM dashboard? I quickly read https://blog.sentry.io/2018/03/06/the-sentry-workflow but would be great to know what this is all about 🙂

@iAdramelk
Copy link
Copy Markdown
Contributor Author

@jorgeorpinel , yep. Pretty much it. We can collect info about user's javascirpt errors with it, see information about environment, stack traces, etc.

Comment thread next.config.js
@@ -0,0 +1,14 @@
/* eslint-env node */

const withSourceMaps = require('@zeit/next-source-maps')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see @zeit/next-source-maps was also added here. Is this specifically just a nice addition for Sentry logs/traces?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Source maps is a mechanism to map bundled and minified code with the original source files. If we just run sentry as it is, it will show errors like: You nave an error on line 1, symbol 3454560 in the gX method. Source maps will ideally show us original file, line and method name. (But right now it's looks like it didn't work correctly)

It's not strictly related to Sentry and is used for example by browser Dev Tools, but it make using Sentry more convenient.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Right, I did read about source maps and it seems like a great feature. I was just wondering if this was added specifically for Sentry traces or whether it can also be used on the Browser now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, we should be, because I leave them enabled (there is also an option to send them directly to sentry through API and not publish on site). But in practice from that I see, I misconfigured them somehow and didn't see them anywhere.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK cool please let us know when you figure it out 🙂

Comment thread pages/_app.js
Comment thread pages/_document.js
Comment on lines +8 to +14
process.on('unhandledRejection', err => {
Sentry.captureException(err)
})

process.on('uncaughtException', err => {
Sentry.captureException(err)
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are these needed if Sentry supposedly has automated handling of uncaught exceptions and unhandled rejections? See https://docs.sentry.io/platforms/javascript/#automatically-capturing-errors

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's for server side errors in node.js, Sentry will automatically log errors in browser, but not unhandled errors in SSR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Got it, thanks.

Would these also be needed separately for server.js or other build-time code? e.g. sidebar.js

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This catches all code in the actual pages including sidebar.js, but I think we are now missing errors from server.js because they happens before we call _document.js.

Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Feb 12, 2020

Choose a reason for hiding this comment

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

Yes some server code runs before the Next app and partially includes sidebar.js code I think. That's what I was wondering about. But it's probably not a big deal? Only runs when the server is being initialized, probably only when new versions are deployed to Heroku (or when they restart it automatically for whatever reason).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's pretty much my thoughts as well. And also the fact that we are planning to migrate to Gatsby and it didn't have a server. So I decided to leave it as is for now.

Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Thanks @iAdramelk. A few more questions here ^

@shcheklein shcheklein deleted the sentry-integration branch March 27, 2020 20:08
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.

3 participants