getsentry / sentry-cocoa

The official Sentry SDK for iOS, tvOS, macOS, watchOS.
https://sentry.io/for/cocoa/
MIT License
807 stars 318 forks source link

Crash caused by inclusion of `beforeSend` option. #4509

Open UniekLee opened 1 day ago

UniekLee commented 1 day ago

Platform

iOS

Environment

TestFlight, Production, Develop

Installed

Swift Package Manager

Version

8.36.0

Xcode Version

16.1

Did it work on previous versions?

No

Steps to Reproduce

  1. Include a beforeSend block in SentrySDK initialisation.
  2. Invoke a crash in the application.
  3. Attempt to relaunch the application

It's worth calling out that this crash started appearing with Xcode 16.0 & Xcode 16.1 builds. Builds with Xcode 15 haven't shown this issue. I've also tried updating Sentry to v8.39.0, but this doesn't resolve the issue.

Sample project for reproduction attached. This crash occurs intermittently in the supplied sample project, but ever time in our main project.

BeforeSendCrash.zip

Expected Result

Application relaunches as usual and report is sent to Sentry.

Actual Result

Application crashes on first attempted relaunch. Application launches successfully on second attempt, and sends two crash reports to Sentry - the original crash and the SentrySDK crash.

Image Image

https://github.com/user-attachments/assets/465e3f5f-065b-4566-b626-cceda34cfbd4

Are you willing to submit a PR?

No

kahest commented 1 day ago

@UniekLee thank you for the detailed report and the repro, we'll investigate this shortly.

Internal notes:

kahest commented 12 hours ago

@UniekLee I did a few runs of your sample app but didn't get the crash - you mention it occurs "intermittently", do you have a rough number?

brustolin commented 12 hours ago

Hello @UniekLee, does your final project also use Swift 6 like the sample you sent?

brustolin commented 10 hours ago

I could reliably reproduce this error only by capturing a message in a different thread. This only happens in Swift 6 mode because of all the extra concurrency checks.

The solution is simple:

options.beforeSend = { @Sendable event in //This closure needs to be marked as @Sensable
    // Process the event
    return event
}

Or you can use an external function

    nonisolated func beforeSend(_ event: Event) -> Event? {
        return event
    }
//...

    options.beforeSend = self.beforeSend

I believe the compiler in Swift 6 mode should throw an error because this closure is not isolated. I will open an issue in the Swift repo to try to get more answers. In the meantime, I don’t think there’s anything we can do on the SDK side.

UniekLee commented 8 hours ago

Hey folks 👋 Thanks for jumping onto this so quickly. I'll answer the questions, although I'm not sure the info is still useful.

@UniekLee I did a few runs of your sample app but didn't get the crash - you mention it occurs "intermittently", do you have a rough number?

I'd say that it was periodic - at time it would crash 0% of the time, and sometimes it would cash >60% of the time. I couldn't figure out what made it more "crash prone".

Hello @UniekLee, does your final project also use Swift 6 like the sample you sent?

Uhm, I want to say no because we've set the project to Swift 5. But we've seen a lot of Swift 6-style behaviour, so not quite sure whether something strange is happening.

The solution is simple

Marking that closure as @Sendable seems to work - thanks so much for the fix 🙏

brustolin commented 8 hours ago

hm, I want to say no because we've set the project to Swift 5. But we've seen a lot of Swift 6-style behaviour, so not quite sure whether something strange is happening.

Interesting, because I could not make the error happen in Swift 5

Marking that closure as @Sendable seems to work

That’s great! Please let us know if it occurs again.

brustolin commented 7 hours ago

We need to update the documentation (snippets, troubleshooting) to reflect this.