Semantics of `Future<Boolean>` for persistence

[ajs6f]

If a persist call (that returns Future<Boolean>) is completed unexpectedly (in my example in hand, an InterruptedException which does not indicate failure of persistence, but failure of the thread monitoring persistence, since the actual persistence is happening elsewhere on the network) should true or false result?

IOW, if the backend cannot give us definite information about the completion, what should it report forward?

I don't want to get into Future<Optional<Boolean>> weirdness. But maybe an enum Completion {Success, Failure, Unknown} or the like and return Future<Completion>? Or maybe there's a better way to say the same thing within the Java concurrency APIs…

[acoburn]

I agree that way too much information is packed into that true and false. The enum you suggest is already way better than true/false. I'd be entirely supportive of moving toward the use of such a data structure.

[ajs6f]

Ok, lemme see if there is anything better straight from the Java APIs, and I'll send a PR in a few days (after reviewing #54 !).

[ajs6f]

Question: is it better to throw a runtime exception in this situation? I don't know what (at the HTTP layer) a Unknown state could result in other than a 5xx anyway…

[acoburn]

Throwing a runtime exception (e.g. TrellisRuntimeException) seems to make sense. Would you want to catch the exception and log it? Such as what's going on here:

trellis/trellis-http/src/main/java/org/trellisldp/http/impl/PatchHandler.java

Line 234 in 421bc4a

LOGGER.error("Error persisting data: {}", ex.getMessage());

Those exceptions fall through to a 5xx error without displaying any ugly stack traces to a client.

[ajs6f]

So, if I get you, you're saying that when we have that weird condition (can't prove persistence was either successful or unsuccessful) then throw an appropriate exception out of the backend and the HTTP layer will do the right thing with it, given the code that is already in place?

[acoburn]

The code that's currently in place will catch any ExecutionException, so if you throw a runtime exception in those mutating methods, the HTTP layer will already do the right thing.

[ajs6f]

Okay, let's play it that way for now. I hate to say it, but (channeling my inner Andy Seaborne) it is the more Java-idiomatic move here, I think.

[ajs6f]

Closed as per discussion above.