Fuel: validatorResponseInterceptor always response FuelError

[babedev]

Fuel: v1.12.0

I add validatorResponseInterceptor(200..499) to FuelManager
Should it allow 200 to 499 as valid status code?

I checked FuelManager that it add validatorResponseInterceptor as new interceptor, but it already has validatorResponseInterceptor(200..299). So it might do the 200..299 before my interceptor and response error.

Here is my code

with(FuelManager.instance) {
    basePath = BuildConfig.BASE_URL

    addResponseInterceptor(validatorResponseInterceptor(200..499))
}

"xxx".httpPost()
    .responseObject(...) { _, _, result ->
        // status code is 422
        result.fold({ items ->
             // Should it response here?
        }, { error ->
             // Always get error         
        })
    }

[kittinunf]

Yeah, if you really need (200..499), you need to remove the existing one first. Then, add your custom one. Does it make sense to you?

[babedev]

Should it replace the old one? Because there is no need to have multiple validatorResponseInterceptor.

[kittinunf]

Hmm, why do you want to replace 200..299 ?

[babedev]

I want to allow 200..499. But it failed because it has validatorResponseInterceptor(200..299) as default validator.
Would it better if new validatorResponseInterceptor replace the existing one?

[kittinunf]

Yeah, but 200..499 won't work. Because 300...499 shouldn't be treated as Success, should we?

[babedev]

Um.... yes, you are right. I have scenario which I need to get JSON response body from status 4xx.
So I might go this way.

{ _, res, result ->
   result.fold({
      // Success
   }, { 
      // Deserialise String(res.data)
   })
}

Should we have JSON deserialiser for error case?

[kittinunf]

You are right, true ... I think we should do that. Sound like a good item for another PR.

I think we could improve in this block.

https://github.com/kittinunf/Fuel/blob/0001c44efe9bf1389ba742292a45b62837723801/fuel/src/main/kotlin/com/github/kittinunf/fuel/core/Deserializable.kt#L69-L73

But, going down this route, we require passing in an Error type and we probably need to upgrade our class to FuelError<T> to accommodate this.

Call-site could be ugly.

Before

fun <T : Any> responseObject(deserializer: ResponseDeserializable<T>, handler: (Request, Response, Result<T, FuelError>) -> Unit) = response(deserializer, handler)

After

fun <V : Any, E: Any> responseObject(deserializerV: ResponseDeserializable<V>, deserializerE: ResponseDeserializable<E>, handler: (Request, Response, Result<V, FuelError<E>>) -> Unit) 

Then, it looks 😢 😭

Another idea is to make the deserializable of error object become a bit more like "on-demand"

"xxx".httpPost()
    .responseObject(...) { _, _, result ->
        // status code is 422
        result.fold({ items ->

        }, { error ->
             val errorObject = error.deserialize(deseriaizer)
             //then use your errorObject 
        })
    }

So, we add a ability to deserialize into the FuelError class.

[babedev]

I like "on-demand" way. It looks easy to use.

[kittinunf]

Yeah. I think I could find some time this weekend to tackle this

[athasamid]

anyone solve this problem, or there is patch or solution for this issue?

[SleeplessByte]

The solution is in this issue: remove the old response interceptor and add it back with the full range.