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

Coil 3.0.0 R8 missing classes detected #2637

Closed
G00fY2 opened this issue Nov 5, 2024 · 17 comments
Closed

Coil 3.0.0 R8 missing classes detected #2637

G00fY2 opened this issue Nov 5, 2024 · 17 comments

Comments

@G00fY2
Copy link
Contributor

G00fY2 commented Nov 5, 2024

Describe the bug
After switching to Coil 3.0.0 the R8 task fails with the following error:

12:07:37  ERROR: Missing classes detected while running R8. Please add the missing classes or apply additional keep rules that are generated in /home/jenkins/agent/workspace/app_android-client_feature_coil3/app/build/outputs/mapping/release/missing_rules.txt.
12:07:37  ERROR: R8: Missing class coil3.PlatformContext (referenced from: coil3.network.ConnectivityChecker coil3.network.okhttp.OkHttpNetworkFetcher$OkHttpNetworkFetcherFactory$6.invoke(coil3.PlatformContext) and 1 other context)

The missing_rules.txt. contains -dontwarn coil3.PlatformContext. So maybe this should just be shipped as a consumer R8/ProGuard rule?

To Reproduce
Migrate to Coil 3.0, try to build minified Android app.

Version
Coil 3.0.0, AGP 8.7.2

@adhirajsinghchauhan
Copy link

adhirajsinghchauhan commented Nov 5, 2024

+1. Additionally, I also get this:

Caused by: [CIRCULAR REFERENCE: com.android.tools.r8.internal.g: Missing class coil3.PlatformContext (referenced from: coil3.network.ConnectivityChecker coil3.network.okhttp.OkHttpNetworkFetcher$OkHttpNetworkFetcherFactory$6.invoke(coil3.PlatformContext) and 1 other context)]

So I don't think -dontwarn would help. Deps: coil-compose, coil-network-okhttp, and coil-network-cache-control, all at 3.0.0.

Per the docs, my application class is like so:

@HiltAndroidApp
MyApplication: Application(), Configuration.Provider, SingletonImageLoader.Factory {
    …
    @OptIn(ExperimentalCoilApi::class)
    override fun newImageLoader(context: PlatformContext) = ImageLoader.Builder(context)
        .components {
            add(OkHttpNetworkFetcherFactory(cacheStrategy = { CacheControlCacheStrategy() }))
        }.build()
}

@G00fY2
Copy link
Contributor Author

G00fY2 commented Nov 5, 2024

@adhirajsinghchauhan You are right. After adding -dontwarn coil3.PlatformContext minified build task succeeded but images are not loaded at runtime. I see the following Coil error message printed by the logger (see below):

Failed - myimageurl.jpg - java.lang.NoClassDefFoundError: Failed resolution of: Lcoil3/PlatformContext;

Forgot to meantion that we also have this code in our Application class:

@HiltAndroidApp
class MyApp : Application(), SingletonImageLoader.Factory, Configuration.Provider {
    …
    @OptIn(ExperimentalCoilApi::class)
    override fun newImageLoader(context: PlatformContext): ImageLoader {
        return ImageLoader.Builder(this)
            .components {
                add(OkHttpNetworkFetcherFactory(cacheStrategy = { CacheControlCacheStrategy() }))
                add(SvgDecoder.Factory())
                add(VideoFrameDecoder.Factory())
            }
            .logger(
                object : Logger {
                    override var minLevel: Level = Level.Verbose

                    override fun log(
                        tag: String,
                        level: Level,
                        message: String?,
                        throwable: Throwable?
                    ) {
                        logVerbose("COIL") { message.toString() }
                    }
                }
            )
            .build()
    }

@colinrtwhite
Copy link
Member

colinrtwhite commented Nov 5, 2024

Hmm we had another report of R8 issues here, but I haven't been able to repro it locally. Missing coil3.PlatformContext is strange as it doesn't exist on Android - it's a typealias for android.content.Context.

Do you use any special R8 config or hardcoded R8 version? As a work-around you could try:

-keep class coil3.PlatformContext { *; }

@adhirajsinghchauhan
Copy link

I already tried the keep rule; doesn't help me. Build still fails.
image
(it does exist in the JAR though, and can be navigated to via Ctrl+N)

I'm using proguard-android-optimize.txt and a basic Crashlytics config:

-keepattributes SourceFile,LineNumberTable        # Keep file names and line numbers.
-keep public class * extends java.lang.Exception  # Optional: Keep custom exceptions.

Everything else is normal. AGP 8.7.2, compile & target SDK 35. If I turn off minification in a debug build, I get the same Coil error log message as above, and images don't load. Perplexing.

@colinrtwhite
Copy link
Member

colinrtwhite commented Nov 5, 2024

@adhirajsinghchauhan Inspecting the JAR indicates coil3.PlatformContextKt exists, but not coil3.PlatformContext. coil3.PlatformContextKt is the generated file for the PlatformContext source file and is empty (no methods/classes) in the Android JAR.

@JanMalch
Copy link

JanMalch commented Nov 6, 2024

I'm also having issues with the coil3.PlatformContext. In fact, images aren't loading in a regular debug build. With the DebugLogger I see the following:

🚨 Failed - https://example.com/redacted.jpeg - java.lang.NoClassDefFoundError: Failed resolution of: Lcoil3/PlatformContext;

The throwable for the log call is null. The app is not a KMP app in any way.

@lmiskovic
Copy link

lmiskovic commented Nov 6, 2024

Same issue, images are not loading in debug build and debug logger saying

Failed - https://images.unsplash.com/photo-1665686377065.... - java.lang.NoClassDefFoundError: Failed resolution of: Lcoil3/PlatformContext;

Removing the custom fetcher factory resolves the issue, without following block it loads images properly

.fetcherFactory(OkHttpNetworkFetcherFactory(cacheStrategy = {
            CacheControlCacheStrategy()
        })

@adhirajsinghchauhan
Copy link

adhirajsinghchauhan commented Nov 6, 2024

Removing the custom fetcher factory resolves the issue

Be wary though, as this is must be provided if your use-case requires respecting Cache-Control headers sent by your server.

I imagine anything that relies on PlatformContext under-the-hood will either not work at all, or have reduced functionality until this bug is fixed. I wonder if creating the coil3 package — with a copy of PlatformContext in our own codebases — would be a reasonable workaround? Haven't tested this idea yet.

@JanMalch
Copy link

JanMalch commented Nov 6, 2024

I wonder if creating the coil3 package — with a copy of PlatformContext in our own codebases — would be a reasonable workaround? Haven't tested this idea yet.

I tried this, but it didn't work.

@lmiskovic
Copy link

Removing the custom fetcher factory resolves the issue

Be wary though, as this is must be provided if your use-case requires respecting Cache-Control headers sent by your server.

I imagine anything that relies on PlatformContext under-the-hood will either not work at all, or have reduced functionality until this bug is fixed. I wonder if creating the coil3 package — with a copy of PlatformContext in our own codebases — would be a reasonable workaround? Haven't tested this idea yet.

Ofcourse, postponing migration to 3.0.0 for now as I require Cache-Control header support

@colinrtwhite
Copy link
Member

colinrtwhite commented Nov 6, 2024

Does anyone have a way to reproduce this issue locally? Referencing PlatformContext in the view sample app which is Android-only, it builds fine. Cash App is also Android-only and builds without issue. Are you using the latest stable Kotlin version?

@JanMalch
Copy link

JanMalch commented Nov 7, 2024

I've created a repository with detailed commits, for my issue with the debug build. Haven't tried a release build: https://github.com/JanMalch/coil3bug

As @lmiskovic mentioned, it only occurs after adding a cacheStrategy.
I found a fix by simply adding the connectivityChecker explicitly.

connectivityChecker = { ctx -> ConnectivityChecker(ctx) }

It seems that even an explicit connectivityChecker = ::ConnectivityChecker works.

@colinrtwhite
Copy link
Member

colinrtwhite commented Nov 7, 2024

@JanMalch Thanks for the repro project! I'm able to reproduce the error locally. It seems like there's some kind issue with the code generated for this function specifically:

fun OkHttpNetworkFetcherFactory(
    callFactory: () -> Call.Factory = ::OkHttpClient,
    cacheStrategy: () -> CacheStrategy = { CacheStrategy.DEFAULT },
    connectivityChecker: (PlatformContext) -> ConnectivityChecker = ::ConnectivityChecker,
)

Hopefully we can work around the generated code issue. For now folks can use your work around.

@G00fY2
Copy link
Contributor Author

G00fY2 commented Nov 7, 2024

I found a fix by simply adding the connectivityChecker explicitly.

This is working. But we still get the R8 error for release builds:

09:59:42  ERROR: Missing classes detected while running R8. Please add the missing classes or apply additional keep rules that are generated in /home/jenkins/agent/workspace/app_android-client_feature_coil3/app/build/outputs/mapping/release/missing_rules.txt.
09:59:42  ERROR: R8: Missing class coil3.PlatformContext (referenced from: coil3.network.ConnectivityChecker coil3.network.okhttp.OkHttpNetworkFetcher$OkHttpNetworkFetcherFactory$6.invoke(coil3.PlatformContext) and 1 other context)

When I build with -dontwarn coil3.PlatformContext the release build runs through and everything seems to work (images are loaded).

@colinrtwhite
Copy link
Member

Hi folks, I merged a fix here. It fixes the issue in @JanMalch's project and didn't produce any R8 errors. @G00fY2 please test out the latest 3.1.0-SNAPSHOT snapshot and let me know if it resolves the R8 issue.

@G00fY2
Copy link
Contributor Author

G00fY2 commented Nov 8, 2024

@colinrtwhite I switched to 3.1.0-SNAPSHOT and removed any custom proguard rules and the workaround from #2637 (comment). So we again only set

add(OkHttpNetworkFetcherFactory(cacheStrategy = { CacheControlCacheStrategy() }))

The release builds does not show any R8 errors anymore and images are loading (so no runtime errors). Looks like your changes fixed the issue! 🎊

@colinrtwhite
Copy link
Member

Awesome, thanks for checking! Will ship this in 3.0.2 in the next day or two.

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

No branches or pull requests

5 participants