dcapwell commented on code in PR #56:
URL: https://github.com/apache/cassandra-accord/pull/56#discussion_r1296490588
##########
accord-core/src/main/java/accord/messages/WaitOnCommit.java:
##########
@@ -141,13 +146,30 @@ public Void reduce(Void o1, Void o2)
@Override
public void accept(Void result, Throwable failure)
{
- ack();
+ if (failure != null)
+ {
+ while (true)
+ {
+ int initialValue = waitingOnUpdater.get(this);
+ if (initialValue == -1)
+ {
+ logger.error("Had error in WaitOnCommit, but already
replied so can't send failure response", failure);
Review Comment:
```suggestion
node.agent().onUncaughtException(new
IllegalStateException("Had error in WaitOnCommit, but already replied so can't
send failure response", failure));
```
##########
accord-core/src/main/java/accord/local/Node.java:
##########
@@ -475,10 +477,15 @@ public void send(Id to, Request send)
messageSink.send(to, send);
}
- public void reply(Id replyingToNode, ReplyContext replyContext, Reply send)
+ public void reply(Id replyingToNode, ReplyContext replyContext, Reply
send, Throwable failure)
{
- // TODO (usability, now): add Throwable as an argument so the error
check is here, every single message gets this wrong causing a NPE here
- if (send == null)
+ if (failure != null)
+ {
+ checkState(send == null, "fail (%s) and send (%s) are both not
null", failure, send);
Review Comment:
```suggestion
if (send != null)
agent().onUncaughtException(new
IllegalArgumentException(String.format("fail (%s) and send (%s) are both not
null", failure, send)));
```
this situation would currently cause us to loose the real exception
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
