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

Ensure replays do not have filtered out errors attached #6285

Closed
Tracked by #5326
mydea opened this issue Nov 24, 2022 · 2 comments · Fixed by #6299
Closed
Tracked by #5326

Ensure replays do not have filtered out errors attached #6285

mydea opened this issue Nov 24, 2022 · 2 comments · Fixed by #6299
Assignees
Labels
Package: replay Issues related to the Sentry Replay SDK Type: Bug

Comments

@mydea
Copy link
Member

mydea commented Nov 24, 2022

Replays attach errors that happened while the replay was running via errorIds.
This is basically a global event processor that adds the ID of each error event it gets to the errorIds set, which in turn ends up being sent to Sentry.

However, turns out that in reality sometimes this includes IDs of error events that do not actually exist on Sentry, leading to weird behavior.

After some investigation, the most probable cause of this is that some of the error events are sampled away after they have been "consumed" by the replay event processor. Some reasons that could lead to this are:

  • The deduplication integration removes them
  • They are dropped due to rate limiting
  • They are dropped by the host app via a custom beforeSend hook or another global event processor
  • They are dropped when being sent over the network (e.g. failed HTTP request, ...)

In order to fix (or at least, mostly fix), all but the last of these issues, we should introduce some logic that removes eventIds that have been dropped.

Alternatives that have been investigated

Some other ideas we thought about, but ultimately discarded:

  • Introduce an afterSend hook: This may not work, as an error could be send at the same time or after a replay was sent.
  • Introduce some custom hooks for this that are always called: Tricky and expands public API surface noticeably.
  • Monkey patch beforeSend: Can be done, but potentially tricky, as this is/can be user configured.

Proposed solution

We should monkey patch recordDroppedEvent on the client. This is called when an event is dropped either because of returning null from beforeSend / a global event processor, or when dropped due to rate limiting.

@mydea mydea self-assigned this Nov 24, 2022
@AbhiPrasad
Copy link
Member

This is something we can possibly solve with getsentry/rfcs#34

@mydea
Copy link
Member Author

mydea commented Nov 24, 2022

This is something we can possibly solve with getsentry/rfcs#34

yeah, def, we also think so. However, as fixing this issue has p0 priority for replay, we'll probably do some "easy" (internal) fix for now, and can then later migrate it to a proper hooks solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: replay Issues related to the Sentry Replay SDK Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants