fuel icon indicating copy to clipboard operation
fuel copied to clipboard

FuelError content is empty if "Content-Type" header is not provided by a server

Open liamkernighan opened this issue 4 years ago • 5 comments

Description

FuelError content is empty if Content-Type header is not provided by a server

To Reproduce

import com.github.kittinunf.fuel.core.Request
import com.github.kittinunf.fuel.core.FuelError
import com.github.kittinunf.result.Result


suspend fun <T> Request.requestWithAuthKey(
    clazz: Class<T>,
    permanentKey: String
): Unit {


    val (_, _, fuelResult) = appendHeader("AuthKey" to permanentKey)
        .authentication()
        .basic(userName, password)
        .awaitByteArrayResponseResult()


    when (fuelResult) {
        is Result.Failure -> logError()
        else -> { /* don't care */}
    }
} 

fun Result.Failure<FuelError>.logError()

    val bytes = error.response.data // empty if Content-Type header is missing, OK, if present
    val len = error.response.contentLength; // always OK, number of response's bytes
    val body = error.response.body().asString("text/plain") // empty if Content-Type header is missing, OK, if present
}

Expected behavior

Error content should be extractable no matter whether or not response contains Content-Type header.

Development Machine

Complete the following information if applicable

Windows 10, Kotlin 1.3, Android Studio 3.6.3

Smartphone or Emulator

Xiaomi Redmi Note 8 Pro or Genymotion

Additional context

###Response headers

Date | Mon, 11 May 2020 15:44:59 GMT
-- | --
Server | Apache/2.4.35 (Win64) OpenSSL/1.1.1b PHP/7.2.19
Content-Length | bytes count here
Connection | close
Content-Type | application/json; charset=UTF-8 (or absent)

Response body

{  "testCyrillicValue": "абвгдеж"  }

liamkernighan avatar May 11 '20 16:05 liamkernighan

If Content-Type is not given, we can not extract any content, because we don't know what the content is. We could expose the data stream as bytes or strings or whatever, and then you can use that directly (instead of the .as(xxx)) -- or perhaps allow this behaviour for string content only, because that's generally safe.

SleeplessByte avatar May 11 '20 21:05 SleeplessByte

@SleeplessByte , sorry if I've opened an issue for no reason. The response could be octet/stream or whatever. And there's no any hint for library to guess what it should be. That's understood.

Which I cannot get is below:

my error.response.contentLength is 96 for example. error.response.data is a byte array type. Why it's length is zero instead of 96?

If I try to get my response as stream it's also to no avail because of it's zero length.

So the more precise question is there any way I can get the same result with Fuel?


    val url = URL("http://localhost:8082")

    
    val connection = url.openConnection() as HttpURLConnection
    connection.apply {
        doOutput = true
        requestMethod = "POST"
    }

    if (connection.responseCode != 200) {
        connection.errorStream?.let {
            val bytes = it.readAllBytes() // not empty array is I'm expecting it with Fuel
            println (String(bytes, Charsets.UTF_8)) // I understand that it could be anything. If I get an exception here, that's OK.
        }
    }

liamkernighan avatar May 12 '20 07:05 liamkernighan

If you agree with me that this is an issue, and there is not too much to rewrite, we can discuss about me creating a Pull Request to fix that. (In my opinion, byte array length I get from Fuel should match the content length. Otherwise it's a leaky abstraction)

liamkernighan avatar May 12 '20 08:05 liamkernighan

Content-Length ís a leaky abstraction. You can not use the Content-Length to determine the actual content-length, as soon as you have a chunked, gzip or br or deflate Transfer-Encoding. That's how it's specced up in HTTP standards. Nothing we can do about that. The double downside is that Content-Encoding is often abused as Transfer-Encoding, meaning that usually Content-Encoding also breaks the Content-Length because the server will incorrectly report the "original" size, instead of the encoded size.

Data should not be empty if there really is an error message. That is a 🐛 bug. We would love to fix that 👍 😄

SleeplessByte avatar May 12 '20 12:05 SleeplessByte

Any news on resolution? I'm worried that this problem occurs not only with errors but only with common response. If Content-Type is not provided the response is an empty byte array and so far I have not found a solution for this.

How can I do?

matteobucci avatar Sep 22 '20 14:09 matteobucci