Handle multiple requests for gRPC-Web #1187
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Lukasa
approved these changes
May 19, 2021
|
CI failures are related to #1188 |
Motivation: Our gRPC Web implementation was _somewhat_ underbaked. It would, for example, disregard the 'connection' header and was not able to handle multiple requests. That's pretty bad! Modifications: - Rework part of the grpc-web state machine - Extend the http/2 to grpc state machine and codec to support multiple requests per instance - grpc-web state machine tests Result: gRPC Web is better tested and supports multiple requests.
- setting state to 'notConfigured' meant that we weren't calling 'finish' on the RPC handler - calling 'finish' on the RPC handler is imporant: it breaks a retain cycle - previously we 'finish'ed the handler on channel inactive, now we call it when both the request and the response streams have been closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
Our gRPC Web implementation was somewhat underbaked. It would, for
example, disregard the 'connection' header and was not able to handle
multiple requests. That's pretty bad!
Modifications:
requests per instance
Result:
gRPC Web is better tested and supports multiple requests.