-
Notifications
You must be signed in to change notification settings - Fork 420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make asynchronous interceptors easier #1619
Comments
I think it makes some sense to offer a constrained interceptor that does the buffering itself, similar to |
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. |
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. |
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. |
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
The text was updated successfully, but these errors were encountered: