Improve how the test-suite deals with exceptions #91
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
See documentation of `sendGrpcException` for details.
While the output is nicer than `Show`, the package is very incomplete, and it was making maintaining/updating the test suite more cumbersome than it should be.
This is the first step towards making exception handling in the test suite more surgical: rather than collecting exceptions at the top-level, we verify _at the source_ whether or not they are expected, and if so, don't allow them to bubble up at all. This means that we can then stop the test suite at the first (unexpected) exception. This will make the tests easier to understand and setup: for example, as things stand, when one client fails, the other clients are killed, but that will then result in further exceptions in those clients. This doesn't get us all the way there yet, but it makes some progress towards this goal.
This is now done locally. The only thing left now is the compression negotation failures.
The client API now has a `CallSetupFailure` just like the server API does. We no longer allow the client to _insist_ on compression; see documentation of `Network.GRPC.Common.Compression.Negotation.choose`.
In the process, this makes a few other improvements: * The client can now insist on an initial compression algorithm, before compression negotation has taken place (`connInitCompression`) * The server can insist on a particular compression algorithm, independent of what the client offers (`insist`) * When the server returns a HTTP status other than 200, we include the response body in the exception (this might help with #22, specifically #22 (comment))
The TLS failures are now handled the same way as the rest. This now means that once we see an exception bubble up from either a client or a handler, that exception really is a test failure, and we should stop the test.
This paves the way for the next step.
|
Verified that this still passes all interop tests (grapesy server, reference client in Java, C++, Go, and Python). |
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.
This also makes a few improvements to the make library itself:
CallSetupFailureexception if the call cannot be established (though they use different definitions).connInitCompression)insist); seeNegotationfor details.insist)GrpcExceptionin the client by callingsendGrpcExceptioninstead by throwing this exception themselves; seesendGrpcExceptionfor discussion for the trade-off.