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

fix: update Sentry Integration #308

Closed
wants to merge 2 commits into from

Conversation

tsekiguchi
Copy link

@tsekiguchi tsekiguchi commented Nov 17, 2024

Problem

In the current version of Sentry, they have changed their Integration type. In the current state, when trying to add the Posthog Integration to Sentry it's throwing an error:

Type 'PostHogSentryIntegration' is not assignable to type 'Integration'.
  Types of property 'setupOnce' are incompatible.
    Type '(addGlobalEventProcessor: (callback: any) => void, getCurrentHub: () => any) => void' is not assignable to type '() => void'.
      Target signature provides too few arguments. Expected 2 or more, but got 0.ts(2322)

A couple other users on the PostHog integration page have also been running into this issue mentioned by Ariel and Vitor.

This is related to two breaking changes in Sentry's updated Integration:

  1. The setupOnce() method no longer passes any functions.
  2. getCurrentHub() is being deprecated

The PostHog sentry-integration now needs a different way to get the projectId and add events coming from Sentry, which are used in capture events in posthog.

Last thing I'll say is that this is my first open source contribution! So apologies if something is messy or not right :) Please let me know if there's anything I can improve!

Changes

Since the method setupOnce() no longer passes the current functions used for events and hub, we need to use setup() which passes the Sentry client() class. Using that class, we have access to addEventProcessor(event) function and getDsn().projectId that was previously being used. That satisfies the information needed by PostHog.

We also no longer need the specific _SentryIntegration interface.

I've also updated the tests to account for the new setup() class with the addEventProcessor() function. It's currently passing on my local machine.

Release info Sub-libraries affected

As noted in the integration, we don't import @sentry/types, but the latest ones were used to check everything in development.

Bump level

  • Major
  • Minor
  • Patch

Libraries affected

  • All of them
  • posthog-web
  • posthog-node
  • posthog-react-native

Changelog notes

  • Fixed the PostHog Sentry Integration to support the latest version of Sentry (v8)

@tsekiguchi tsekiguchi marked this pull request as draft November 17, 2024 01:28
@tsekiguchi tsekiguchi marked this pull request as ready for review November 17, 2024 01:28
@marandaneto
Copy link
Member

@tsekiguchi there was already a similar PR
something for @PostHog/team-error-tracking

@tsekiguchi
Copy link
Author

Ah I didn't see that! And I'm seeing that one has a way to differentiate for Sentry v7. I see that he submitted that PR about a month ago, is there any update on when it might be merged?

@daibhin
Copy link
Contributor

daibhin commented Nov 20, 2024

Hi @tsekiguchi, I managed to work on this today: #311. Just waiting on a review and then hopefully you will be able to use Sentry v8. We're also working on an error tracking product right now. Feel free to email me if you're interested in early access to that :)

@daibhin daibhin closed this Nov 20, 2024
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