twitter / finagle

A fault tolerant, protocol-agnostic RPC system
https://twitter.github.io/finagle
Apache License 2.0
8.78k stars 1.45k forks source link

com.twitter.finagle.Http not work #955

Open samthebest opened 1 year ago

samthebest commented 1 year ago

Describe the bug

I try to call a simple endpoint, which I can do easily in another library and using curl, but with finagle http I get:

Exception in thread "main" Failure(null at remote address: REDACTED/REDACTED:443. Remote Info: Not Available, flags=0x08) with RemoteInfo -> Upstream Address: Not Available, Upstream id: Not Available, Downstream Address: REDACTED/REDACTED:443, Downstream label: REDACTED:443, Trace Id: 4c9d19804cb9d6f7.83e43b9c049014cd<:e043d2f9c8705e82 with Service -> REDACTED:443
Caused by: com.twitter.finagle.ConnectionFailedException: null at remote address: REDACTED/REDACTED:443. Remote Info: Not Available
    at com.twitter.finagle.netty4.ConnectionBuilder$$anon$1.operationComplete(ConnectionBuilder.scala:105)
    at com.twitter.finagle.netty4.ConnectionBuilder$$anon$1.operationComplete(ConnectionBuilder.scala:84)
    at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:590)
    at io.netty.util.concurrent.DefaultPromise.notifyListenersNow(DefaultPromise.java:557)
...

As stated I know the server is available because I can hit it using other libraries, or curl.

To Reproduce

  private val client: Service[Request, Response] =
    Http.client.withTls(s"$host").newService(s"$host:443")
  //  Http.client.newService(s"$host:443") not work either

   Future
      .apply(logger.log(s"Calling URI: $fullUri"))
      .flatMap(_ =>
        client.apply(
          RequestBuilder()
            .url(fullUri)
            .addHeader(Fields.Authorization, s"Basic ${basicAuth.key}")
            .buildGet(),
        ),
      )

Expected behavior

It behaves like any other library or curl, i.e. hits the endpoint and returns a response.

Also I don't understand why we need to specify the host 3 times (twice to create the service, once in the URI)

samthebest commented 1 year ago

After a little playing around, realised something fishy has to be going on with sbt also, because we cannot reproduce the same error by trying different build files!

...

going to try to find a minimal build file to reproduce

samthebest commented 1 year ago

Here is a minimal build file that reproduces the error ... I've put comments in the build file to show that if you comment/uncomment certain lines the issue goes away

https://github.com/samthebest/reproducing-finagle-bug/tree/master

So in this project you can do:

sbt
project hvDomain
console

then paste in

import com.twitter.finagle.{Http, Service}
import com.twitter.finagle.http.{Request, RequestBuilder, Response}
import com.twitter.util.{Await, Future}

val client: Service[Request, Response] = Http.client.withTls("REDACTED").newService("REDACTED:443")
val req = Request("REDACTED", "start" -> "2023-08-10", "end" -> "2023-08-10")
Await.result(client.apply(req))
csaltos commented 5 months ago

Just send the hostname in the request for comply with the expected HTTP protocol with something like:

import com.twitter.finagle.Http
import com.twitter.finagle.http

val client = Http.client.withTls("myserver.com").newClient("myserver.com:443")
val requestBuilder = http.RequestBuilder().url("https://myserver.com/").addHeader(http.Fields.Host, "myserver.com")
val req = requestBuilder.buildGet() // or buildPost or the one you need.
client.toService.apply(req)

Finagle does not send any explicit HTTP host headers for covering more possible corner cases requests ... but just add a call to addHeader with the hostname and it should work.

Additionally is recommended to pass the http.Fields.UserAgent header and any other header information for following a standard HTTP call.