Change Actor gRPC client to async.#564
Conversation
Codecov Report
@@ Coverage Diff @@
## master #564 +/- ##
============================================
+ Coverage 80.52% 80.61% +0.09%
- Complexity 981 985 +4
============================================
Files 88 88
Lines 3024 3039 +15
Branches 331 332 +1
============================================
+ Hits 2435 2450 +15
+ Misses 423 422 -1
- Partials 166 167 +1
Continue to review full report at Codecov.
|
| if (daprApiToken != null) { | ||
| metadata.put(Metadata.Key.of("dapr-api-token", Metadata.ASCII_STRING_MARSHALLER), daprApiToken); |
There was a problem hiding this comment.
Is there a negative case if the token is empty? Or is this related to the feature that has to be enabled and if it's not we hit the null (expected) case.
There was a problem hiding this comment.
We hit null if not set. If it is set as empty, it is OK to pass it through.
halspang
left a comment
halspang
left a comment
There was a problem hiding this comment.
Overall looks good to me. Just a quick question on how the token works.
| ExecutionException.class, | ||
| "UNKNOWN", | ||
| "UNKNOWN: java.lang.ArithmeticException", | ||
| "UNKNOWN: ", |
There was a problem hiding this comment.
This has to do with how Exceptions are handled in the grpc library, it does not show the exception as it would in the blocking path.
| callWithRetry(() -> { | ||
| assertThrowsDaprExceptionSubstring( | ||
| "INTERNAL", | ||
| "INTERNAL: error invoke actor method: error from actor service", |
There was a problem hiding this comment.
When testing as an integration test, we got the error message, but the original exception is not displayed.
There was a problem hiding this comment.
This behavior is observed prior to this change as well.
There was a problem hiding this comment.
Seems like a fine error to me. Just curious, would the actual error be displayed anywhere? For instance, in the dapr runtime?
There was a problem hiding this comment.
It should be logged in Dapr runtime. The whole error handling for dapr is something that is not proper designed in the APIs today.
Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
3 participants
Description
Change Actor gRPC client to async.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #562
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: