retrofit icon indicating copy to clipboard operation
retrofit copied to clipboard

HTTP 204/205 bypass converter and unconditionally send null to adapter

Open NeverwinterMoon opened this issue 7 years ago • 18 comments

I stumbled upon this: https://github.com/square/retrofit/issues/1554 and added a converter because I wanted to handle the empty response body and not pass it as null to Observable. That thread is talking about code 200 and empty body though.

Then I found out that in the current Retrofit version no converter is called at all for 204 and 205:

    if (code == 204 || code == 205) {
      rawBody.close();
      return Response.success(null, rawResponse);
    }

This forces me to use Single<Response<SomeModel>> instead of Single<SomeModel> I was hoping to use...

I understand that 204 stands for no content, so it's only logical to pass the response as null. But for my specific case, I would prefer to handle 204 in onError, so I hoped to handle the empty body in the converter and just throw a custom error there instead.

Is there any chance that approach will be changed? I am relatively new to Retrofit but I am used to Alamofire on iOS and there it's possible to specify during configuration what is treated as a successful response code:

.validate(statusCode: 200)

So every code that is not on that list would be handled as an error.

NeverwinterMoon avatar Aug 23 '18 17:08 NeverwinterMoon

You should be able to add an OkHttp Interceptor that can convert original response code from 204 to 200 before it gets passed to Retrofit

artem-zinnatullin avatar Aug 23 '18 22:08 artem-zinnatullin

If you need to error the request out, you can detect 204 in OkHttp Interceptor and throw a error, sorry I misinterpret the issue, I though you wanted to handle 204 as 200.

artem-zinnatullin avatar Aug 23 '18 22:08 artem-zinnatullin

@artem-zinnatullin Aha, thanks for your advice! I just tried the throwing-in-interceptor approach out, and it seems to satisfy my needs.

Nonetheless, I am curious if Retrofit would ever allow to configure custom validation (similar to Alamofire) for response codes or if it's completely against the Retrofit's design ideas.

NeverwinterMoon avatar Aug 24 '18 06:08 NeverwinterMoon

    if (code == 204 || code == 205) {
      return Response.success(null, rawResponse);
    }

Isn't this violating the Response class contract? Reading from Response.java:

  /** Returns true if {@link #code()} is in the range [200..300). */
  public boolean isSuccessful() {
    return rawResponse.isSuccessful();
  }

  /** The deserialized response body of a {@linkplain #isSuccessful() successful} response. */
  public @Nullable T body() {
    return body;
  }

The way I read it, response.body() is always non-null if isSuccesful() returns true. But in fact, if the server returns 204, we get a null regardless.

And because OkHttpCall short-circuits the body parsing to null, we don't even get a chance to change it to something more sensible with Converters, like for example receiving the empty ResponseBody with BuiltInConverters.responseBodyConverter:

    @DELETE("user/{id}")
    Single<ResponseBody> returns204(@Path("id") int id);

So this does not work, which is rather unexpcted (empty body is a valid body after all, right?)

villesinisalo avatar Oct 22 '18 23:10 villesinisalo

I'm going to look at trying a PR for better handling of this, but part of the confusion is also that RxJava silently drops the null value emitted, and hence why it's a NoSuchElementException rather than NPE

https://github.com/ReactiveX/RxJava/blob/0e7b8eaa61f9cac0538ef6a59bfbd0b119b87732/src/main/java/io/reactivex/internal/operators/observable/ObservableSingleSingle.java#L74-L83

ZacSweers avatar Jan 14 '19 05:01 ZacSweers

I've put up a proposal PR in #3003, feedback welcome!

ZacSweers avatar Jan 14 '19 19:01 ZacSweers

A quick workaround for those who want to start using the 2.6.0, add the below as your first interceptor in OkHttp client. After that, you would be able to call services which returns 204 / 205 as such: image

import okhttp3.Interceptor
import okhttp3.MediaType
import okhttp3.Response
import okhttp3.ResponseBody
import timber.log.Timber

class EmptyBodyInterceptor : Interceptor {
    override fun intercept(chain: Interceptor.Chain): Response {
        val response = chain.proceed(chain.request())
        if (!response.isSuccessful) {
            return response
        }

        if (response.code() != 204 && response.code() != 205) {
            return response
        }

        if ((response.body()?.contentLength() ?: -1) >= 0) {
            return response
        }

        Timber.d("Rewriting response to contain an empty body.")
        val emptyBody = ResponseBody.create(MediaType.get("text/plain"), "")

        return response
            .newBuilder()
            .code(200)
            .body(emptyBody)
            .build()
    }
}

Regards

jgavazzisp avatar Jul 16 '19 19:07 jgavazzisp

Inspired from the snippet just above, I made a version that doesn't fail when the server responds with a content length of zero, and is lenient if there's actually some content:

import okhttp3.Interceptor
import okhttp3.MediaType
import okhttp3.Response
import okhttp3.ResponseBody

object EmptyBodyInterceptor : Interceptor {

    override fun intercept(chain: Interceptor.Chain): Response {
        val response = chain.proceed(chain.request())
        if (response.isSuccessful.not() || response.code().let { it != 204 && it != 205 }) {
            return response
        }

        if ((response.body()?.contentLength() ?: -1) >= 0) {
            return response.newBuilder().code(200).build()
        }

        val emptyBody = ResponseBody.create(MediaType.get("text/plain"), "")

        return response
            .newBuilder()
            .code(200)
            .body(emptyBody)
            .build()
    }
}

LouisCAD avatar Apr 21 '20 01:04 LouisCAD

Also inspired from the snippets above, here is my version:

import okhttp3.Interceptor
import okhttp3.MediaType.Companion.toMediaType
import okhttp3.Response
import okhttp3.ResponseBody.Companion.toResponseBody

class NoContentInterceptor : Interceptor {
    
    private val noContentHttpStatusCodes = arrayOf(204, 205) 
    
    override fun intercept(chain: Interceptor.Chain): Response {
        val response = chain.proceed(chain.request())
        
        if (!response.isSuccessful) {
            return response
        }

        if (!noContentHttpStatusCodes.contains(response.code)) {
            return response
        }

        return response
            .newBuilder()
            .code(200)
            .body("".toResponseBody("text/plain".toMediaType()))
            .build()
    }
}

crisu83 avatar Feb 07 '22 22:02 crisu83

You should still propagate headers

JakeWharton avatar Feb 08 '22 01:02 JakeWharton

Also inspired from the snippets above, here is my version:

import okhttp3.Interceptor
import okhttp3.MediaType.Companion.toMediaType
import okhttp3.Response
import okhttp3.ResponseBody.Companion.toResponseBody

class NoContentInterceptor : Interceptor {
    
    private val noContentHttpStatusCodes = arrayOf(204, 205) 
    
    override fun intercept(chain: Interceptor.Chain): Response {
        val response = chain.proceed(chain.request())
        
        if (!response.isSuccessful) {
            return response
        }

        if (!noContentHttpStatusCodes.contains(response.code)) {
            return response
        }

        return response
            .newBuilder()
            .code(200)
            .body("".toResponseBody("text/plain".toMediaType()))
            .build()
    }
}

Hi, can you tell me how to properly use toMediaType? I've tried to import the headers and it told me Companion is not found...

image

My Retrofit version is 2.9.0. Thanks!

RoyRao2333 avatar Feb 10 '22 08:02 RoyRao2333

It requires that you are using OkHttp 4.x

JakeWharton avatar Feb 10 '22 13:02 JakeWharton

It requires that you are using OkHttp 4.x

Thanks for the reply!

As far as I know, Retrofit is a wrapper around OkHttp, and I can find okhttp3 package in my project after I configured Retrofit 2.9.0.

However, I don't know what version exactly Retrofit brought me... Should I configure OkHttp 4.x independently in build.gradle?

RoyRao2333 avatar Feb 10 '22 13:02 RoyRao2333

Yes. Retrofit depend on OkHttp 3.14 and the latest binary compatible release is 4.9.

JakeWharton avatar Feb 10 '22 13:02 JakeWharton

Yes. Retrofit depend on OkHttp 3.14 and the latest binary compatible release is 4.9.

Got it! You really saved my day. Thanks again!

RoyRao2333 avatar Feb 10 '22 13:02 RoyRao2333

I've solved the issue by (in Interceptor):

return if (response.code == 204 || response.code == 205) { response.newBuilder() .code(200) .body("{}".toResponseBody()) .build() } else { response }

nikita-bilous avatar Nov 29 '22 08:11 nikita-bilous