-
Notifications
You must be signed in to change notification settings - Fork 26
feat(worker): Add sentry to xapian worker scope and angular update #1618
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
base: master
Are you sure you want to change the base?
feat(worker): Add sentry to xapian worker scope and angular update #1618
Conversation
src/build/add-sentry.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@castaway , this approach of code gen and using git to revert is very messy and was causing me issues. I suggest we have this one env file that is generated using the ./gen-env.js
script. We use this module instead of interpolating writing and reverting with git.
fae7ce7
to
2674e2a
Compare
src/app/sentry.ts
Outdated
import * as Sentry from '@sentry/browser'; | ||
|
||
Sentry.init({ | ||
dsn: environment.SENTRY_DSN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could add to capture console.error messages:
integrations: [Sentry.captureConsoleIntegration()],
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got this working after an update of typescript, angular and sentry. Integrations are not supported by the Sentry version we are using. The newer version of Sentry required a later version of Typescript. Hence the typescript and angular update.
61f8cf9
to
35262b5
Compare
hi @shadowbas do the account setting tests (first e2e one) run on your dev box? mine keep failing (tho they've never been terribly stable).. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good and seems to run .. only issue I have is with the access-settings (first e2e test) failing locally
One might have noticed that we currently have console. calls registered as breadcrumbs. This behavior is only part of Sentry@5. Later versions of Sentry no longer do this by default and require the console integration. |
35262b5
to
606b48e
Compare
Add Sentry to the worker context. Also log console logs to sentry. This required Update sentry and required deps. A newer version of sentry requireded an update of typescript. A newer version of typescript required an update of angular.
48112d6
to
a46d3b7
Compare
Add sentry to the Xapian worker.
Related: #1617