Skip to content

Update Sentry dependencies #1238

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
Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from
Open

Update Sentry dependencies #1238

wants to merge 4 commits into from

Conversation

wojtekn
Copy link
Contributor

@wojtekn wojtekn commented Apr 22, 2025

Related issues

  • N/A

Proposed Changes

  • I propose to update Sentry dependencies to fix security issues uncovered by Dependabot.

Testing Instructions

  • N/A

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@wojtekn wojtekn requested a review from a team April 22, 2025 10:41
@wojtekn wojtekn force-pushed the update/sentry-dependencies branch from 22a077d to c3f3cd3 Compare April 22, 2025 10:44
katinthehatsite
katinthehatsite approved these changes Apr 22, 2025
View reviewed changes
Copy link
Contributor

@katinthehatsite katinthehatsite left a comment

Choose a reason for hiding this comment

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

Not sure how to test this, I opened the app, looked through and everything seemed to be correct.

wojtekn
wojtekn commented Apr 22, 2025
View reviewed changes
src/renderer.ts
},
reactInit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I keep the reactInit, it breaks the renderer. Sentry docs say we can pass it if we use framework specific SDK: Framework-Specific SDKs.

@nightnei could you please advise how to test breadcrumb code with that change?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't test it, sicne looks like we have and issue in Sentry with limitations:
Screenshot 2025-04-23 at 14 26 54

Also more info here: p1745411730036399-slack-C04GESRBWKW

@wojtekn wojtekn self-assigned this Apr 22, 2025
gcsecsey
gcsecsey approved these changes Apr 24, 2025
View reviewed changes
Copy link
Contributor

@gcsecsey gcsecsey left a comment

Choose a reason for hiding this comment

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

Changes LGTM. I tested a clean build with these changes, and Sentry still initializes as on trunk:
CleanShot 2025-04-24 at 10 25 58@2x

nightnei
nightnei approved these changes Apr 24, 2025
View reviewed changes
Copy link
Contributor

@nightnei nightnei left a comment

Choose a reason for hiding this comment

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

LGTM and works well 👍
Screenshot 2025-04-24 at 12 50 05
Screenshot 2025-04-24 at 12 50 18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nightnei nightnei nightnei approved these changes

@gcsecsey gcsecsey gcsecsey approved these changes

@katinthehatsite katinthehatsite katinthehatsite approved these changes

Assignees

@wojtekn wojtekn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wojtekn @nightnei @gcsecsey @katinthehatsite