Allow integrating with third-party libraries / frameworks

[adamw]

@frekw has been working on fixing the zio-http integration with tapir (in softwaremill/tapir#2718), however due to the changes done in #1916 we have encountered some serious roadblocks.

The main problem is that in zio-http routing and handling is now separate, while tapir doesn't have such concepts. And since some middlewares are applied only after the Router has computed the Handler, we can either provide an inefficient tapir integration (which is what @frekw PR implements), or an efficient, but broken one (which is undesirable).

To separate routing and handling in tapir as required by zio-http, we have to perform input decoding twice. Once to check, if an endpoint matches the request. Once this is done, and we know that an endpoint matches, we run the decoding again, to actually handle the request. This is because in tapir, we decode the endpoint's inputs, and if they decode successfully, we know that the endpoint matches the request, and the decoded values are fed into the server's logic. If there's any decoding failure (such as a missing parameter, wrong arity, or any exception), the endpoint doesn't match.

Btw.: the same inefficiency is required for the Play interpreter, which unfortunately allows integrating only through a PartialFunction[Request, Response]. If they exposed a Request => Option[Response] we would be able to do that in one go. All other interpreters don't have this limitation (except zio-http and Play). This also forces us to disable some tapir functionalities (such as returning Method Not Allowed responses when appropriate)

A possible integration point of running the handler middlewares is by providing a custom EndpointHandler.onDecodeSuccess method. This is called when an endpoint's input are successfully decoded (so we know that the endpoint matches), and the last handler in the chain then runs the server logic. So if we could somehow extract the middlewares and run them there, maybe we could work around the inefficiencies. But I have no idea if that's plausible. But maybe there are some better design options for zio-http which would allow us to integrate.

And finally for completeness - this used to work fine with the previous versions of zio-http.

I were to frame this is a feature request, I'd say that tapir would like to be responsible for both routing & handling requests that much any of its endpoints (if no endpoints match, then tapir's interpreter returns an Empty handler), while still making sure that the specified middlewares/aspects are run.

[vigoo]

Thanks for raising the issue @adamw! As I never had the chance to use tapir I'm not fully understanding the issue. I will check @frekw 's implementation to get a better idea but could you maybe write a specific example of the problem? A zio-http Http composed in the way how the inefficient tapir integration would do. I think it would help the discussion.

[adamw]

@vigoo Ok, maybe I'll start with our problem statement.

We have a List[Endpoint], where each Endpoint instance describes a single tapir endpoint. The interpreter's task is to convert this into some value that can be attached to the target server instance and composed with other server-native routes. Typically the output of an interpreter is some form of an Request => Option[Response] function, where if the response is not defined for a request, subsequent (tapir-generated or "native") routes are attempted.

In our current implementation, the zio-http interpreter returns an HttpApp[R, Throwable] instance. The problem is how this can be constructed. The structure of your Route enforces (as far as I understand) that you have to decide, before handling the request, and running any logic, if the request matches the endpoint. Such a check is not possible in tapir without cost: we have to first decode the inputs (which determines if the request matches the endpoint) without running the logic; once this is successful, the returned Handler then does the same thing again.

Why not simply return a Handler? As then, aspects (middleware) which are designed to short-circuit a request, allow the logic to always run (see softwaremill/tapir#2708). I think the gist of the problem is what you've described in #1975: tapir does routing & handling as a single step.

[vigoo]

I'm sure I'm missing something, but let's consider this simple example:

object Test extends ZIOAppDefault {

  sealed trait Endpoint
  object Endpoint {
    case object A extends Endpoint
    case object B extends Endpoint

    def fromRequest(request: Request): ZIO[Any, Nothing, Option[Endpoint]] = { // possibly effectful
      // Custom determining which endpoint to call (and decoding everything needed to do so)
      ZIO.succeed {
        request.url.path match {
          case !! / "a" => Some(A)
          case !! / "b" => Some(B)
          case _        => None
        }
      }.debug("fromRequest")
    }

    def toResponse(endpoint: Endpoint): ZIO[Any, Nothing, Response] = // possibly effectful
      ZIO.succeed {
        endpoint match {
          case A => Response.text("A")
          case B => Response.text("B")
        }
      }.debug("toResponse")
  }

  val app: HttpApp[Any, Nothing] =
    Http.fromOptionalHandlerZIO { request =>
      Endpoint.fromRequest(request).flatMap { // effectful routing
        case Some(endpoint) =>
          ZIO.succeed {
            Handler.fromZIO {
              Endpoint.toResponse(endpoint)
            }
          }

        case None =>
          ZIO.fail(None) // not handling this request
      }
    }

  val appWithMiddlewares = app @@ Middleware.runBefore(ZIO.debug("Before")) @@ Middleware.runAfter(ZIO.debug("After"))

  def run =
    appWithMiddlewares.runZIO(Request.get(URL(!! / "b"))).flatMap(_.body.asString).debug("Response: ")
}

You have a potentially effectful function that takes a Request and can decide which endpoint it matches, and can also "fail with None" which means this request cannot be handled.

Once you have a match you can have any custom data prepared in that phase, and have a Handler in which, although is an effectual Request => Response function, you are not forced to recalculate (decode) all the necessary inputs - as you write both the routing and the handler you can just create a separate handler for each endpoint and pass the already available information to them as in the above example.

Now I can imagine that composing such a Http from the interpreter is a bit harder but I don't see yet what's missing from the new Http/Handler types to do it.

[vigoo]

Running the above example outputs:

fromRequest: Some(B)
Before
toResponse: zio.http.Response$BasicResponse@6bd24378
After
Response: : B

which is the desired sequence of evaluation in my opinion (at least that's what we tried to achieve with the 0.0.4 changes :)

[adamw]

@vigoo Ah! :) In your example, routing and request handling is separate: first there's a "pick endpoint" stage, then there's a "run endpoint logic" stage. This does happen in tapir, of course, but this is all part of tapir's own "request executor". The below roughly reflects tapir's model:

import zio.http._
import zio.{UIO, ZIO, ZIOAppDefault}

object Test extends ZIOAppDefault {
  case class TapirRequest(r: Request)
  case class TapirResponse(v: String)

  object TapirInterpreter {
    def apply(r: TapirRequest): UIO[Option[TapirResponse]] = ZIO.succeed {
      if (r.r.path.toString().startsWith("/a")) Some(TapirResponse("A")) else None
    }.debug("TapirInterpreter")
  }

  val app: HttpApp[Any, Nothing] =
    Http.fromOptionalHandlerZIO { request =>
      TapirInterpreter(TapirRequest(request)).flatMap {
        case Some(value) => ZIO.succeed(Handler.succeed(Response.text(value.v)))
        case None        => ZIO.fail(None)
      }
    }

  val appWithMiddlewares = app @@ Middleware.runBefore(ZIO.debug("Before")) @@ Middleware.runAfter(ZIO.debug("After"))

  def run =
    appWithMiddlewares.runZIO(Request.get(URL(!! / "a"))).flatMap(_.body.asString).debug("Response: ")
}

output:

TapirInterpreter: Some(TapirResponse(A))
Before
After
Response: : A

The TapirInterpreter class is fixed and we can't do much do change it. Do you think it is still possible for tapir to integrate with zio-http given its changes?

[vigoo]

Thanks I think I fully understand the issue now.

Based on all this, it feels like what tapir generates should be a Handler - as it is fully responsible for producing a response from a request, including the routing. But with that we loose composability of the tapir-managed endpoints with other zio-http apps. If this is something people usually do, then this is not an option.

As you mentioned earlier this could be "fixed" by making handlers able to "not handle" the request, so making them a Request => Option[Response]. We cannot do that, because it would bring back the same problem this whole refactoring (#1916) was intended to fix:

For example if you have:

val app = (app1 @@ auth) ++ app2`

If app1 is a single handler which can fallback (return with None), then the application of the auth middleware leaks out. If auth prevents access, the handler that returns None will never run, so the auth middleware will be implicitly applied to app2 as well, even though we don't want that. The separation to Http and Handler and that "handler middlewares" are applied after the routing are fixing this.

So adding back the Option[Response] on the handler level is not an option.

That said we could simulate it by failing with a specific ZIO.die(TapirUnhandled) within the handler, if we would readd a catchAllCause on the Http level. But that sounds quite hacky, implementing that catchAllCause is not that straightforward (but possible), and it would have the same defect I described above related to middleware application.

So I took a look at https://github.com/softwaremill/tapir/blob/b3d71468c9/server/zio-http-server/src/main/scala/sttp/tapir/server/ziohttp/ZioHttpInterpreter.scala

If I understand it correctly, the problem is that DecodeBasicInputs has to be done once for the routing, and then ServerInterpreter performs the same decoding again in the handler. It seems to me that on Tapir side these input decoding step (https://github.com/softwaremill/tapir/blob/master/server/core/src/main/scala/sttp/tapir/server/interpreter/ServerInterpreter.scala#L88-L95) could be decoupled and exposed separately (first compute them for each endpoint - possibly lazily) and then just get the decoded inputs in the tryServerEndpoint from that. That way some implementations could precalculate the inputs and others could use a default single apply that would do the same as today (by directly passing the decoded mapping to the rest of the logic). But I understand you probably don't want to do this just for better supporting zio-http (and I'm sure I missed a lot of details as I just quickly looked at the code).

Other than the above options I don't have any better idea at the moment.

[adamw]

Thanks for the explanation - now the problem & your solution are much clearer :) Indeed I had the same problem in tapir, where I wanted to run the security logic only once, and only when I'm sure that the endpoint shape matches the request. This is done indeed in the ServerInterpreter section that you pointed to.

I'd prefer to avoid refactoring the entire ServerInterpreter just for the sake of zio-http (it already works with all other server implementations), but maybe there's a middle ground. Tapir has an option to pre-filter the endpoints that might match the request (see FilterServerEndpoints), using the requests path. Maybe this can be done as part of the routing. Then, we can be "almost" sure that if there are endpoints matching the path shape, tapir will somehow handle the request. This excludes some esoteric setups, where subsequent zio-http handlers would handle the same path shapes as tapir, so we'd have to document that clearly.

@frekw what do you think? Normally we pass the filtering as a parameter to ServerInterpreter, here that step would be done before that in the Route. Additionally, if tapir doesn't provide any response (success or error) given a list of matching endpoints, we'd have to return a failed effect (saying that such setups are not possible).

That way we would avoid any duplicate invocations of DecodeBasicInputs.

As a side-note, maybe tapir v2 should adopt some of the ideas here, defaulting to handling the request with an endpoint if the path shape matches. Plus separating the routing & handling into separate classes so that we could integrate with play/zio-http more easily.

[frekw]

@adamw I think that sounds like a very reasonable tradeoff!

I may not be in the best position to judge this, as I'm not a big user outside of Caliban, but all examples I've seen using Tapir + zio-http either just lets Tapir handle everything or uses zio-http's own routing (via e.g collectRoute), and I think that would work well for both those cases (perhaps at the loss of some power for more advanced / esoteric use cases).

Not sure I'm familiar enough with Tapir to make the change, but I can dig around a bit!

[jdegoes]

@adamw Thanks for reporting this issue! ZIO HTTP has lots of happy Tapir users and we want to keep it that way, so as we discuss with @vigoo and @frekw, I just want to emphasize we will invest significantly to ensure high-quality integrations with Tapir and other libraries and frameworks remains both possible and affordable.

[adamw]

@jdegoes thanks, though I think in the end there's no problem on zio-http's side, rather my lack of understanding of the design & rationale of the architecture change. Likewise ZIO users are active participants on the tapir/sttp communities so I'd like to serve them well, and I think it should be possible using the approach described above. When I get back from vacation next week I'll try to translate it to code :)

[adamw]

The tapir side is now implemented, no changes required on zio-http side, so sorry for the noise, but thanks for the explanations :)

softwaremill/tapir#2747