You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ratis.apache.org by GitBox <gi...@apache.org> on 2020/05/05 09:33:42 UTC

[GitHub] [incubator-ratis] runzhiwang opened a new pull request #87: RATIS-920. Fix error use of onCompleted in onError

runzhiwang opened a new pull request #87:
URL: https://github.com/apache/incubator-ratis/pull/87


   What's the problem ?
   When run test, a lot of exception was thrown, as the image shows.
   ![image](https://user-images.githubusercontent.com/51938049/81052182-953a9e80-8ef5-11ea-8661-70d5bdd1751b.png)
   
   What's the reason ?
   When run into onError, the call has already been cancelled, there is no need to call onCompleted. And there is [a similar problem](https://github.com/scalapb/ScalaPB/issues/71) in other project.
   You can find why the exception was thrown in the following code, copied from [gRPC].(https://github.com/grpc/grpc-java/blob/master/stub/src/main/java/io/grpc/stub/ServerCalls.java).
   ```
       @Override
       public void onCompleted() {
         if (cancelled) {
           if (onCancelHandler == null) {
             throw Status.CANCELLED.withDescription("call already cancelled").asRuntimeException();
           }
         } else {
           call.close(Status.OK, new Metadata());
           completed = true;
         }
       }
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-ratis] lokeshj1703 commented on pull request #87: RATIS-920. Fix error use of onCompleted in onError

Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on pull request #87:
URL: https://github.com/apache/incubator-ratis/pull/87#issuecomment-625780849


   @runzhiwang Thanks for the contribution! I have committed the PR to master branch.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-ratis] runzhiwang commented on pull request #87: RATIS-920. Fix error use of onCompleted in onError

Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #87:
URL: https://github.com/apache/incubator-ratis/pull/87#issuecomment-625717688


   @lokeshj1703  Thanks for review. The suggestion does work. I have updated.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-ratis] lokeshj1703 commented on pull request #87: RATIS-920. Fix error use of onCompleted in onError

Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on pull request #87:
URL: https://github.com/apache/incubator-ratis/pull/87#issuecomment-625692880


   @runzhiwang Thanks for working on this! I think this scenario will occur when client cancels the call. How about we extract the status in onError(using Status.fromThrowable(Throwable)) and check if it is cancelled. If it is cancelled we do not make the onCompleted call. Let's see if that solves the problem.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org