Unsuccessful responses don't return converted error object

[almeric]

Hi,

Just tested your adapter, but when calling a end-point which returns a 404 I'm not getting the converted error object back in the NetworkResponse.ServerError() (body is always null).

It looks like here https://github.com/haroldadmin/NetworkResponseAdapter/blob/master/src/main/kotlin/com/haroldadmin/cnradapter/NetworkResponseCall.kt#L22 it will always return a null body, and the actual error in Response.errorBody is ignored.

Also isn't it better to check if the Response is succesfull by using response.isSuccessful?

I think the following (lines 19-23 in NetworkResponseCall):

if (body != null) {
    callback.onResponse(this@NetworkResponseCall, Response.success(NetworkResponse.Success(body, response.headers())))
} else {
    callback.onResponse(this@NetworkResponseCall, Response.success(NetworkResponse.ServerError(null, response.code(), response.headers())))
}

could be replaced with something like this (note response.extractError doesn't exist yet) :

if(response.isSuccessful) {
    callback.onResponse(this@NetworkResponseCall,   Response.success(NetworkResponse.Success(body, response.headers())))
} else {
    callback.onResponse(this@NetworkResponseCall, Response.success(NetworkResponse.ServerError(response.extractError(errorBodyConverter), response.code(), response.headers())))
}

An example of a failed request:

<-- 404 https://api.themoviedb.org/4/discover/movies?sort_by=popularity.desc (126ms)
date: Fri, 18 Oct 2019 11:49:09 GMT
content-type: application/json;charset=utf-8
...
{"status_code":34,"status_message":"The resource you requested could not be found.","success":false}
<-- END HTTP (100-byte body)

And the logged response:

ServerError(body=null, code=404, headers=[snipped for brevity])

[haroldadmin]

Thank you for filing this bug. It does appear that the NetworkResponseCall class does not handle the ServerError case correctly. But since this issue is related to this class only, the problem is only confined to suspend methods. Methods using the Deferred return type should continue to work for now.

I'll fix this error and add tests to make sure this does not resurface in the future.

[haroldadmin]

@almeric This has been fixed and is now available in Version 3.0.1. Thank you for reporting this issue 😊

[almeric]

Thanks for the quick fix!