square / retrofit

A type-safe HTTP client for Android and the JVM
https://square.github.io/retrofit/
Apache License 2.0
42.99k stars 7.3k forks source link

Parse Kotlin metadata manually to honor nullability and avoid kotlin-metadata-jvm dep #3075

Open AidanLaing opened 5 years ago

AidanLaing commented 5 years ago

Version: 2.6.0-SNAPSHOT Exception: KotlinNullPointerException Message: Response from {path to my suspend fun...} was null but response body type was declared as non-null

The current structure of my suspend fun in my service interface:

@POST("api/...")
suspend fun request(@Body body: Body): Response?

I'm using Gson for serialization.

Is there a way to declare the response body type as nullable that I'm missing? I've tried adding null safety on my response object with ? and @Nullable with no success.

JakeWharton commented 5 years ago

Not currently. I'm working on a parser for Kotlin metadata so we don't have to depend on a gigantic jar to read one isNullable boolean.

On Thu, Apr 11, 2019, 8:27 PM Aidan Laing notifications@github.com wrote:

Version: 2.6.0-SNAPSHOT Exception: KotlinNullPointerException Message: Response from {path to my suspend fun...} was null but response body type was declared as non-null

The current structure of my suspend fun in my service interface:

@POST("api/...") suspend fun request(@Body body: Body): Response?

I'm using Gson for serialization.

Is there a way to declare the response body type as nullable that I'm missing? I've tried adding null safety on my response object with ? and @Nullable with no success.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/square/retrofit/issues/3075, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEb6RFk0-5aDUMDF6fMlN9U2r9w7Hks5vf9MHgaJpZM4crGCw .

rodion-m commented 5 years ago

@JakeWharton Please notice that suspend ...: List<*> also is not supported. Version: 2.6.0-SNAPSHOT For instance: suspend fun fetchStrings(): List<String>

JakeWharton commented 5 years ago

I'm not sure how that's related to null body handling. Feel free to file a separate issue with a failing test case.

On Wed, Apr 17, 2019, 9:41 PM Rodion Mostovoy notifications@github.com wrote:

@JakeWharton https://github.com/JakeWharton Please notice that suspend ...: List<*> also is not supported. Version: 2.6.0-SNAPSHOT For instance: suspend fun fetchStrings(): List

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/square/retrofit/issues/3075#issuecomment-484323140, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAQIEKC4PRPCZTA5WTQBNDPQ7GTBANCNFSM4HFMMCYA .

wlara commented 5 years ago

Added @JakeWharton I'm getting a similar KotlinNullPointerException using a service that returns 204 (no content) on a delete call. Something like this: @DELETE("/api/{id}") suspend fun deleteItem(@Path("id") id: String) Would that use case also be fixed as part of this issue?

JakeWharton commented 5 years ago

No. That's #2867

Hospes commented 5 years ago

Hi there! Any chance to get update on this issue? :) Want to move to 2.6.0 and remove all other dependencies but can't cause of null response body not supported :(

jgavazzisp commented 5 years ago

Moved to #2867

JakeWharton commented 5 years ago

That is not related to this issue. It's #2867.

JavierSegoviaCordoba commented 5 years ago

So at this moment we should force an exception with a interceptor for 204 responses?

JakeWharton commented 5 years ago

This issue has nothing to do with 204 handling and when it's fixed 204 handling won't have changed in any way. That's #2867.

claucookie commented 5 years ago

Just in case it helps. I was experiencing the exact same KotlinNPE issue over the last 2 days and after trying Converters and Adapters what really worked was to return Response<Unit> in the retrofit method declaration.

I use Retrofit 2.6.0.

Pitel commented 4 years ago

Any progress? We've just changed some of our REST endpoints to 204. So I thought it would be enought to just remove response type (making it Unit), but no.

Using Response<Unit> works :+1:

vladimirfx commented 4 years ago

Why not:

https://github.com/Kotlin/kotlinx.reflect.lite

JakeWharton commented 4 years ago

Because it doesn't provide the necessary info

september669 commented 4 years ago

Using Response<Unit> works

In this case retrofit does not throw any exception when got 4xx codes Reproduced at 2.6.2

JakeWharton commented 4 years ago

That is the expected behavior for that return type.

MiSikora commented 4 years ago

Is it something that is open for a PR or do you want to implement it yourself in Retrofit or as an external project and use it here? I see that there is an old branch so I'd like to ask first before spending some time on it.

JakeWharton commented 4 years ago

Feel free to give it a shot

On Tue, Feb 25, 2020, at 5:31 AM, Michał Sikora wrote:

Is it something that is open for a PR or do you want to implement it yourself in Retrofit or as an external project and use it here? I see that there is an old branch https://github.com/square/retrofit/tree/jakew/nully/2019-02-15 so I'd like to ask first before spending some time on it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/square/retrofit/issues/3075?email_source=notifications&email_token=AAAQIELMEQYZS3ED6MORGVDRETXOJA5CNFSM4HFMMCYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEM3NYOQ#issuecomment-590797882, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQIENX2JYOB4K4V6XXXVDRETXOJANCNFSM4HFMMCYA.

RoarGronmo commented 4 years ago

Found that empty response failed with the NPE mentioned. To cope with that problem, I wrapped all my backend json calls (on the backend side) inside an array. So if the result was empty, I returned an empty array at least.

Just a tip to solve the problem if you have access and ability to change your backend data source.

RG

jemshit commented 4 years ago

Using Response<Unit> works

In this case retrofit does not throw any exception when got 4xx codes Reproduced at 2.6.2

Hi, to handle empty response only choice seems like Response<Void> but that makes 4xx look like success. Any proper way to handle both empty response AND see 4xx as error (e.g. @GET suspend fun should throw exception) ?

Update: Using custom response converter suggested in #1554 works fine, (not converting empty body and returning null in such cases). But return type in API interface must be Unit instead of Any?. Otherwise it throws exception written above by OP

magneticflux- commented 3 years ago

https://github.com/square/retrofit/issues/3075#issuecomment-586156499

Why not:

https://github.com/Kotlin/kotlinx.reflect.lite

https://github.com/square/retrofit/issues/3075#issuecomment-586279157

Because it doesn't provide the necessary info

Are you sure? It looks to me like you can read "one isNullable boolean" right here: https://github.com/Kotlin/kotlinx.reflect.lite/blob/47bfb95540f361143add39300017c6904720812a/src/main/java/kotlinx/reflect/lite/api.kt#L45-L47

klaszlo8207 commented 3 years ago

Any new on this?

How to handle 400 codes and empty body? I tried everything in this topic, but nothing worked.

Success called with the 400 error

GiridharaSPK commented 3 years ago

I have added wrong headers that caused me that HTTP 400 Bad request error.

removing the following worked .addHeader("Content-Encoding", "UTF-8") .header("Accept-Encoding", "identity")

  might help someone
rezznov commented 3 years ago

@JakeWharton not any updates about this? Its been 2 years since issue has open :D

rezznov commented 3 years ago

right now I handled this by a simple try catch like this

try {
                ResultWrapper.Success(request.invoke())
            } catch (e: KotlinNullPointerException) {
                ResultWrapper.NoContentError
            }
LouisCAD commented 3 years ago

Having an interceptor that rewrites the http 204 and 205 responses to 200 should work best.

ferinagy commented 3 years ago

Hi, I hit this a few times recently, so I thought I'd give it a shot: https://github.com/square/retrofit/pull/3544. Any feedback is welcome.

Snailedlt commented 3 years ago

If you get No type arguments expected for class Response when using Response<Unit>, make sure you're using retrofit2.Response<> and not something like okhttp3.Response. That fixed the issue for me

tamimattafi commented 1 year ago

If you get No type arguments expected for class Response when using Response<Unit>, make sure you're using retrofit2.Response<> and not something like okhttp3.Response. That fixed the issue for me

This saved me a lot of digging, was using a Response<T> class from my module

MoustafaElsaghier commented 1 year ago

any suggestion for flow?