grpc / grpc-swift

The Swift language implementation of gRPC.
Apache License 2.0
2.01k stars 416 forks source link

Make asynchronous interceptors easier #1619

Open weissi opened 1 year ago

weissi commented 1 year ago

Currently, if you use a Server/ClientInterceptor you need to manually buffer if any of your processing is asynchronous. This is very error prone, hard to test and actually also causes back-pressure issues.

I think an interceptor should actually only be called once it's done processing the previously intercepted message part. Are there really many use cases where you need to see the next part before having fully processed the previous one?

Related #1618 / #1620

Lukasa commented 1 year ago

I think it makes some sense to offer a constrained interceptor that does the buffering itself, similar to ByteToMessageDecoder. But it is useful to give access to the unvarnished stream, it grants more capabilities to the interceptors.

FranzBusch commented 1 year ago

IMO if we are going to make the interceptors fully async this problem should go away and we should just be able to forward the async sequences.

weissi commented 1 year ago

Right, if we model interceptors as async sequences this would be solved. This is very easily doable in the futures world too though, unclear if required.

FranzBusch commented 1 year ago

Right, if we model interceptors as async sequences this would be solved. This is very easily doable in the futures world too though, unclear if required.

I let @glbrntt speak to that in more detail but there are a bunch of improvements that we have discussed for the current interceptors and a new async approach would solve those. Furthermore, the SSWG is currently building out a common middleware protocol which hopefully can be used here as interceptors as well.

glbrntt commented 1 year ago

We have some (tentative) plans to do a bunch of work on grpc-swift... part of that would be making grpc-swift 'async first' and not exposing futures in the interface (where possible). This would include interceptors because they are a particular pain point. As Franz notes, this could be using a common middleware protocol, but we'll see.

I'd rather we held off on this for now until we have a better idea of what our plans are.