disneystreaming / smithy4s

https://disneystreaming.github.io/smithy4s/
Other
339 stars 68 forks source link

0.18.0 SimpleRestJson for http4s Fetch client issue #1245

Closed Quafadas closed 3 months ago

Quafadas commented 9 months ago

I'm having a headscratcher, which I'm beginning to think, might be upgrade to smithy 0.18 related. I have this service - simply defined.

@readonly
@http(method: "GET", uri: "/api/events")
operation GetAllIlsEvents {  
  output: AllIlsEvents
}

I want to call it in browser... through vite's proxy

test1 - http://localhost:8080/api/events - http4s server directly, success

test2 - browser console -

 let res = await fetch('http://localhost:8080/api/events', {method: 'GET'}).then(function (response) {
    console.log(response.text());
  }) ;

suceeds.

test3 - browser console ... but going through vite proxy

let res = await fetch('/api/events', {method: 'GET'}).then(function (response) {
    console.log(response.text());
  }) ;

succeeds.

test4 - http4s fetch client, direct use, compiled to scalaJS in the frontend.

    val vanillaClient = FetchClientBuilder[IO].create
    vanillaClient.get("/api/events")(r => IO.println(r)).unsafeToFuture()

Succeeds.

test5 -

SimpleRestJson
  def eventClient(
      http4sClient: Client[IO]
  ): IlsEventService[IO] =
    SimpleRestJsonBuilder
      .withMaxArity(arity)
      .apply(IlsEventService)
      .client(http4sClient)
      .uri(Uri.unsafeFromString("/"))
      .make
      .fold(throw _, identity)

    Clients.eventClient(vanillaClient).getAllIlsEvents().unsafeToFuture().map { r =>
      println(r)
    }  

Fails (!). I believe there is a difference in browser between the requests. In the browser Network tab The successful "raw" request from the http4s vanilla fetch client looks like; http://localhost:5173/api/events

The request coming out of the SimpleRestJsonBuilder, appears to look like. http://localhost/api/events

I think I claim, that the SimpleRestJson is making the Fetch client "forget" about some part of it's environment... the port perhaps? Is this possible?

daddykotex commented 9 months ago

it definitely seems like the port is missing.

are you talking about 0.18 because you ran this on 0.17 and it was fine? with that said, it's possible there is different behaviour because we've reworked the http internals to make it easier to integrate other http frameworks. It's possible that we use some default for host when we should... not?

Quafadas commented 9 months ago

Based on the discord discussion with @Baccata starting here; https://discord.com/channels/1045676621761347615/1045676622491164724/1159842987610738738

This can worked around with some middleware which massages outgoing requests.

val fetchClient = FetchClientBuilder[IO].create
val frontendClient = Client[IO] { req =>
  val amendedUri = req.uri.copy(scheme = None, authority = None)
  val amendedRequest = req.withUri(amendedUri)
  fetchClient.run(amendedRequest)
}
daddykotex commented 8 months ago

We've found other interesting information that relates to this. If you use org.http4s.Uri.fromString("some.host.example.com"), you'll get a Uri w/o a host:

scala> org.http4s.Uri.fromString("some.host.example.com").right.get.host
val res4: Option[org.http4s.Uri.Host] = None

scala> org.http4s.Uri.fromString("some.host.example.com").right.get.path
val res5: org.http4s.Uri.Path = some.host.example.com

As such, our conversion in smithy4s.http4s.kernel.toSmithy4sHttpUri will fallback to localhost

daddykotex commented 6 months ago

This will be fixed in 0.19

kubukoz commented 3 months ago

Do you mind if we close this? (as it's implemented, just not released due to being on the 0.19 branch)

Baccata commented 3 months ago

Gopher it.