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

Auto-Installation version mismatch causes runtime crash #329

Open
PaulWoitaschek opened this issue Jun 11, 2022 · 15 comments
Open

Auto-Installation version mismatch causes runtime crash #329

PaulWoitaschek opened this issue Jun 11, 2022 · 15 comments
Labels
bug Something isn't working Platform: Android

Comments

@PaulWoitaschek
Copy link

Integration

sentry-android

Build System

Gradle

AGP Version

7.2.1

Proguard

Enabled

Version

6.0.0, plugin=3.1.0

Steps to Reproduce

Build an obfuscated release build and call:

fun initSentry(application: Application) {
  val options = Sentry.OptionsConfiguration<SentryAndroidOptions> { options ->
    options.apply {
      isEnableNdk = false // <- this fixes the crash
      ...
    }
  }
  SentryAndroid.init(application, NoOpLogger.getInstance(), options)
}

Expected Result

It doesn't crash

Actual Result

Follow up of the discussion in getsentry/sentry-java#2031

Calling options.isEnableNdk = false causes the crash to not appear.

Stacktrace

signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr --------
Abort message: 'Throwing new exception 'no non-static method "Lio/sentry/android/core/SentryAndroidOptions;.getDsn()Ljava/lang/String;"' with unexpected pending exception: java.lang.NoSuchMethodError: no non-static method "Lio/sentry/android/core/SentryAndroidOptions;.getOutboxPath()Ljava/lang/String;"
  at void io.sentry.android.ndk.SentryNdk.initSentryNative(io.sentry.android.core.SentryAndroidOptions) (SourceFile:-2)
  at void io.sentry.android.ndk.SentryNdk.init(io.sentry.android.core.SentryAndroidOptions) (SourceFile:7)
  at java.lang.Object java.lang.reflect.Method.invoke(java.lang.Object, java.lang.Object[]) (Method.java:-2)
  at void io.sentry.android.core.l0.register(kp.r, io.sentry.SentryOptions) (SourceFile:106)
  at void io.sentry.u.l(io.sentry.SentryOptions, boolean) (SourceFile:98)
  at void io.sentry.u.m(kp.r0, io.sentry.u$a, boolean) (SourceFile:9)
  at void io.sentry.android.core.s0.e(android.content.Context, kp.s, io.sentry.u$a) (SourceFile:26)
  at void rv.c.c(android.app.Application) (SourceFile:26)
  at void yazio.App.onCreate() (SourceFile:9)
  at void android.app.Instrumentation.callApplicationOnCreate(android.app.Application) (Instrumentation.java:1223)
  at void android.app.ActivityThread.handleBindApplication(android.app.ActivityThread$AppBindData) (ActivityThread.java:6734)
  at void android.app.ActivityThread.access$1500(android.app.ActivityThread, android.app.ActivityThread$AppBindData) (ActivityThread.java:256)
  at void android.app.ActivityThread$H.handleMessage(android.os.Message) (ActivityThread.java:2090)
  at void android.os.Handler.dispatchMessage(android.os.Message) (Handler.java:106)
  at boolean android.os.Looper.loopOnce(android.os.Looper, long, int) (Looper.java:201)
  at void android.os.Looper.loop() (Looper.java:288)
  at void android.app.ActivityThread.main(java.lang.String[]) (ActivityThread.java:7842)
  at java.lang.Object java.lang.reflect.Method.invoke(java.lang.Object, java.lang.Object[]) (Method.java:-2)
  at void com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run() (RuntimeInit.java:548)
  at void com.android.internal.os.ZygoteInit.main(java.lang.String[]) (ZygoteInit.java:1003)
@marandaneto
Copy link
Contributor

@PaulWoitaschek are you using proguard or r8? are you using any special configuration in your proguard config?

Can you add to your app's proguard file:

// You can specify any path and filename.
-printconfiguration ~/tmp/full-r8-config.txt

And check if the rule to keep SentryAndroidOptions is there?
https://github.com/getsentry/sentry-java/blob/e4a46aa6dfe09aa185f1aaf6bd31235c12b520db/sentry-android-ndk/proguard-rules.pro#L5-L7

Thanks.

@PaulWoitaschek
Copy link
Author

It does not exist.

But wait; we're not using sentry-android-ndk. It seems the plugin adds that, where we add the 6.0.0 core dependency.

Maybe this is related to the plugin adding an outdated dependency?

@PaulWoitaschek
Copy link
Author

PaulWoitaschek commented Jun 12, 2022

Yes, confirmed. Setting the version in the autoInstallation block fixes it.

I think this is a dependency issue:
In a gradle module we have a dependency on sentry-core using 6.0.0. Now the plugins in the app module adds 5.7.0.

In the final dependeny graph this leads to inconsistent dependencies:

+--- io.sentry:sentry-android:5.7.0
|    +--- io.sentry:sentry-android-core:5.7.0 -> 6.0.0
|    |    \--- io.sentry:sentry:6.0.0
|    \--- io.sentry:sentry-android-ndk:5.7.0
|         +--- io.sentry:sentry:5.7.0 -> 6.0.0
|         \--- io.sentry:sentry-android-core:5.7.0 -> 6.0.0 (*)

Disabling the auto installation also works.

From your side this coulde be mitigated by publishing a bom.
The approach could look like this: You publish a bom. The plugin applies the bom. The plugin uses now either uses the plugin defined version for the bom or (with precedence) the user supplied sentry version and applies the bom using that version.

@romtsn
Copy link
Member

romtsn commented Jun 12, 2022

We do publish a bom already -> https://docs.sentry.io/platforms/android/configuration/bill-of-materials/. I'm not sure the bom would help here though, because if we apply the bom with the version 5.7.0, this still would be a problem for your case, because you specify explicitly the -core version of 6.0.0, so the ndk would still be applied with 5.7.0, wouldn't it?

Ideally we'd look at your dependency tree and use the version that you've specified in the module, but that's unfortunately not possible without resolving the configuration, and this can be quite heavy for the build time (we have an improvement to at least warn you about that though (#324).

There are also other multiple improvements to this:

  1. Bump the sentry SDK version in the plugin automatically and release a new version of the plugin together with the runtime sdk (Automatically bump sentry runtime SDK version with a new release #322)
  2. Auto-installing NDK can be optional in the plugin (so only the -core package is installed by default)

@PaulWoitaschek
Copy link
Author

From my understanding, an applied enforcedPlatform would do it.

Besides that, moving the plugin and the java library to the same repository and releasing them with the same version would mitigate these errors (where plugin and lib are out of sync) as well.

@romtsn
Copy link
Member

romtsn commented Jun 12, 2022

Ah, I wasn't aware of enforcedPlatform, yeah it seems like that would do.

We actually wanted to be conservative here, and do not override any of the user-defined dependencies, but maybe it'd be better to have them enforced (e.g. by using a bom and giving user an option to specify a version) @bruno-garcia @marandaneto wdyt?

@bruno-garcia
Copy link
Member

bruno-garcia commented Jun 12, 2022

Ah, I wasn't aware of enforcedPlatform, yeah it seems like that would do.

We actually wanted to be conservative here, and do not override any of the user-defined dependencies, but maybe it'd be better to have them enforced (e.g. by using a bom and giving user an option to specify a version) @bruno-garcia @marandaneto wdyt?

Problem of us overriding the version is that user can't override our behavior it turns out to be wrong. Could we log a build warning instead?
Or have a separate option to disable our overriding behavior if we add it?

@PaulWoitaschek
Copy link
Author

In general, the approach to let users overwrite versions is good.

But in the case that you use auto-install, it is error prone and leads to inconsistent versions (which has caused the crash I'm reporting). And you can explicitly set the version in the auto install extension already so I expect this to solve more issues than it causes.

@romtsn
Copy link
Member

romtsn commented Jun 12, 2022

Problem of us overriding the version is that user can't override our behavior it turns out to be wrong. Could we log a build warning instead?

It's kind of possible through autoInstallation.sentryVersion config, but it will set a single version for all integrations. The only problem I see with this, is if they want to keep an older version of, say, -timber, while updating -core to 6.0.0. But this still would be possible to enforce via gradle's dependency resolution.

Or have a separate option to disable our overriding behavior if we add it?

Having an option for that is also possible, yep.

@marandaneto
Copy link
Contributor

Ah, I wasn't aware of enforcedPlatform, yeah it seems like that would do.

We actually wanted to be conservative here, and do not override any of the user-defined dependencies, but maybe it'd be better to have them enforced (e.g. by using a bom and giving user an option to specify a version) @bruno-garcia @marandaneto wdyt?

Yep, we should not override the user-defined dependencies to not force the need of upgrading the Sentry SDK nor the need of removing the Sentry Gradle plugin due to not being compatible.

@marandaneto
Copy link
Contributor

Problem of us overriding the version is that user can't override our behavior it turns out to be wrong. Could we log a build warning instead?

It's kind of possible through autoInstallation.sentryVersion config, but it will set a single version for all integrations. The only problem I see with this, is if they want to keep an older version of, say, -timber, while updating -core to 6.0.0. But this still would be possible to enforce via gradle's dependency resolution.

Or have a separate option to disable our overriding behavior if we add it?

Having an option for that is also possible, yep.

All Sentry packages should be the very same version, we should not allow the mix of e.g. sentry-android-core 5.7 and sentry-android-timber 6.0, packages are published together with the very same version to avoid incompatibilities.

@marandaneto
Copy link
Contributor

If the user defined 5.7.0 in its dependencies block (it doesn't matter if its the core package or not, versions should be aligned), we should keep using 5.7.0, if this is technically not possible or too expensive, I'd say we should turn off the auto-install for this use case.
If the user defined the sentryVersion, then we respect that.

@marandaneto marandaneto added bug Something isn't working Status: Backlog labels Jun 14, 2022
@romtsn romtsn transferred this issue from getsentry/sentry-java Jun 14, 2022
@romtsn
Copy link
Member

romtsn commented Jun 14, 2022

Moved this to the SAGP repo, as it rather belongs here.

@romtsn romtsn changed the title SentryAndroid#init crashes Auto-Installation version mismatch causes runtime crash Jun 14, 2022
@romtsn
Copy link
Member

romtsn commented Jun 15, 2022

@PaulWoitaschek we've released 3.1.1 which bumps the runtime sdk version to 6.1.0 , so this problem should be gone, but we'll look into a more future-proof solution. Thanks for your ideas and the bug report!

@romtsn
Copy link
Member

romtsn commented Feb 22, 2023

We could have a gradle task which looks over the dependency tree and see if there are mismatched sentry packages' versions, fail the build or show a warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Platform: Android
Projects
Status: Backlog
Development

No branches or pull requests

5 participants