Body of request can "go missing" when constructed using the apply method of Request
bastewart opened this issue · 4 comments
http4s
version 0.20.0-M4
.
I have created a repo to demonstrate this. I'm on gitter if anything needs an explanation and it would clutter here. https://github.com/bastewart/http4s-body-missing-test
I think this may have the same cause as http4s/http4s#2346 because when the body goes missing the request logger also does not log properly.
Possibly something is (in a non-functional way) consuming the Stream
that represents the request body? Enabling the request logger for OkHttp
leads to an error caused by reading the response body twice (see README
of my repo) which also indicates non-functional consumption.
In a nutshell when I construct a request "manually" (apply
method of Request
) then the body does not get sent by the client if the implementation is Async
or OkHttp
. It does work for Jetty
and Blaze
though. By manual I mean like this:
def requestManual(uri: Uri, body: Json): F[String] = {
val client: Client[F] = ???
for {
hBody <- http4sBody(body.noSpaces)
request = Request(method = Method.POST, uri = uri, headers = Headers(`Content-Type`(MediaType.application.json)), body = hBody)
response <- client.expect[String](request)
} yield response
}
def http4sBody(body: String)(implicit encoder: EntityEncoder[F, String]): F[EntityBody[F]] =
F.pure(encoder.toEntity(body).body) // I have tried this as F.delay and it does not affect the result
I know it's weird to send a request in the above method, I've cut down some project code to end up with this. The fact that it works for Jetty
and Blaze
makes me think while weird it's not necessarily wrong.
The same request using the DSL works under all client implementations (not with client logger enabled using OkHttp
though, see README
):
def requestDsl(uri: Uri, body: Json): F[String] = {
val client: Client[F] = ???
client.expect[String](POST(body, uri))
I think this (and http4s/http4s#2346) is caused because I don't specify the content-length manually and the fact that OkHttpBuilder
and AsyncHttpClient
blindly trust this (so use an empty body builder). Blaze
at least (I haven't chased the Jetty
code through) uses the actual request body to build the header (and the body).
I'm also having this problem with OkHttp. I worked around it by providing the body in a separate request.withBodyStream(...)
call - that also works.
Sorry correction - it appears that it's in fact the addition of content-length
header, as you suggested, that fixed this, not withBodyStream
as I said previously. :(
This seems like something that would be cleaned up by http4s/http4s#4509.