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

Crash caused by inclusion of beforeSend option. #4509

Closed
UniekLee opened this issue Nov 5, 2024 · 7 comments · Fixed by #4512
Closed

Crash caused by inclusion of beforeSend option. #4509

UniekLee opened this issue Nov 5, 2024 · 7 comments · Fixed by #4512

Comments

@UniekLee
Copy link

UniekLee commented Nov 5, 2024

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

SentrySDK-beforeSend-crash.mp4

Are you willing to submit a PR?

No

@kahest
Copy link
Member

kahest commented Nov 5, 2024

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

Internal notes:

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Nov 5, 2024
@kahest kahest moved this from Needs Discussion to Needs Investigation in Mobile & Cross Platform SDK Nov 5, 2024
@kahest
Copy link
Member

kahest commented Nov 6, 2024

@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
Copy link
Contributor

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

@brustolin
Copy link
Contributor

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
Copy link
Author

UniekLee commented Nov 6, 2024

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 🙏

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 6, 2024
@brustolin
Copy link
Contributor

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.

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Nov 6, 2024
@brustolin brustolin moved this from Needs Investigation to Backlog in Mobile & Cross Platform SDK Nov 6, 2024
@brustolin brustolin moved this from Backlog to Todo in Mobile & Cross Platform SDK Nov 6, 2024
@brustolin
Copy link
Contributor

brustolin commented Nov 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants