Skip to content
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

Jump to bottom

fix(v8/vue): Re-throw error when no errorHandler exists #14943

Merged
merged 2 commits into from
Jan 9, 2025
Merged

fix(v8/vue): Re-throw error when no errorHandler exists #14943

merged 2 commits into from
Jan 9, 2025
+126 −143

Conversation

s1gr1d
Copy link
Member

@s1gr1d s1gr1d commented Jan 8, 2025 •
edited
Loading

PR for v9: #14958

Lms24
Lms24 reviewed Jan 8, 2025
View reviewed changes
Copy link
Member

@Lms24 Lms24 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! Once we added/updated tests for this :shipit:

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 8, 2025 •
edited
Loading

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.29 KB added added
@sentry/browser - with treeshaking flags 21.96 KB added added
@sentry/browser (incl. Tracing) 35.85 KB added added
@sentry/browser (incl. Tracing, Replay) 73.09 KB added added
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 63.48 KB added added
@sentry/browser (incl. Tracing, Replay with Canvas) 77.4 KB added added
@sentry/browser (incl. Tracing, Replay, Feedback) 89.34 KB added added
@sentry/browser (incl. Feedback) 39.5 KB added added
@sentry/browser (incl. sendFeedback) 27.89 KB added added
@sentry/browser (incl. FeedbackAsync) 32.69 KB added added
@sentry/react 25.96 KB added added
@sentry/react (incl. Tracing) 38.66 KB added added
@sentry/vue 27.57 KB added added
@sentry/vue (incl. Tracing) 37.71 KB added added
@sentry/svelte 23.45 KB added added
CDN Bundle 24.49 KB added added
CDN Bundle (incl. Tracing) 37.56 KB added added
CDN Bundle (incl. Tracing, Replay) 72.75 KB added added
CDN Bundle (incl. Tracing, Replay, Feedback) 78.11 KB added added
CDN Bundle - uncompressed 71.93 KB added added
CDN Bundle (incl. Tracing) - uncompressed 111.42 KB added added
CDN Bundle (incl. Tracing, Replay) - uncompressed 225.5 KB added added
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 238.6 KB added added
@sentry/nextjs (client) 38.92 KB added added
@sentry/sveltekit (client) 36.36 KB added added
@sentry/node 162.8 KB added added
@sentry/node - without tracing 98.94 KB added added
@sentry/aws-serverless 126.63 KB added added

Lms24
Lms24 approved these changes Jan 9, 2025
View reviewed changes
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Realizing we already have tests that need to be adjusted anyway, so feel free to merge once this is done (ofc feel free to add tests if it makes sense). Thanks for tackling this!

@s1gr1d s1gr1d added the Package: vue Issues related to the Sentry Vue SDK label Jan 9, 2025
Lms24
Lms24 reviewed Jan 9, 2025
View reviewed changes
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

One more Q but feel free to merge once it is addressed :)

@s1gr1d s1gr1d force-pushed the sig/vue-error-throw branch from 70c98ae to a6459e2 Compare January 9, 2025 13:04
@s1gr1d s1gr1d requested review from a team as code owners January 9, 2025 13:04
@s1gr1d s1gr1d changed the base branch from develop to v8 January 9, 2025 13:05
@s1gr1d s1gr1d changed the title fix(vue): Re-throw error when no errorHandler exists fix(v8/vue): Re-throw error when no errorHandler exists Jan 9, 2025
@s1gr1d s1gr1d mentioned this pull request Jan 9, 2025
Merged
@s1gr1d s1gr1d merged commit 8926cb7 into v8 Jan 9, 2025
127 checks passed
@s1gr1d s1gr1d deleted the sig/vue-error-throw branch January 9, 2025 15:26
s1gr1d added a commit that referenced this pull request Jan 10, 2025
@s1gr1d
fix(vue): Remove logError from vueIntegration (#14958)
04711c2 
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.
This was referenced Jan 10, 2025
Merged
Merged
This was referenced Jan 27, 2025
Merged
Merged
Merged
@Lms24 Lms24 mentioned this pull request Feb 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lms24 Lms24

Assignees
No one assigned
Labels
Package: vue Issues related to the Sentry Vue SDK
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@s1gr1d @Lms24