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

[Android] [R8] IllegalArgumentException thrown when calling a service operation with unused return type #4011

Closed
vjgarciag96 opened this issue Dec 6, 2023 · 4 comments

Comments

@vjgarciag96
Copy link

vjgarciag96 commented Dec 6, 2023

Steps to reproduce:

  1. Define an API service that looks as follows:
@Serializable
data class ResponseBody(
    @SerialName("status") val status: String? = null,
)

interface ExampleApiService {

    @GET("endpoint/v1")
    suspend fun getOperation(): Response<ResponseBody>

    companion object {
        fun create(): ExampleApiService {
            Retrofit.Builder()
                ...
                .addConverterFactory(Json.asConverterFactory("application/json".toMediaType()))
                .build()
                .create(ExampleApiService::class.java)
        }
    }
}
  1. Use the API operation but ignore the response body
val apiService  = ExampleApiService.create()

val response = apiService.getOperation()

if (response.isSuccessful) {
    // ignore body
}
  1. Run that code in an Android minified app, and the next exception will be thrown:
FATAL EXCEPTION: main
Process: com.vicgarci.repro, PID: 20782
java.lang.IllegalArgumentException: Unable to create converter for class java.lang.Object
  for method ExampleApiService.getOperation
  at retrofit2.Utils.methodError(Utils.java:54)
	at retrofit2.HttpServiceMethod.createResponseConverter(HttpServiceMethod.java:126)
	at retrofit2.HttpServiceMethod.parseAnnotations(HttpServiceMethod.java:85)
	at retrofit2.ServiceMethod.parseAnnotations(ServiceMethod.java:39)
	at retrofit2.Retrofit.loadServiceMethod(Retrofit.java:202)
	at retrofit2.Retrofit$1.invoke(Retrofit.java:160)
	at java.lang.reflect.Proxy.invoke(Proxy.java:1006)
	at $Proxy2.postOperation1(Unknown Source)

...
Caused by: kotlinx.serialization.SerializationException: Serializer for class 'Any' is not found.
Please ensure that class is marked as '@Serializable' and that the serialization compiler plugin is applied.                                                                                                  
  at kotlinx.serialization.internal.PlatformKt.serializerNotRegistered(Platform.kt:31)
	at kotlinx.serialization.SerializersKt__SerializersJvmKt.serializer(SerializersJvm.kt:77)
	at kotlinx.serialization.SerializersKt.serializer(Unknown Source:1)
	at com.jakewharton.retrofit2.converter.kotlinx.serialization.Serializer.serializer(Serializer.kt:21)
	at com.jakewharton.retrofit2.converter.kotlinx.serialization.Factory.responseBodyConverter(Factory.kt:26)
	at retrofit2.Retrofit.nextResponseBodyConverter(Retrofit.java:362)
	at retrofit2.Retrofit.responseBodyConverter(Retrofit.java:345)
	at retrofit2.HttpServiceMethod.createResponseConverter(HttpServiceMethod.java:124)
	... 80 more

Android app project with code to repro the issue: https://github.com/vjgarciag96/retrofit-unused-return-bug-repro

Retrofit version is 2.9.0, but I imagine it can be reproduced in earlier version too.

Cause

I believe the problem is caused by a combination of R8, retrofit, and Json's ConverterFactory in this example:

  • R8 detects the ResponseBody type is unused, and so it removes the class and replaces its usages with the Object type.
  • When retrofit tries to create the responseConverter for ExampleApiService.getOperation, it delegates to Jsons ConverterFactory, which will provide the appropriate deserializer.
  • Jsons ConverterFactory finds that the type it needs to provide a deserializer for is Object (or Any in the Kotlin world) and fails because it does not know how to do that.

Solution

A simple solution would be to remove the unused ResponseBody class, and replace its usage with Unit. This works fine and fixes the issue, although I'm not sure whether you could run into the same problem when building a release build if you only use the result in debug builds.

Issue?

I'm still not sure this is an issue retrofit should try to prevent, but it feels like it is something that apps could run into inadvertently so perhaps it could be a good idea to ship consumer proguard rules in the library that would prevent an scenario like that from happening? I'm keen to hear thoughts and opinions about this, thanks!

@Goooler
Copy link
Contributor

Goooler commented Dec 18, 2023

Check #3751 (comment).

@vjgarciag96
Copy link
Author

Thanks for the pointer @Goooler! I believe the issue still happens with those rules added. The sample project to repro the problem shared in the issue has the rules that follow:

-keep,allowobfuscation,allowshrinking class kotlin.coroutines.Continuation
-keep,allowobfuscation,allowshrinking interface retrofit2.Call
-keep,allowobfuscation,allowshrinking class retrofit2.Response
# R8 full mode strips generic signatures from return types if not kept.
-if interface * { @retrofit2.http.* public *** *(...); }
-keep,allowoptimization,allowshrinking,allowobfuscation class <3>

https://github.com/vjgarciag96/retrofit-unused-return-bug-repro/blob/main/app/proguard-rules.pro#L26-L31

@JakeWharton
Copy link
Collaborator

This is #3588. There is no workaround via rules. You must explicitly keep the concrete types.

I'll move the annotation processor once I'm off of pat leave.

@JakeWharton JakeWharton closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2023
@JakeWharton
Copy link
Collaborator

By the way, thanks for the repro! Too few issues have them leading me to guess at the problem, cause, and fix. My responses are terse because I'm on mobile!

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

3 participants