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

3.3.0 is binary-incompatible with 3.2.0 #466

Closed
autonomousapps opened this issue Oct 2, 2020 · 7 comments
Closed

3.3.0 is binary-incompatible with 3.2.0 #466

autonomousapps opened this issue Oct 2, 2020 · 7 comments
Labels
bug Something isn't working

Comments

@autonomousapps
Copy link

✍️ Describe the bug

I have a project that uses Chucker, which also uses a library that uses Chucker. The library is still using 3.2.0, and when I updated my app to 3.3.0 and ran it, it crashed at runtime with this error:

java.lang.NoSuchMethodError: No direct method <init>(Landroid/content/Context;Lcom/chuckerteam/chucker/api/ChuckerCollector;JLjava/util/Set;ILkotlin/jvm/internal/DefaultConstructorMarker;)V in class Lcom/chuckerteam/chucker/api/ChuckerInterceptor; or its super classes (declaration of 'com.chuckerteam.chucker.api.ChuckerInterceptor' appears in /data/app/~~GJiF7AJl427rbyfaLOyo1A==/com.myproject-GIw2-OO7zIRGQFD6KsWDQg==/base.apk)

According to @cortinico,

Seems like the ChuckerInterceptor(Context, ChuckerCollector, Set) is missing.

💣 Steps to reproduce

  1. Have a library with Chucker 3.2.0.
  2. Have an app with Chucker 3.3.0, and which uses the above library.
  3. Launch app.
  4. Observe crash.

🔧 Expected behavior

App should not crash.

📄 Additional context

Adding the missing constructor will make it easier to migrate to Chucker 3.3.0.

@autonomousapps
Copy link
Author

Btw, I haven't formalized it yet, but it would be possible to use my plugin to get a snapshot of your ABI at any given moment, and you could use that to compare across versions. You'd run ./gradlew :lib:abiAnalysisDebug and then look at the output at build/reports/dependency-analysis/debug/intermediates/abi-dump.txt. I plan to make this easier in the future. If you do try this, let me know how it goes, as having a real use-case to discuss would make it easier to build out the feature fully.

@MiSikora
Copy link
Contributor

MiSikora commented Oct 3, 2020

I think some information is missing. These are constructors from 3.2.0 and what they compile down to.

class ChuckerInterceptor internal constructor(
private val context: Context,
private val collector: ChuckerCollector = ChuckerCollector(context),
private val maxContentLength: Long = 250000L,
private val fileFactory: FileFactory,
headersToRedact: Set<String> = emptySet()
) : Interceptor {
/**
* An OkHttp Interceptor which persists and displays HTTP activity
* in your application for later inspection.
*
* @param context An Android [Context]
* @param collector A [ChuckerCollector] to customize data retention
* @param maxContentLength The maximum length for request and response content
* before their truncation. Warning: setting this value too high may cause unexpected
* results.
* @param headersToRedact a [Set] of headers you want to redact. They will be replaced
* with a `**` in the Chucker UI.
*/
@JvmOverloads
constructor(
context: Context,
collector: ChuckerCollector = ChuckerCollector(context),
maxContentLength: Long = 250000L,
headersToRedact: Set<String> = emptySet()
) : this(context, collector, maxContentLength, AndroidCacheFileFactory(context), headersToRedact)

public final class com.chuckerteam.chucker.api.ChuckerInterceptor implements okhttp3.Interceptor {
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, java.util.Set<java.lang.String>);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, java.util.Set, int, kotlin.jvm.internal.DefaultConstructorMarker);

  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, com.chuckerteam.chucker.internal.support.FileFactory, java.util.Set<java.lang.String>);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, com.chuckerteam.chucker.internal.support.FileFactory, java.util.Set, int, kotlin.jvm.internal.DefaultConstructorMarker);
}

Same for 3.3.0.

public class ChuckerInterceptor internal constructor(
private val context: Context,
private val collector: ChuckerCollector = ChuckerCollector(context),
private val maxContentLength: Long = 250000L,
private val cacheDirectoryProvider: CacheDirectoryProvider,
private val alwaysReadResponseBody: Boolean = false,
headersToRedact: Set<String> = emptySet(),
) : Interceptor {
/**
* An OkHttp Interceptor which persists and displays HTTP activity
* in your application for later inspection.
*
* @param context An Android [Context]
* @param collector A [ChuckerCollector] to customize data retention
* @param maxContentLength The maximum length for request and response content
* before their truncation. Warning: setting this value too high may cause unexpected
* results.
* @param alwaysReadResponseBody If set to `true` Chucker will read full content of response
* bodies even in case of parsing errors or closing the response body without reading it.
* @param headersToRedact a [Set] of headers you want to redact. They will be replaced
* with a `**` in the Chucker UI.
*/
@JvmOverloads
public constructor(
context: Context,
collector: ChuckerCollector = ChuckerCollector(context),
maxContentLength: Long = 250000L,
headersToRedact: Set<String> = emptySet(),
alwaysReadResponseBody: Boolean = false,
) : this(
context = context,
collector = collector,
maxContentLength = maxContentLength,
cacheDirectoryProvider = { context.cacheDir },
alwaysReadResponseBody = alwaysReadResponseBody,
headersToRedact = headersToRedact,
)

public final class com.chuckerteam.chucker.api.ChuckerInterceptor implements okhttp3.Interceptor {
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, java.util.Set<java.lang.String>);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, java.util.Set<java.lang.String>, boolean);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, java.util.Set, boolean, int, kotlin.jvm.internal.DefaultConstructorMarker);

  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, com.chuckerteam.chucker.internal.support.CacheDirectoryProvider, boolean, java.util.Set<java.lang.String>);
  public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, com.chuckerteam.chucker.internal.support.CacheDirectoryProvider, boolean, java.util.Set, int, kotlin.jvm.internal.DefaultConstructorMarker);
}

It doesn't look to me like there ever was ChuckerInterceptor(Context, ChuckerCollector, Set) constructor. Two bottom constructors are irrelevant as they are internal. The difference is this.

+ public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, java.util.Set<java.lang.String>, boolean);
- public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, java.util.Set, int, kotlin.jvm.internal.DefaultConstructorMarker);
+ public com.chuckerteam.chucker.api.ChuckerInterceptor(android.content.Context, com.chuckerteam.chucker.api.ChuckerCollector, long, java.util.Set, boolean, int, kotlin.jvm.internal.DefaultConstructorMarker);

There is one additional constructor which is not a problem. The last one is problematic as it adds an additional boolean parameter and Kotlin code calls it by default but it does not reflect the reported issue as there is no such a constructor. I also don't think that it is that big of a deal due to the nature of the library (though it is an issue). Nevertheless, we can solve i.e. with my suggestion in #462. It won't help in itself 3.x but will allow us to avoid this problem in 4.x.

@autonomousapps Is the library that's causing issues public? Can you maybe link it? Because I'm curious about the signature it reports.

@MiSikora MiSikora added the bug Something isn't working label Oct 3, 2020
@MiSikora
Copy link
Contributor

MiSikora commented Oct 3, 2020

Oh, and the cause is obvious. It's #443. But this shows that we need another mechanism for creating the interceptor if we want to have an extensible API.

@vbuberen
Copy link
Collaborator

vbuberen commented Oct 3, 2020

Thanks for reporting @autonomousapps

This case also shows that we should add more tools to do automatic analysis for each PR to avoid such unintended consequences. I had some in my drafts for other purposes, like size changes, etc., but compatibility is definitely something to add and put up in the list of priorities.

Also thanks for suggesting your plugin - I tried it just a month ago on another project and it worked well.

The case you described is quite specific and we will try to safe-guard from such cases with tools in future as well as implement the suggestion from @MiSikora.

More details to reproduce the case will be helpful.

For now I would suggest to update that other library you mentioned to use 3.3.0 as well.

@cortinico
Copy link
Member

There is one additional constructor which is not a problem. The last one is problematic as it adds an additional boolean parameter and Kotlin code calls it by default but it does not reflect the reported issue as there is no such a constructor. I also don't think that it is that big of a deal due to the nature of the library (though it is an issue). Nevertheless, we can solve i.e. with my suggestion in #462. It won't help in itself 3.x but will allow us to avoid this problem in 4.x.

@autonomousapps Is the library that's causing issues public? Can you maybe link it? Because I'm curious about the signature it reports.

Thanks for investigating @MiSikora. I've already did a quick run with japicmp and the result is the same:

Screenshot 2020-10-03 at 22 00 05

Seems like we never had a ChuckerInterceptor(Context, ChuckerCollector, Set)

As it was already suggested in #462, we should definitely get rid of @JvmOverloads on the constructor as it makes hard to track such changes.

This case also shows that we should add more tools to do automatic analysis for each PR to avoid such unintended consequences.

This would be a great plus. As a rule of thumb, we can just make sure to run japicmp (or whatever tools) just before releasing a new version

@autonomousapps
Copy link
Author

tl;dr both the library and the consuming app are private, but I will try to create a reproducer.

For more context, my team has two main repos. One is an SDK that we use for communicating with our API, among other things. We both use this internally in our own apps and make it available to partners for their own integrations. The source is obfuscated. The app that I primarily work on is the one that triggered the error that led to this ticket. It has a couple of independent APIs that aren't part of the SDK. Therefore both the SDK and app use Chucker, for better debug logging. When the consuming app bumped to 3.3.0, the SDK still had 3.2.0. The app tried to instantiate something that created a ChuckerInterceptor instance and 💥

It may also relate to how the app uses the SDK. The SDK provides its own dagger modules, and these tend to be written in Kotlin. The app also has dagger, of course, and incorporates these modules into its graph. The app's modules tend to be written in Java, if that matters.

@vbuberen
Copy link
Collaborator

Looks like we can close this one thanks to #509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants