sentry-cocoa icon indicating copy to clipboard operation
sentry-cocoa copied to clipboard

Update sessions after running beforeSend

Open serglom21 opened this issue 1 year ago • 6 comments

Problem Statement

Sentry reports some crashes like SIGPIPE which cause the application to terminate if they are not handled properly. I don't want those crashes to affect my Crash Free Session Rate but I still want to get a signal of how often these affect the user base. In this case, I would like to explore different options other than filtering them out since I still want to see them in Sentry.

I tried modifying the mechanism.handled property of the crash within beforeSend but that did not seem to have any effect on CFSR (the SIGPIPE crash still negatively affected CFSR even though the mechanism handled was true)

options.beforeSend = { event in
      let exception = event.exceptions?.first
      if event.level == .fatal {
           exception?.mechanism?.handled = true
       }

       return event
}

Solution Brainstorm

A way to ignore certain crashes from marking a session as crashed. I'm not sure if beforeSend is the best place to allow this or if it is even possible.

Maybe an option in the configuration to specify exceptions that should not mark a session as crashed

Are you willing to submit a PR?

No response

serglom21 avatar Jul 19 '24 18:07 serglom21

Thanks @serglom21 for reaching out. We will discuss whether this is viable.

brustolin avatar Jul 22 '24 06:07 brustolin

Following. Thanks @serglom21! SIGPIPE crashes became visible after switching to Sentry. They're not visible in Apple monitored crashes in Xcode or Crashlytics. It appears SIGPIPE are applicable to just Sentry.

https://github.com/getsentry/sentry-cocoa/blob/f3b9a21dbf31620e32657c001d902d52efe5a4ec/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_MachException.c#L240

We've noticed the SIGPIPE errors we're seeing are all occurring after the app is moved to the background and likely related to Firebase analytics trying to communicate after the app is in the background. Since they're not user impacting we want to mark these as handled such that they don't affect our crash free rate.

I'm happy to submit a PR if you can point me in the right direction. Thanks!

jay-at-youversion avatar Jul 30 '24 20:07 jay-at-youversion

The problem is that beforeSend for an event runs after we modify the session state. We would need a different hook. Let me double-check if we have something similar for other Sentry SDKs. If we do, we can add it also to the Cocoa SDK.

philipphofmann avatar Jul 31 '24 14:07 philipphofmann

The Java SDK has the session update logic in the client which runs after beforeSend. I think we should move the session logic from the hub to the client and also run it after beforeSend, but this is a breaking change.

philipphofmann avatar Aug 02 '24 11:08 philipphofmann

Hey, one question on this: Does sampling affect the CFSR? So 0.1 has a different outcome than 1.0? or should that also be ignored

Angelodaniel avatar Oct 17 '24 08:10 Angelodaniel

@Angelodaniel, if CFSR is the crash-free session rate, then no. Sampling has no impact on it.

philipphofmann avatar Oct 17 '24 11:10 philipphofmann