getsentry / sentry-kotlin-multiplatform

Sentry SDK for Kotlin Multiplatform
MIT License
137 stars 19 forks source link

Test Apple targets don't find Sentry framework when using SPM #176

Closed thomasflad closed 9 months ago

thomasflad commented 10 months ago

Platform

Apple, Android

Installed

Swift Package Manager

Version

0.4.0

Steps to Reproduce

  1. Add Sentry KMP dependency to commonMain
  2. Add a simple test to commenTest
  3. Run linkDebugTestIosSimulatorArm64 Gradle task or run test on iOS simulator target

See fork from my co-worker: https://github.com/PaulWoitaschek/sentry-kotlin-multiplatform/commit/4e856fc11414ad2a1ee44f40d4e189b7b639698e

Expected Result

Test also runs on Apple targets as expected

Actual Result

Gradle task linkDebugTestIosSimulatorArm64 fails with error message: ld: framework 'Sentry' not found

buenaflor commented 10 months ago

Hi, I'm not 100% how to tackle this since SPM support on KMP is generally not well documented / supported currently but it is on the roadmap: https://youtrack.jetbrains.com/issue/KT-64571

I'll take a look regardless

thomasflad commented 10 months ago

Hi @buenaflor, thanks for taking a look. My title is a little bit misleading. We besically generate a regular xcframework and use it on iOS/watchOS in combination with the Sentry SPM library.

PaulWoitaschek commented 9 months ago

Hey @buenaflor !

Were you able to already take a look at this issue?

It would be great if it was documented https://docs.sentry.io/platforms/kotlin-multiplatform/#swift-package-manager-spm that if you're using that option it's no longer possible to run tests.

msasikanth commented 9 months ago

@thomasflad were you able to find a workaround or solution for the issue, I am getting a similar issue on my project

https://github.com/msasikanth/twine/pull/196

thomasflad commented 9 months ago

@msasikanth No, not yet. Since we already have a working solution by using the native libraries on each platform separately we've investigated not further and blocking the integration of the KMP version until there is a fix from Sentry

PaulWoitaschek commented 9 months ago

The main thing we're trying to solve here is to get proper stack traces in sentry for iOS crashes. Am I correct in the assumption that this is not possible without sentry kmp? Or is there a way to set the stack trace addresses by implementing a callback from swift and keep using the native sdks?

buenaflor commented 9 months ago

Sorry no update here yet.

sentry for iOS crashes. Am I correct in the assumption that this is not possible without sentry kmp

If you only care about iOS crashes then you can also use https://github.com/rickclephas/NSExceptionKt which also works well. But then you'll still have the same problems where you can't use SPM in combination with tests

Or is there a way to set the stack trace addresses by implementing a callback from swift and keep using the native sdks?

Unfortunately not yet. https://github.com/getsentry/sentry-cocoa/issues/1999

buenaflor commented 9 months ago

In addition this is not a Sentry specific problem but I assume a problem with SPM dependency usage in KMP in general (at least specific to libraries that have ingested other cocoa dependencies)

buenaflor commented 9 months ago

Just a hunch but I imagine it is connected with this issue: https://github.com/getsentry/sentry-kotlin-multiplatform/issues/108 (linker stuff)

PaulWoitaschek commented 9 months ago

And would it be possible to do this manually until we have official sentry support?

Crashkios has solved this by basically telling the linker to treat the listed symbols as undefined. That means the linker will not try to find these symbols during the linking process and assumes these symbols will be provided by other means.

If yes, could you give a hint how?

@rickclephas is deprecating the sentry integration and point to using this SDK. We would really benefit from proper stacktraces at this point 🙏 https://github.com/rickclephas/NSExceptionKt/pull/24/files

rickclephas commented 9 months ago

Crashkios has solved this by basically telling the linker to treat the listed symbols as undefined. That means the linker will not try to find these symbols during the linking process and assumes these symbols will be provided by other means.

I would like to mention that tricking the linker like that results in issues when you distribute your app with app thinning enabled.

Besides that, last I heard, the CrashKiOS approach hasn't been working for tests in recent Kotlin versions either.

@rickclephas is deprecating the sentry integration and point to using this SDK.

Yeah, like @buenaflor already mentioned, NSExceptionKt currently suffers from the same issue. I have been working on a new API that will decouple the implementations from the Kotlin code. The Sentry implementation requires a lot of private APIs, and is no longer needed since there is this official SDK, so I indeed decided to deprecate it.

PaulWoitaschek commented 9 months ago

Hm and what about making the sentry iOS stuff public so that we don't need reflection? Then we could just have something in kotlin exposing the addresses and map them in iOS?

rickclephas commented 9 months ago

Then we could just have something in kotlin exposing the addresses and map them in iOS?

Yeah correct. That is more or less how the new API for NSExceptionKt works.

I created https://github.com/getsentry/sentry-cocoa/issues/1999 a while ago to request some public APIs. What is currently missing is a simple entry point into the native SDK. Which, in theory, could even be private. The current approach is just way too dependent on internal Sentry logic.

PaulWoitaschek commented 9 months ago

That linked proposal makes very much sense to me. @buenaflor could you prioritize this? This would be really great to have 🙏

PaulWoitaschek commented 9 months ago

Turns out someone raised this in the past too 😂 https://github.com/getsentry/sentry-cocoa/issues/1623

buenaflor commented 9 months ago

could you prioritize this

The cocoa team prio'ed this as p4 and their backlog is pretty full so tbh not sure if I can give more hope here that it will be implemented anytime soon.

@thomasflad @PaulWoitaschek here's a workaround for working tests although a bit annoying

  1. Choose a release and download the Sentry.xcframework.zip and unzip somewhere
  2. Add the linker opts -F/your/unzipped/path/Carthage/Build/Sentry.xcframework/ios-arm64_x86_64-simulator/
  3. Create a Frameworks folder in the folder where the test.exe resides and insert the Sentry.framework folder (this folder can be found inside of ios-arm64_x86_64-simulator): Screenshot 2024-02-07 at 01 52 50

Here the short code snippet for build.gradle.kts that worked for me

    listOf(
        iosX64(),
        iosArm64(),
        iosSimulatorArm64(),
    ).forEach {
        it.binaries.framework {
            baseName = "shared"
            isStatic = true
        }
        it.compilations.all {
            if (compilationName == "test" && target.platformType == KotlinPlatformType.native) {
                compilerOptions.configure {
                    freeCompilerArgs.add("-linker-options")
                    freeCompilerArgs.add("-F/Users/giancarlobuenaflor/Downloads/Carthage/Build/Sentry.xcframework/ios-arm64_x86_64-simulator/")
                }
            }
        }
    }

Now it should run

You can prolly make the config a bit more cleaner, optimized but that's the gist of it

lmk if that works

thomasflad commented 9 months ago

Hi @buenaflor,

Thanks for the workaround. Unfortunately, this is not a satisfactory solution, as we no longer notice when the library is updated. I think it would be a good idea to mention this issue in the README because you can't use the library like that in an enterprise context.

buenaflor commented 9 months ago

Got it, yea, will definitely add it to our docs. If that doesn't work in your use case we'd prolly have to wait until the kotlin folks deliver a proper swiftpm solution similar to their cocoapods plugin.

The other option would be to create a gradle plugin and add this workaround automatically

buenaflor commented 9 months ago

I added the docs, I'll close this issue for now but feel free to reopen for any more questions

ataulm commented 8 months ago

A colleague shared this, a template for publishing libraries to SPM and CocoaPods: https://touchlab.co/kmmbridge-quick-start

You can publish to either or both with the same project, and the initial setup for both is very similar, but CocoaPods has several extra steps. If you want to publish to CocoaPods, make sure to do both.

buenaflor commented 8 months ago

@ataulm unfortunately this doesn't solve our problem, they're actually quite unrelated

We have few solutions to this problem and are currently discussing them

buenaflor commented 6 months ago

Just wanted to give a quick update that we are working on a gradle plugin that helps solve this issue

abrown252 commented 4 months ago

Just wanted to give a quick update that we are working on a gradle plugin that helps solve this issue

Hello 👋

Do you have an update on the gradle plugin? I've just run into this issue while upgrading. Has anything changed in more recent versions? We were integrating via SPM fine on 0.2.0, updating to 0.7.1 appears to have triggered it

buenaflor commented 4 months ago

@abrown252 we're on it, it's working in a local build but still need to smoothen things out.

Meanwhile you can try this out https://docs.sentry.io/platforms/kotlin-multiplatform/troubleshooting/#tests-not-working

We were integrating via SPM fine on 0.2.0, updating to 0.7.1 appears to have triggered it

The reason it changed is because we upgraded our kotlin version to 1.9.21 which now runs the tests on ios etc dynamically and not statically.

Now that it's dynamic it needs linker configuration which our gradle plugin should help automate.

buenaflor commented 3 months ago

@abrown252 we've released a pre-release of the plugin: https://github.com/getsentry/sentry-kotlin-multiplatform/releases/tag/0.8.0-beta.1

you can try it out like so:

plugins {
  // version is synced with library version
  id("io.sentry.kotlin.multiplatform.gradle") version "0.8.0-beta.1" 
}

sentryKmp {
  // configure it if needed
}

it's still a preview so we also don't have extensive documentation yet.

Feel free to ask questions if something doesn't work!