Open quincyjo opened 2 years ago
I'm not sure I understand the problem. Can't you do the following:
trait MyInterpreter extends AkkaHttpServerInterpreter {
override def toRoute(ses: List[ServerEndpoint[AkkaStreams with WebSockets, Future]]): Route =
myDirective1 { myDirective2 { super.toRoute(ses) } }
}
Or maybe even just defining a custom List[ServerEndpoint] => Route
function which would call the interpreter internally? That's what the interpreter really is :)
I have noticed some weird things with route result future mapping when placing directives around the toRoute
call as you did there (which is equivalent to the initial approach). But wrapping the List[ServerEndpoint] => Route
assumes directives are agnostic of the resource. For instance, in tracing each EP has a traced operation based on its method and URI (eg traced ("GET /status"), and it may instead be required to wrap each
ServerEndpoint`.
In your example trait you could of course do:
trait MyInterpreter extends AkkaHttpServerInterpreter {
override def toRoute(ses: List[ServerEndpoint[AkkaStreams with WebSockets, Future]]): Route =
Directives.concat(ses.map { se => myDirective1 { myDirective2 { super.toRoute(Seq(se)) } }}: _*)
}
This is effectively what was used at first just extending AkkaHttpServerInterpreter
and is a good point (I usually avoid super usage I guess so didn't think about that). What actually made me dig into this was that the open tracing directive we wrote makes use of mapRouteResultFuture
to scope the MDC across the ec. It worked before switching to Tapir, but now it seems that the future returned from mapRouteResultFuture
is the future from the server interpreter and can map before the future contained in the server logic of a given ServerEndpoint
has been composed. I was digging into Akka Http to try and figure out maybe a better way to do that but might give more context into the issue being encountered.
Hmm ok I think there's several things here :)
First:
I have noticed some weird things with route result future mapping when placing directives around the toRoute
Can you maybe demonstrate what these weird things are? Maybe that's a manifestation of some other issue which would be good to fix :)
Second, you can use composition instead of inheritance quite easily I think:
class MyInterpreter(delegate: AkkaHttpServerInterpreter) {
def toRoute(ses: List[ServerEndpoint[AkkaStreams with WebSockets, Future]]): Route =
Directives.concat(ses.map { se => myDirective1 { myDirective2 { delegate.toRoute(Seq(se)) } }}: _*)
}
Finally, the problems with the MDC may indicate that we are using the wrong EC somewhere. Again, an example reproducing the problem would be great to see what's really going on :)
AkkaHttpServerInterpreter
is implemented as a trait, and it seems very logical that a consumer might want to implement a custom instance of this in order to meet their needs. For example, in our use case, we want to wrap all EP routes with a directive for logging and a custom directive for handling open tracing support. Instead of having to wrap this in a custom class, it would seem that the proper way to do this would be to provide a customAkkaHttpServerInterpreter
that would generate theRoute
appropriately. While the implementation ofAkkaHttpServerInterpreter
is pretty light, it makes use of several private classes that I think should be usable:AkkaServerRequest
,AkkaRequestBody
, andAkkaToResponseBody
.I would maybe like to make see something like this:
or even
Which would then be used as expected:
This would allow standard route directives to be implemented and interpreted as expected instead of having to wrap Tapir's models in redundant custom ones. Being able to provide custom converters and interpreter construction logic might also be useful.