Skip to content

fix(vue): Remove logError from vueIntegration - #14958

Merged
s1gr1d merged 4 commits into
developfrom
sig/vue-throw-error-v9
Jan 10, 2025
Merged

fix(vue): Remove logError from vueIntegration#14958
s1gr1d merged 4 commits into
developfrom
sig/vue-throw-error-v9

Conversation

@s1gr1d

@s1gr1ds1gr1d commented Jan 9, 2025

Copy link
Copy Markdown
Member

Removes logError and always re-throws the error when it's not handled by the user.

PR for v8 (without removing logError): #14943

Logging the error has been a problem in the past already (see #7310). By just re-throwing the error we don't mess around with the initial message and stacktrace.

@github-actions

github-actionsBot commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser23.01 KB--
@sentry/browser - with treeshaking flags21.68 KB--
@sentry/browser (incl. Tracing)35.53 KB--
@sentry/browser (incl. Tracing, Replay)72.31 KB--
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags62.83 KB--
@sentry/browser (incl. Tracing, Replay with Canvas)76.57 KB--
@sentry/browser (incl. Tracing, Replay, Feedback)88.59 KB--
@sentry/browser (incl. Feedback)39.23 KB--
@sentry/browser (incl. sendFeedback)27.64 KB--
@sentry/browser (incl. FeedbackAsync)32.41 KB--
@sentry/react25.7 KB--
@sentry/react (incl. Tracing)38.29 KB--
@sentry/vue27.12 KB-0.27%-73 B 🔽
@sentry/vue (incl. Tracing)37.27 KB-0.19%-72 B 🔽
@sentry/svelte23.13 KB--
CDN Bundle24.28 KB--
CDN Bundle (incl. Tracing)35.82 KB--
CDN Bundle (incl. Tracing, Replay)70.47 KB--
CDN Bundle (incl. Tracing, Replay, Feedback)75.62 KB--
CDN Bundle - uncompressed70.78 KB--
CDN Bundle (incl. Tracing) - uncompressed106.08 KB--
CDN Bundle (incl. Tracing, Replay) - uncompressed216.91 KB--
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed229.44 KB--
@sentry/nextjs (client)38.42 KB--
@sentry/sveltekit (client)36.08 KB--
@sentry/node161.72 KB--
@sentry/node - without tracing97.48 KB--
@sentry/aws-serverless127.36 KB--

View base workflow run

@s1gr1ds1gr1d self-assigned this Jan 9, 2025
Comment threadpackages/vue/src/errorhandler.ts Outdated
captureException(error, {
captureContext: { contexts: { vue: metadata } },
mechanism: { handled: false },
mechanism: { handled: !!originalErrorHandler, type: 'vue-errorHandler' },

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.

Typically we just use the framework name as the mechanism type, so:

Suggested change
mechanism: {handled: !!originalErrorHandler,type: 'vue-errorHandler'},
mechanism: {handled: !!originalErrorHandler,type: 'vue'},

We have a brief comment about this in relay, but I'll make a PR to develop to make this more clear.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's my "bad" - I didn't know that this was more than coincidence in the places where we set a custom type. Thanks for pointing this out and develop sounds good!

* When set to `true`, original Vue's `logError` will be called as well.
* https://github.com/vuejs/vue/blob/c2b1cfe9ccd08835f2d99f6ce60f67b4de55187f/src/core/util/error.js#L38-L48
*/
logErrors: boolean;

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 wonder if there is an easy way to deprecate this in v8

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I deprecated it in this PR: #14943

@s1gr1d
s1gr1d requested a review from AbhiPrasadJanuary 9, 2025 14:45
s1gr1d added a commit that referenced this pull request Jan 9, 2025
@s1gr1d
s1gr1d merged commit 04711c2 into developJan 10, 2025
@s1gr1d
s1gr1d deleted the sig/vue-throw-error-v9 branch January 10, 2025 10:37
Sign up for freeto 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

@s1gr1d@Lms24@AbhiPrasad