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

provide retrofit2.Response on retrofit2.Callback.onFailure (if error happened on "processing the response") #3749

Open
1 of 3 tasks
cakcaraka opened this issue Jun 21, 2022 · 3 comments

Comments

@cakcaraka
Copy link

cakcaraka commented Jun 21, 2022

What kind of issue is this?

  • Question. This issue tracker is not the place for questions. If you want to ask how to do
    something, or to understand why something isn't working the way you expect it to, use Stack
    Overflow. https://stackoverflow.com/questions/tagged/retrofit

  • Bug report. If you’ve found a bug, spend the time to write a failing test. Bugs with tests
    get fixed. Here’s an example: https://gist.github.com/swankjesse/6608b4713ad80988cdc9

  • Feature Request. Start by telling us what problem you’re trying to solve. Often a solution
    already exists! Don’t send pull requests to implement new features without first getting our
    support. Sometimes we leave features out on purpose to keep the project small.

In my project, currently I'm using MoshiConverterFactory which returns JsonDataException when the payload returned from server is invalid (This one is expected), which triggers onFailure on retrofit2.Callback.onFailure (as stated in the docs https://github.com/square/retrofit/blob/master/retrofit/src/main/java/retrofit2/Callback.java#L43, this is an unexpected exception during 'processing the response')

Right now, I want to build some logging capability which logs an event every time JsonDataException happens, And i need to log some extra information that is returned from the response's header [returned by server, need to log these for cross-reference checking between logs]).

Reference that I have checked:

  1. https://github.com/square/retrofit/blob/master/retrofit/src/main/java/retrofit2/OkHttpCall.java#L156, it doesn't propagate the rawResponse to callFailure
  2. https://github.com/square/retrofit/blob/master/retrofit/src/main/java/retrofit2/OkHttpCall.java#L243, it only sends the rawBody instead of rawResponse so logging it inside a custom converter isn't possible as well.

Is it sensible to request a retrofit.Response instance on retrofit2.Callback.onFailure? Or is there any way I can achieve this?

@JakeWharton
Copy link
Collaborator

Unfortunately the callback can be invoked before a response is received if the transport fails sending the request or it fails while reading the response so it wasn't designed in a way where the response was available. Additionally, the response isn't given to the converter so there isn't a good way to provide such functionality through a converter wrapper.

I don't have a good way to do this off the top of my head.

@cakcaraka
Copy link
Author

One workaround I can think of is to create a CallAdapter that expects Map<String, Any> instead of the actual ResponseModel, and then do the moshi conversion manually on the retrofit2.Callback.onResponse (do try..catch here).

I think this might work but the cons is it does double de-serialization (rawResponse -> Map<String, Any> -> ResponseModel), but I'm not sure the perf impact of doing this.


Or is there a way for the CallAdapter to receive the RawResponse directly so no need to convert into an temporary Map<String,Any>?

@cakcaraka
Copy link
Author

Okay..
Looks like I was able to do it by making changing CallAdapter<ResponseModel, T> to CallAdapter<ResponseBody,T>, and then convert it to ReponseModel manually on retrofit.Callback.onResponse by using moshi.adapter<ResponseModel>(returnType).fromJson(response.body.source()). (try catch here and log if it returns JsonDataException)..So far it works but I haven't done any benchmarks yet.

Do you think I'm heading into the right direction on achieving what I'm intended to? 🤔

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

2 participants