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.

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

chore: posthog and sentry exception capture #27836

Merged
merged 19 commits into from
Feb 6, 2025

Conversation

daibhin
Copy link
Contributor

@daibhin daibhin commented Jan 23, 2025

Problem

Realised we had not yet replaced manual exception capture with a wrapping function that sends data to both Sentry and PostHog

Changes

Replace capture_exception from Sentry with an equivalent method in the posthog.exceptions module which also calls PostHog's capture_exception method

Given we don't support the concept of scopes & tagging right now I just ignored them. It feels like our issues will have slightly less context as a result but capturing the exceptions is better than what we have right now

@daibhin daibhin changed the title replace more sentry calls chore: posthog and sentry exception capture Jan 23, 2025
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Contributor

Size Change: -5 B (0%)

Total Size: 1.16 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.16 MB -5 B (0%)

compressed-size-action

@daibhin daibhin requested a review from a team January 31, 2025 09:47

sentry_capture_exception(error, scope=sentry_scope, **sentry_scope_kwargs)

if api_key:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really don't like having to use this as the check but tests were failing otherwise. I tried using TEST but didn't seem to work in CI. Try / catch was also possible but seemed like overkill

Copy link
Contributor

Choose a reason for hiding this comment

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

Eh, I think we just take it (it's possible we should actually do this inside the call to capture? I'm not sure, no strong feeling)


sentry_capture_exception(error, scope=sentry_scope, **sentry_scope_kwargs)

if api_key:
Copy link
Contributor

Choose a reason for hiding this comment

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

Eh, I think we just take it (it's possible we should actually do this inside the call to capture? I'm not sure, no strong feeling)

@daibhin daibhin merged commit bbc8047 into master Feb 6, 2025
96 checks passed
@daibhin daibhin deleted the dn-chore/replace-sentry-capture-calls branch February 6, 2025 11:11
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.

3 participants