Send meaningful io.grpc.Status.Code to gRPC clients [No. 2] #6088
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
Some API reference & Python bot examples exposed an API bug the Java CLI has been hiding. Due due this Bisq v1 Offer entity design: - In fiat offers, the baseCurrencyCode=BTC, counterCurrencyCode=FiatCode - In altcoin offers, baseCurrencyCode=AltcoinCode, counterCurrencyCode=BTC, new API examples were not doing what the CLI has been doing for several months, which is get (cryptocurrency) all BTC offers from the server, and filter on the altcoin code. The CLI side filtering should have been done on the server, as it is in this commit. A lot of dead gRPC client side offer filtering code is removed as well. Based on `master`.
Exceptions thrown by the core.api services for the daemon Grpc*Services have to be converted into gRPC StatusRuntimeExceptions before being sent to gRPC clients. Most of these gRPC StatusRuntimeExceptions had a gRPC Status.Code.UNKNOWN, which not helpful to client error handlers. This change partially resolves the issue by sending more meaningful io.grpc.Status.Codes to clients, where possible. But it is not as comprehensive as it an be for a webapp because HTTP has so many more possible response status codes than the gRPC library (sixteen). See: https://github.com/grpc/grpc-java/blob/master/api/src/main/java/io/grpc/Status.java There are three types of changes: - Create custom exceptions in bisq.core.api.exception. - Map any custom bisq.core.api.exception to a meaningful io.grpc.Status.Code within daemon Grpc*Service classes. - Adjust apitest cases to new grpc status codes. Based on branch `move-cli-crypto-offer-filter-to-server`, PR bisq-network#6086
ghubstan
added a commit
to ghubstan/bisq
that referenced
this pull request
Mar 7, 2022
I think this bug was introduced when deprecating GrpcOffersService .getMyOffer(id), in favor of using only getOffer(id) for 'my' and 'available' offers. This change explicitly sets the proto's isActivated flag in the OfferInfo factory methods, and adds checks to api offer test cases. Based on branch `2-improve-grpc-exception-status-code-mapping`, PR bisq-network#6088
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.
Exceptions thrown by the Core API services up to the daemon's Grpc*Services have to be converted into gRPC
StatusRuntimeExceptions before being sent to gRPC clients. Most of these gRPCStatusRuntimeExceptions had a gRPCStatus.Code.UNKNOWN, which not helpful to client error handlers.This change partially resolves the issue by sending more meaningful gRPC response status codes to clients, where possible. But it is not as comprehensive as it can be for a webapp because HTTP has so many more possible response status codes than the gRPC library (sixteen). See: https://github.com/grpc/grpc-java/blob/master/api/src/main/java/io/grpc/Status.java
There are three types of changes:
Create custom exceptions in
bisq.core.api.exception.Map any custom
bisq.core.api.exceptionto a meaningfulio.grpc.Status.Codewithin daemon Grpc*Service classes.Adjust apitest cases to new grpc status codes.
Based on branch
move-cli-crypto-offer-filter-to-server, PR #6086