softwaremill / tapir

Rapid development of self-documenting APIs
https://tapir.softwaremill.com
Apache License 2.0
1.36k stars 418 forks source link

zio-http interpreter: the server logic pattern doesn't work with a list of endpoints #1389

Closed ithinkicancode closed 3 years ago

ithinkicancode commented 3 years ago

Tapir version: 0.18.1

Scala version: 2.13.6

When using zServerLogic and toHttp pattern, if a list of endpoints are passed to toHttp, then the endpoint doesn't respond as if the resource is not defined (404-NotFound).

How to reproduce? The example shown in https://github.com/softwaremill/tapir/blob/master/examples/src/main/scala/sttp/tapir/examples/ZioExampleZioHttpServer.scala. If petServerEndpoint is actually used in the httpApp, then hitting the endpoint would return 404. Once the List is removed at line #34 (https://github.com/softwaremill/tapir/blob/master/examples/src/main/scala/sttp/tapir/examples/ZioExampleZioHttpServer.scala#L34) then it will work. However, passing a list to toHttp is supported and it should be supported as there is a use class for that.

Will provide an example to reproduce.

adamw commented 3 years ago

I'm not sure I understand - when I run ZioExampleZioHttpServer (as in the repository, with a List) - I get a correct response when hitting the endpoint (curl localhost:8080/pet/35), not a 404.

adamw commented 3 years ago

I think this might be fixed by #1388 - indeed there was a problem with multiple endpoints.

adamw commented 3 years ago

Fixed by #1388

ithinkicancode commented 3 years ago

The following snippet should demonstrate the issues with zio-http interpreter:

import sttp.tapir.server.ziohttp.ZioHttpInterpreter
import sttp.tapir.ztapir._
import zhttp.service.{ ChannelFactory, EventLoopGroup, Server }
import zio.{ App, ExitCode, URIO, ZEnv, ZIO }
import zio.console.putStrLn

object HttpServer extends App {

  private val port = 8080
  private val requirements = ChannelFactory.auto ++ EventLoopGroup.auto()

  private val helloEndpoint = endpoint
    .in("hello")
    .in(query[String]("name"))
    .out(stringBody)
    .get
    .zServerLogic[Any] { name =>
      ZIO.succeed(s"Hello $name")
    }

  private val baseStatusEndpoint = endpoint.in("status")

  private val statusHeadEndpoint = baseStatusEndpoint
    .zServerLogic[Any] { _ => ZIO.unit }

  private val statusGetEndpoint = baseStatusEndpoint
    .zServerLogic[Any] { _ => ZIO.succeed("Healthy") }

  private val httpApp = 
    ZioHttpInterpreter().toHttp(helloEndpoint) <>
    ZioHttpInterpreter().toHttp(statusHeadEndpoint) <>
    ZioHttpInterpreter().toHttp(statusGetEndpoint)

  override def run(notUsed: List[String]): URIO[ZEnv, ExitCode] = {
    val app = for {
      server <- Server.start(port, httpApp)
      _ <- putStrLn(s"Web server is running at $port...")
    } yield server

    app
      .provideCustomLayer(requirements)
      .exitCode
  }
}

I have these dependencies:

"dev.zio" %% "zio" % "1.0.10",
"io.d11" %% "zhttp" % "1.0.0-RC17",
"com.softwaremill.sttp.tapir" %% "tapir-core" % tapirVersion,
"com.softwaremill.sttp.tapir" %% "tapir-zio-http" % tapirVersion

With tapirVersion set to 0.18.1, the snippet compiles. However, if tapirVersion is set to 0.19.0-M4, the compilation fails, complaining about overloaded method toHttp with alternatives... cannot be applied to.... Basically the signature doesn't fit any toHttp overloading variants. You can see the error at https://scastie.scala-lang.org/WKOcgFJTQMGZPZ3xJMJLWg.

The second problem is, with tapirVersion set to 0.18.1, update the snippet with this change:

  private val httpApp = 
    ZioHttpInterpreter().toHttp(
      List(
        helloEndpoint,
        statusHeadEndpoint,
        statusGetEndpoint
      )
    )

The code compiles, however, when you run the code, none of the endpoints responds - more accurately all return 404-NotFound.

adamw commented 3 years ago

As for tapir 0.18.x, I don't think the fix has been backported, so that's why it doesn't work.

As for tapir 0.19, the ZTapir integration now has websockets & streams in the required capabilities, so the interpreter must support these. This is the case for the http4s-zio one, but the zio-http interpreter doesn't support websockets (there was a bug in zio-http, which is now fixed, but there hasn't been a new release for a long time, so I think adding websocket support to this interpreter is still not possible). So you'll have to use "normal" .serverLogic to make this work:

import sttp.tapir.server.ziohttp.ZioHttpInterpreter
import sttp.tapir.ztapir._
import zhttp.service.{ChannelFactory, EventLoopGroup, Server}
import zio.{App, ExitCode, RIO, URIO, ZEnv, ZIO}
import zio.console.putStrLn

object HttpServer extends App {

  private val port = 8080
  private val requirements = ChannelFactory.auto ++ EventLoopGroup.auto()

  private val helloEndpoint = endpoint
    .in("hello")
    .in(query[String]("name"))
    .out(stringBody)
    .get
    .serverLogic[RIO[Any, *]] { name =>
      ZIO.succeed(Right(s"Hello $name"))
    }

  private val baseStatusEndpoint = endpoint.in("status")

  private val statusHeadEndpoint = baseStatusEndpoint
    .serverLogic[RIO[Any, *]] { _ => ZIO.unit.map(Right(_)) }

  private val statusGetEndpoint = baseStatusEndpoint
    .serverLogic[RIO[Any, *]] { _ => ZIO.succeed("Healthy").map(Right(_)) }

  private val httpApp =
    ZioHttpInterpreter().toHttp(helloEndpoint) <>
      ZioHttpInterpreter().toHttp(statusHeadEndpoint) <>
      ZioHttpInterpreter().toHttp(statusGetEndpoint)

  override def run(notUsed: List[String]): URIO[ZEnv, ExitCode] = {
    val app = for {
      server <- Server.start(port, httpApp)
      _ <- putStrLn(s"Web server is running at $port...")
    } yield server

    app
      .provideCustomLayer(requirements)
      .exitCode
  }
}

Note that you can also do simply:

ZioHttpInterpreter().toHttp(List(helloEndpoint, statusHeadEndpoint, statusGetEndpoint))
ithinkicancode commented 3 years ago

Thank you very much, Adam. I'll apply the change later today.

ithinkicancode commented 3 years ago

Thank you for the explanation, Adam. By switching to serverLogic, I was able to fix the issues. I hope zio-http will have a new release soon and we can return to using zServerLogic to remove some boilerplate.

And yes, I can confirm passing a List[ServerEndpoint[...]] to toHttp works in 0.19. However, I've found that you can only group endpoints that have the same R. If you want to compose endpoints with different R parameters, you'd have to use the <> combinator to compose the routes together. Luckily, for my case, the <> combinator composes well.

adamw commented 3 years ago

Ah yes, for adjusting the R there's a .widen extension function which extends the environment to a common type. This is unfortunately needed as we need a single effect type.

Interpreting a list of endpoints has some benefits, like proper metrics as we can capture the "start-of-processing" time properly.