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

OkHttpCall problems #958

Closed
alexmprog opened this Issue Jul 13, 2015 · 21 comments

Comments

8 participants
@alexmprog

alexmprog commented Jul 13, 2015

Hello, guys.

I tried use Retrofit 2.0 + OkHttp 2.5 in my project.
Before that I used Retrofit 1.9 with OkHttp 2.5 - I had not problems.

About my Retrofit configuration:

  1. I added GsonConverter and GsonConverterFactory from your retrofit-connectors sample.
  2. I created my Gson object with Type adapters.
  3. I created my custom OkHttpClient with Application interceptor(for adding headers)
  4. I created my custom BaseUrl with my service endpoint.
  5. I created ServiceAPI interface with method like this :
    @get("/api/v1/users/{fbid}/init/")
    Call getAppData(@path("fbid") String fbid, @query("delta_since_timestamp") long delta, @query("chat_enabled") long chat);
  6. I created Retorfit object like this :
    private static final Retrofit RETROFIT_MAIN = new Retrofit.Builder().baseUrl
    (sMainEndpoint).client(generateNewOkClient(sMainEndpoint.getUrl()))
    .converterFactory(GsonConverterFactory.create(gsonBuilder)).callbackExecutor
    (Executors.newFixedThreadPool(2))
    .build();
    I added custom Executor only for tests.
  7. I created my service like this:
    private static final ServiceAPI SERVICE_MAIN = RETROFIT_MAIN.create(ServiceAPI.class);

Before next note it's ok.

  1. Now I want use getAppData() method:
    Call call = getService().getAppData(playerFbid, delta, BuildConfig.CHAT_VERSION);
    call.enqueue(new Callback() {
    @override
    public void onResponse(Response response) {
    AppData appData = response.body();
    if (appData != null) {

            }
        }
    
        @Override
        public void onFailure(Throwable error) {
    
        }
    });
    

After that I alwayshave problem in this code (OkHttpCall class, parseResponse method):

// Remove the body's source (the only stateful object) so we can pass the response along.
rawResponse = rawResponse.newBuilder()
    .body(new NoContentResponseBody(rawBody.contentType(), rawBody.contentLength()))
    .build();

After this code I always have Exception like this -
IllegalStateException -"Cannot read raw response body of a converted body."

This exception was generated in NoContentResponseBody class, source() method.
It very strange because I have Response with valid http url and code = 200(Result=OK).

Why I have this exception? May be I have wrong Retrofit configuartion or problem contains in Retrofit 2.0 library?

Thank you for attension.

@felipecsl

This comment has been minimized.

Show comment
Hide comment
@felipecsl

felipecsl Jul 13, 2015

Contributor

if your response is not successful (response.isSuccess() is false), you have to use response.errorBody() instead

Contributor

felipecsl commented Jul 13, 2015

if your response is not successful (response.isSuccess() is false), you have to use response.errorBody() instead

@alexmprog

This comment has been minimized.

Show comment
Hide comment
@alexmprog

alexmprog Jul 13, 2015

Hello felipecsl,
As I wrote earlier I have good Response like this:

Response{protocol=http/1.1, code=200, message=OK, url=https://myservice.com/api/v1/users/100004607503701/init/?delta_since_timestamp=1&chat_enabled=3}

(myservice.com - alias for example)

When I can find response.isSuccess() in your code?

alexmprog commented Jul 13, 2015

Hello felipecsl,
As I wrote earlier I have good Response like this:

Response{protocol=http/1.1, code=200, message=OK, url=https://myservice.com/api/v1/users/100004607503701/init/?delta_since_timestamp=1&chat_enabled=3}

(myservice.com - alias for example)

When I can find response.isSuccess() in your code?

@felipecsl

This comment has been minimized.

Show comment
Hide comment
@felipecsl

felipecsl Jul 13, 2015

Contributor

https://github.com/square/retrofit/blob/master/retrofit/src/main/java/retrofit/Response.java#L92-L94

Oh if your response is actually successful, then I think the problem is that this behavior changed on Retrofit 2.0, since now you can't read the response after it has been already converted. I'm not sure if there's a workaround for this limitation

Contributor

felipecsl commented Jul 13, 2015

https://github.com/square/retrofit/blob/master/retrofit/src/main/java/retrofit/Response.java#L92-L94

Oh if your response is actually successful, then I think the problem is that this behavior changed on Retrofit 2.0, since now you can't read the response after it has been already converted. I'm not sure if there's a workaround for this limitation

@alexmprog

This comment has been minimized.

Show comment
Hide comment
@alexmprog

alexmprog Jul 13, 2015

Sorry, but I don't understand. Is it mean that I will always have this Exception? "you can't read the response after it has been already converted" - where I can find this code? Can you explain this moment?

alexmprog commented Jul 13, 2015

Sorry, but I don't understand. Is it mean that I will always have this Exception? "you can't read the response after it has been already converted" - where I can find this code? Can you explain this moment?

@felipecsl

This comment has been minimized.

Show comment
Hide comment
@felipecsl

felipecsl Jul 13, 2015

Contributor

hmm.. so you're using a raw type (Call) for your service method, you should actually be using Call<YourType> in order to have the body converted to the type of your response body. Then. you'd have Response<YourType> in your onResponse() method. I'm not sure this is the reason why you're getting the exception, but it's a red flag

Contributor

felipecsl commented Jul 13, 2015

hmm.. so you're using a raw type (Call) for your service method, you should actually be using Call<YourType> in order to have the body converted to the type of your response body. Then. you'd have Response<YourType> in your onResponse() method. I'm not sure this is the reason why you're getting the exception, but it's a red flag

@alexmprog

This comment has been minimized.

Show comment
Hide comment
@alexmprog

alexmprog Jul 13, 2015

Sorry, I'm mistaken
I used this method :
"Call getAppData(@path("fbid") String fbid, @query("delta_since_timestamp") long delta, @query("chat_enabled") long chat);"

After I used this code:
"Call call = getService().getAppData(playerFbid, delta, BuildConfig.CHAT_VERSION);
call.enqueue(new Callback() {
@override
public void onResponse(Response response) {
AppData appData = response.body();
if (appData != null) {

    }
}

@Override
public void onFailure(Throwable error) {

}

});"

alexmprog commented Jul 13, 2015

Sorry, I'm mistaken
I used this method :
"Call getAppData(@path("fbid") String fbid, @query("delta_since_timestamp") long delta, @query("chat_enabled") long chat);"

After I used this code:
"Call call = getService().getAppData(playerFbid, delta, BuildConfig.CHAT_VERSION);
call.enqueue(new Callback() {
@override
public void onResponse(Response response) {
AppData appData = response.body();
if (appData != null) {

    }
}

@Override
public void onFailure(Throwable error) {

}

});"

@alexmprog

This comment has been minimized.

Show comment
Hide comment
@alexmprog

This comment has been minimized.

Show comment
Hide comment
@alexmprog

alexmprog Jul 13, 2015

May be I have problem with GsonConverter? But with Retrofit 1.9 I had not this problem

alexmprog commented Jul 13, 2015

May be I have problem with GsonConverter? But with Retrofit 1.9 I had not this problem

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Jul 14, 2015

Collaborator

Paste the entirety of your onResponse callback.

Collaborator

JakeWharton commented Jul 14, 2015

Paste the entirety of your onResponse callback.

@alexmprog

This comment has been minimized.

Show comment
Hide comment
@alexmprog

alexmprog Jul 14, 2015

Hello, Jake
getAppData() method declaration

@GET("/api/v1/users/{fbid}/init/")
    Call<AppData> getAppData(@Path("fbid") String fbid, @Query("delta_since_timestamp") long delta, @Query("chat_enabled") long chat);

Call getAppData() method

Call<AppData> call = getService().getAppData(playerFbid, delta, BuildConfig.CHAT_VERSION);
call.enqueue(new Callback<AppData>() {
            @Override
            public void onResponse(Response<AppData> response) {
                AppData appData = response.body();
                if (appData != null) {
                   // do something
                }
            }

            @Override
            public void onFailure(Throwable error) {
                error.getMessage
            }
        });

I always received Throwable error in onFailure() method, but I have never got Response in onResponse() method. Do you want see any other code?

alexmprog commented Jul 14, 2015

Hello, Jake
getAppData() method declaration

@GET("/api/v1/users/{fbid}/init/")
    Call<AppData> getAppData(@Path("fbid") String fbid, @Query("delta_since_timestamp") long delta, @Query("chat_enabled") long chat);

Call getAppData() method

Call<AppData> call = getService().getAppData(playerFbid, delta, BuildConfig.CHAT_VERSION);
call.enqueue(new Callback<AppData>() {
            @Override
            public void onResponse(Response<AppData> response) {
                AppData appData = response.body();
                if (appData != null) {
                   // do something
                }
            }

            @Override
            public void onFailure(Throwable error) {
                error.getMessage
            }
        });

I always received Throwable error in onFailure() method, but I have never got Response in onResponse() method. Do you want see any other code?

@alexmprog

This comment has been minimized.

Show comment
Hide comment
@alexmprog

alexmprog Jul 14, 2015

I think I found reason why I got this issue - problem in Newrelic agent.

OkHttpCall class, private Response parseResponse(com.squareup.okhttp.Response rawResponse) method:

     ResponseBody rawBody = rawResponse.body();

    // Remove the body's source (the only stateful object) so we can pass the response along.
    rawResponse = rawResponse.newBuilder()
        .body(new NoContentResponseBody(rawBody.contentType(), rawBody.contentLength()))
        .build();

instead of com.squareup.okhttp.Response.Builder app try use com.newrelic.agent.android.instrumentation.okhttp2.ResponseBuilderExtension class from Newrelic

 public Builder body(ResponseBody body) {
        BufferedSource source = body.source();
        boolean length = false;
        Buffer buffer = new Buffer();

        try {
            source.readAll(buffer);
        } catch (IOException var6) {
            log.error("IOException reading from source: ", var6);
        }

        return this.impl.body(new PrebufferedResponseBody(body, buffer));
    }

body.source() throw IllegalStateException:

 java.lang.IllegalStateException: Cannot read raw response body of a converted body.
            at retrofit.NoContentResponseBody.source(NoContentResponseBody.java:41)
            at com.newrelic.agent.android.instrumentation.okhttp2.ResponseBuilderExtension.body(ResponseBuilderExtension.java:70)
            at com.newrelic.agent.android.instrumentation.okhttp2.OkHttp2Instrumentation.body(OkHttp2Instrumentation.java:33)
            at retrofit.OkHttpCall.parseResponse(OkHttpCall.java:124)
            at retrofit.OkHttpCall.access$000(OkHttpCall.java:25)
            at retrofit.OkHttpCall$1.onResponse(OkHttpCall.java:90)
            at com.newrelic.agent.android.instrumentation.okhttp2.CallbackExtension.onResponse(CallbackExtension.java:45)
            at com.squareup.okhttp.Call$AsyncCall.execute(Call.java:170)
            at com.squareup.okhttp.internal.NamedRunnable.run(NamedRunnable.java:33)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
            at java.lang.Thread.run(Thread.java:818)

Do you have any suggestion which can help me?

alexmprog commented Jul 14, 2015

I think I found reason why I got this issue - problem in Newrelic agent.

OkHttpCall class, private Response parseResponse(com.squareup.okhttp.Response rawResponse) method:

     ResponseBody rawBody = rawResponse.body();

    // Remove the body's source (the only stateful object) so we can pass the response along.
    rawResponse = rawResponse.newBuilder()
        .body(new NoContentResponseBody(rawBody.contentType(), rawBody.contentLength()))
        .build();

instead of com.squareup.okhttp.Response.Builder app try use com.newrelic.agent.android.instrumentation.okhttp2.ResponseBuilderExtension class from Newrelic

 public Builder body(ResponseBody body) {
        BufferedSource source = body.source();
        boolean length = false;
        Buffer buffer = new Buffer();

        try {
            source.readAll(buffer);
        } catch (IOException var6) {
            log.error("IOException reading from source: ", var6);
        }

        return this.impl.body(new PrebufferedResponseBody(body, buffer));
    }

body.source() throw IllegalStateException:

 java.lang.IllegalStateException: Cannot read raw response body of a converted body.
            at retrofit.NoContentResponseBody.source(NoContentResponseBody.java:41)
            at com.newrelic.agent.android.instrumentation.okhttp2.ResponseBuilderExtension.body(ResponseBuilderExtension.java:70)
            at com.newrelic.agent.android.instrumentation.okhttp2.OkHttp2Instrumentation.body(OkHttp2Instrumentation.java:33)
            at retrofit.OkHttpCall.parseResponse(OkHttpCall.java:124)
            at retrofit.OkHttpCall.access$000(OkHttpCall.java:25)
            at retrofit.OkHttpCall$1.onResponse(OkHttpCall.java:90)
            at com.newrelic.agent.android.instrumentation.okhttp2.CallbackExtension.onResponse(CallbackExtension.java:45)
            at com.squareup.okhttp.Call$AsyncCall.execute(Call.java:170)
            at com.squareup.okhttp.internal.NamedRunnable.run(NamedRunnable.java:33)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
            at java.lang.Thread.run(Thread.java:818)

Do you have any suggestion which can help me?

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Jul 14, 2015

Collaborator

It sounds like you need to put the NewRelic stuff in an OkHttp interceptor. It shouldn't know anything about Retrofit and it definitely shouldn't be sitting on top of it. If it's doing this automatically (I know they like to do dirty things for "easy" integration) then it's a bug on their side. And if it's not a bug, it's a usage question and since I have never used New Relic you're unlikely to get a good answer here.

I would suggest posting on StackOverflow so that other users of both see it. The amount of people who will see it here is very few and the number who know about New Relic is probably close to 0.

Collaborator

JakeWharton commented Jul 14, 2015

It sounds like you need to put the NewRelic stuff in an OkHttp interceptor. It shouldn't know anything about Retrofit and it definitely shouldn't be sitting on top of it. If it's doing this automatically (I know they like to do dirty things for "easy" integration) then it's a bug on their side. And if it's not a bug, it's a usage question and since I have never used New Relic you're unlikely to get a good answer here.

I would suggest posting on StackOverflow so that other users of both see it. The amount of people who will see it here is very few and the number who know about New Relic is probably close to 0.

@alexmprog

This comment has been minimized.

Show comment
Hide comment
@alexmprog

alexmprog Jul 14, 2015

Sorry, but I don't understand how I can put NewRelic stuff in an OkHttp interceptor. To be clear - I used Application Interceptor in my OkHttp client - I add several headers to request. Can you give me some code example about "NewRelic stuff"?

alexmprog commented Jul 14, 2015

Sorry, but I don't understand how I can put NewRelic stuff in an OkHttp interceptor. To be clear - I used Application Interceptor in my OkHttp client - I add several headers to request. Can you give me some code example about "NewRelic stuff"?

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Jul 14, 2015

Collaborator

I have never used New Relic and I have no idea what they're doing. This is why I suggested you post elsewhere like StackOverflow or on their support forum so that those who work on/with New Relic can help.

If you are using an application interceptor this should work fine because it happens before Retrofit removes the body. I don't see how New Relic would get access to the raw response with the removed body.

Collaborator

JakeWharton commented Jul 14, 2015

I have never used New Relic and I have no idea what they're doing. This is why I suggested you post elsewhere like StackOverflow or on their support forum so that those who work on/with New Relic can help.

If you are using an application interceptor this should work fine because it happens before Retrofit removes the body. I don't see how New Relic would get access to the raw response with the removed body.

@alexmprog

This comment has been minimized.

Show comment
Hide comment
@alexmprog

alexmprog commented Jul 15, 2015

@kaeawc

This comment has been minimized.

Show comment
Hide comment
@kaeawc

kaeawc Jul 16, 2015

@JakeWharton I've run into this too and am currently working with NewRelic on a fix. You mention that you don't use NewRelic... how should a dev using Retrofit monitor network requests? My first thought is to use OkHttp Interceptors and then batch requests to a monitoring service, which is possibly what NewRelic is trying to do. Is there a better or different way?

Apologies if this is the wrong place to ask, but I'm not sure where else to post such a question.

kaeawc commented Jul 16, 2015

@JakeWharton I've run into this too and am currently working with NewRelic on a fix. You mention that you don't use NewRelic... how should a dev using Retrofit monitor network requests? My first thought is to use OkHttp Interceptors and then batch requests to a monitoring service, which is possibly what NewRelic is trying to do. Is there a better or different way?

Apologies if this is the wrong place to ask, but I'm not sure where else to post such a question.

@JakeWharton

This comment has been minimized.

Show comment
Hide comment
@JakeWharton

JakeWharton Jul 16, 2015

Collaborator

That's exactly what we do, yes. We pass API call data from an OkHttp interceptor along like any other analytics data which gets batched automatically and sent somewhere (as all analytics services should be doing).

I'm not against NewRelic or anything. I just have zero exposure to it and since v2.0 is undergoing development I don't really have the time to find out what they're doing.

Collaborator

JakeWharton commented Jul 16, 2015

That's exactly what we do, yes. We pass API call data from an OkHttp interceptor along like any other analytics data which gets batched automatically and sent somewhere (as all analytics services should be doing).

I'm not against NewRelic or anything. I just have zero exposure to it and since v2.0 is undergoing development I don't really have the time to find out what they're doing.

@ialmetwally

This comment has been minimized.

Show comment
Hide comment
@ialmetwally

ialmetwally Sep 10, 2015

I'm having the same issue without NewRelic when I try to read the raw response body, for some reason I need to have both the converted response and save the raw JSON string as well. So I implemented a ResponseCallback that extends Callback and in onResponse(Response response) I can receive the converted body as type T, but when I try to read the raw response response.raw().body().string(), I get the following exception:

java.lang.IllegalStateException: Cannot read raw response body of a converted body.
at retrofit.NoContentResponseBody.source(NoContentResponseBody.java:41)
at com.squareup.okhttp.ResponseBody.bytes(ResponseBody.java:54)
at com.squareup.okhttp.ResponseBody.string(ResponseBody.java:83)
at .....ResponseCallback.getRawBodyAsString(ResponseCallback.java:181)
at .....ResponseCallback.onResponse(ResponseCallback.java:81)
at retrofit.ExecutorCallAdapterFactory$ExecutorCallback$1.run(ExecutorCallAdapterFactory.java:84)
at android.os.Handler.handleCallback(Handler.java:739)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:155)
at android.app.ActivityThread.main(ActivityThread.java:5696)
at java.lang.reflect.Method.invoke(Native Method)
at java.lang.reflect.Method.invoke(Method.java:372)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1028)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:823)

By the way I was using NewRelic, but it's completely disabled now until they fix the conflict.

ialmetwally commented Sep 10, 2015

I'm having the same issue without NewRelic when I try to read the raw response body, for some reason I need to have both the converted response and save the raw JSON string as well. So I implemented a ResponseCallback that extends Callback and in onResponse(Response response) I can receive the converted body as type T, but when I try to read the raw response response.raw().body().string(), I get the following exception:

java.lang.IllegalStateException: Cannot read raw response body of a converted body.
at retrofit.NoContentResponseBody.source(NoContentResponseBody.java:41)
at com.squareup.okhttp.ResponseBody.bytes(ResponseBody.java:54)
at com.squareup.okhttp.ResponseBody.string(ResponseBody.java:83)
at .....ResponseCallback.getRawBodyAsString(ResponseCallback.java:181)
at .....ResponseCallback.onResponse(ResponseCallback.java:81)
at retrofit.ExecutorCallAdapterFactory$ExecutorCallback$1.run(ExecutorCallAdapterFactory.java:84)
at android.os.Handler.handleCallback(Handler.java:739)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:155)
at android.app.ActivityThread.main(ActivityThread.java:5696)
at java.lang.reflect.Method.invoke(Native Method)
at java.lang.reflect.Method.invoke(Method.java:372)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1028)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:823)

By the way I was using NewRelic, but it's completely disabled now until they fix the conflict.

@OlisGS

This comment has been minimized.

Show comment
Hide comment
@OlisGS

OlisGS Apr 12, 2016

just write "onResponse(Response ResponseBody response)" to convert your response to ResponseBody

OlisGS commented Apr 12, 2016

just write "onResponse(Response ResponseBody response)" to convert your response to ResponseBody

@ghuiii

This comment has been minimized.

Show comment
Hide comment
@ghuiii

ghuiii Jun 21, 2017

@ialmetwally did you find a way to do that?

ghuiii commented Jun 21, 2017

@ialmetwally did you find a way to do that?

@st0rmtroop3r

This comment has been minimized.

Show comment
Hide comment
@st0rmtroop3r

st0rmtroop3r Jul 5, 2018

but when I try to read the raw response response.raw().body().string(), I get the following exception:

I faced the same issue today (I don't use NewRelic too). Calling body() from response instead of raw() works fine.
response.body().string()

st0rmtroop3r commented Jul 5, 2018

but when I try to read the raw response response.raw().body().string(), I get the following exception:

I faced the same issue today (I don't use NewRelic too). Calling body() from response instead of raw() works fine.
response.body().string()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment