Skip to content

fix: enable console logging of Vue errors in development #214

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
Jul 28, 2020
Merged

Conversation

rchl
Copy link
Member

@rchl rchl commented Jul 28, 2020

Sentry by default swallows all errors caught by Vue error handler. This
options re-posts them so that they show up in the console.

I have always had this enabled in my configuration and I think it makes sense to have it enabled by default.

Sentry by default swallows all errors caught by Vue error handler. This
options re-posts them so that they show up in the console.
@rchl rchl requested a review from pimlie July 28, 2020 13:01
@codecov
Copy link

codecov bot commented Jul 28, 2020

Codecov Report

Merging #214 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #214   +/-   ##
=======================================
  Coverage   55.55%   55.55%           
=======================================
  Files           1        1           
  Lines          27       27           
  Branches        8        8           
=======================================
  Hits           15       15           
  Misses          9        9           
  Partials        3        3           
Impacted Files Coverage Δ
lib/module.js 55.55% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8236472...ad99ee0. Read the comment docs.

lib/module.js Outdated
@@ -28,7 +28,7 @@ export default function SentryModule (moduleOptions) {
ExtraErrorData: {},
ReportingObserver: {},
RewriteFrames: {},
Vue: { attachProps: true }
Vue: { attachProps: true, logErrors: true }
Copy link
Member

Choose a reason for hiding this comment

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

Could we make it configurable anyway? That should be a quick fix and logging in production could be frowned upon..

Copy link
Member Author

@rchl rchl Jul 28, 2020

Choose a reason for hiding this comment

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

Technically it is configurable by overriding clientIntegrations.
I wouldn't be opposed to adding a configuration option specifically for that if I would have a good place for it. I don't think that adding a new global option for configuring one specific integration would be worth it...

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really see any problems with posting errors in production as those are client errors and there is nothing to hide there.

But I'm open to setting the default value to this.options.dev instead.

@rchl
Copy link
Member Author

rchl commented Jul 28, 2020

I've just realized that, since we are deep-merging options, it's easy to override settings for a specific integration but at the same time it's not possible to disable integration that is enabled by default.

But since no one has complained yet about that, we can leave as is.

@pimlie
Copy link
Member

pimlie commented Jul 28, 2020

As long as logging in prod is configurable then Im happy :)

@rchl rchl changed the title fix: enable console logging of Vue errors fix: enable console logging of Vue errors in development Jul 28, 2020
@rchl rchl merged commit 55b7efe into master Jul 28, 2020
@rchl rchl deleted the fix/log-errors branch July 28, 2020 19: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.

None yet

2 participants