Is it required to always annotate a method of a retrofit Service class?

[halizton]

Lately I ran into a problem with retrofit + r8_full_mode combination, although I am using the proguard rules file offered by retrofit:

https://github.com/square/retrofit/blob/trunk/retrofit/src/main/resources/META-INF/proguard/retrofit2.pro

There is one class still caused a run time exception, here's the pseudocode:

interface ServiceBase {
        @GET
        fun <T> getNextPage(
            @Url url: String
        ): Single<T>
    }

interface ServiceA : ServiceBase {
    fun getNextImageFeed(url: String) = getNextPage<ImageFeed>(url)
}


        retrofit.addBaseCallAdapterFactory(customExceptionHandlerAdapterFactory)
            .addCallAdapterFactory(adapterFactory)
            .addConverterFactory(converterFactory)
            .build()
            .create(ServiceA::class.java)

In runtime, here's the exception that I got (with r8 full mode turned on), there is no problem if I disable r8 full mode

java.lang.ClassCastException
    at _COROUTINE._BOUNDARY$$ExternalSyntheticThrowCCEIfNotNull3.m(Unknown Source:5)
    at "the place where the retrofit...create(ServiceA::class.java) is called"

This problem goes away if I change service A into the following by adding retrofit annotations:

interface ServiceA : ServiceBase {
    @GET fun getNextImageFeed(@Url url: String) = getNextPage<ImageFeed>(url)
}

So my question is: should we always annotate the service class's methods and if this is a hard requirement, if so is there any guidance of how to enforce it in dev? (lint rule, doc, etc?)

[ricowind]

Just following up here as well, I was looking into this from the R8 side to get a repro https://issuetracker.google.com/398887842

The error that you get when you are not using R8 (minifyEnabled: false -> you are using D8) is actually very actionable:

PROCESS CRASHED
                                                                                                    processName: com.example.retrokotlin
                                                                                                    pid: 8733
                                                                                                    shortMsg: java.lang.IllegalArgumentException
                                                                                                    longMsg: java.lang.IllegalArgumentException: HTTP method annotation is required (e.g., @GET, @POST, etc.).
                                                                                                        for method ImageFeedPagingService.getNextImageFeed
                                                                                                    timeMillis: 1740644467503
                                                                                                    stackTrace:
                                                                                                    java.lang.RuntimeException: Unable to start activity ComponentInfo{com.example.retrokotlin/com.example.retrokotlin.MainActivity}: java.lang.IllegalArgumentException: HTTP method annotation is required (e.g., @GET, @POST, etc.).
                                                                                                        for method ImageFeedPagingService.getNextImageFeed
                                                                                                    	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3270)
                                                                                                    	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3409)
                                                                                                    	at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:83)
                                                                                                    	at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)

You should always validate that your build actually works without R8 before trying to figure out the problem with an R8 compiled app

[halizton]

You should always validate that your build actually works without R8 before trying to figure out the problem with an R8 compiled app

Thanks, yeah this is a good point and it makes me realize the configuration matrix is quite complex now:

1. minifyEnabled: true: our dreamed config, issue described above encountered
2. minifyEnabled: true and r8 full-mode disabled: our current production config, no issues
3. minifyEnabled: false: did not test and did not want to support

In our case, we tested configuration (1) and (2) but didn't test the configuration (3). We ideally wanted to go with option (1). Now I realized that the option (3) could be used as a lint for this issue. At the same time, I hope to have some form of lint in a lighter form :)

[JakeWharton]

You do not need to annotate functions which have implementations such as your getNextImageFeed.

The problem is that the Kotlin compiler is still compiling your interface in a mode that supports Java 6 (for historical reasons) despite producing Java 8 bytecode. This means that the implementation of a function on an interface has to be hoisted to a sibling class, and the original function looks like it has no body.

If you add the -Xjvm-default=all flag (or -Xjvm-default=all-compatibility if you are inside a library and need to maintain binary compatibility), the compiler will switch to using the Java 8 bytecode default method feature.

Retrofit does not process interface methods which have default implementations. So once you enable this compilation strategy, the problems go away.

You can compare the output of the two strategies here: https://kotlin.godbolt.org/z/5n1Ko5hzv. Hopefully you can see that the normal behavior on the top results in Retrofit thinking that your getNextImageFeed is an unannotated endpoint definition because it has no body. But on the bottom, with -Xjvm-default=all specified, the interface changes to have the body directly specified, allowing Retrofit to ignore it.