Add the dapr runtime returned error details to the Java DaprException #998
Conversation
…Exception Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
|
@cicoyle - Are you doing this for v1.13? |
| } catch (DaprException exception) { | ||
| System.out.println("Error code: " + exception.getErrorCode()); | ||
| System.out.println("Error message: " + exception.getMessage()); | ||
|
|
||
| try { |
There was a problem hiding this comment.
This is too complex for users. Can we have an opinionated structure that encapsulates this into the DaprException object?
There was a problem hiding this comment.
See the latest version with the "opinionated" layer I added.
Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Artur Souza <[email protected]>
| /** | ||
| * Object mapper to parse DaprError with or without details. | ||
| */ | ||
| private static final ObjectMapper DAPR_ERROR_DETAILS_OBJECT_MAPPER = new ObjectMapper(); |
There was a problem hiding this comment.
Clarifying my change: no need for a custom ObjectMapper. The stock one is enough now.
| result.put("@type", generateErrorTypeFqn(message.getClass())); | ||
|
|
||
| for (Field field : fields) { | ||
| if (field.isSynthetic() || Modifier.isStatic(field.getModifiers())) { |
There was a problem hiding this comment.
Clarifying my change: This avoids the need to check for _ in the name.
| continue; | ||
| } | ||
|
|
||
| String normalizedFieldName = field.getName().replaceAll("_$", ""); |
There was a problem hiding this comment.
Since the code still contains _, we need to remove it. If the library decided to stop appending the _ to the attribute names, we would not need to change this code.
| } | ||
|
|
||
| public enum ErrorDetailType { | ||
| ERROR_INFO, |
There was a problem hiding this comment.
Clarifying my change: Purposely mapping to our internal types to avoid exposing types from the gRPC library to the app.
| return Collections.unmodifiableMap(result); | ||
| } | ||
|
|
||
| private static <T extends com.google.protobuf.Message> String generateErrorTypeFqn(Class<T> clazz) { |
There was a problem hiding this comment.
Clarifying my change: just a small refactoring to allow this logic to be reused.
| private static final Map<Class<? extends Message>, ErrorDetailType> SUPPORTED_ERROR_TYPES = | ||
| Collections.unmodifiableMap(new HashMap<>() { | ||
| { | ||
| put(com.google.rpc.ErrorInfo.class, ErrorDetailType.ERROR_INFO); |
There was a problem hiding this comment.
Clarifying my change: avoids exposing 3rd-party library types to the app.
| } | ||
| }); | ||
|
|
||
| private static final Map<String, Class<? extends Message>> ERROR_TYPES_FQN_REVERSE_LOOKUP = |
There was a problem hiding this comment.
Clarifying my change: handy to map a string to a class when parsing JSON error response in HTTP API.
| try { | ||
| client.getState("Unknown state store", "myKey", String.class).block(); | ||
| client.publishEvent("unknown_pubsub", "mytopic", "mydata").block(); |
There was a problem hiding this comment.
Clarifying my change: keeping this example simple.
| exception.printStackTrace(); | ||
| System.out.println("Dapr exception's error code: " + exception.getErrorCode()); | ||
| System.out.println("Dapr exception's message: " + exception.getMessage()); | ||
| System.out.println("Dapr exception's reason: " + exception.getStatusDetails().get( |
There was a problem hiding this comment.
Clarifying my change: this is the opinionated layer in the SDK and determines how users can retrieve error details. We don't enumerate all the attributes on purpose. We can decide to change this later but I think this is a good first design to give a semi-structured way to fetch error details.
Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Artur Souza <[email protected]>
artursouza
dismissed
their stale review
I redesigned to add an opinionated layer.
Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Artur Souza <[email protected]>
Signed-off-by: Artur Souza <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #998 +/- ##
============================================
+ Coverage 77.42% 77.56% +0.13%
- Complexity 1539 1567 +28
============================================
Files 143 144 +1
Lines 4642 4751 +109
Branches 541 554 +13
============================================
+ Hits 3594 3685 +91
- Misses 771 781 +10
- Partials 277 285 +8 ☔ View full report in Codecov by Sentry. |
Description
Add the error details from dapr runtime to the java dapr exception, such that users may parse for error details in their code accordingly.
Here is the output from running the PubSub Client code:
Issue reference
dapr/docs#3907 (comment)
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: