jsondb: do not discard a rollback failure that follows a failed commit

[lagergren]

Fixes #567.

When a commit fails, Client.commit() runs a compensating rollback and throws away anything that rollback raises:

} catch (Exception e) {
    log($"Exception during commit of {this}: {e}");
    result = DatabaseError;
    try {
        txManager.rollback(writeId_);
    } catch (Exception ignore) {}     // ← the secondary failure disappears
}

The commit failure is logged. The rollback failure is not.

Why the second failure is the one worth keeping

These two outcomes are not equivalent, and the current code cannot tell them apart.

A commit that fails and rolls back cleanly is handled: the transaction did not happen, and DatabaseError describes it accurately. A commit that fails and whose rollback also fails is different in kind — the client no longer knows the transaction's disposition, and it may be partially applied. Both return DatabaseError, so a caller sees the same thing either way.

TxManager owns recovery and this change does not touch that. But the secondary failure is exactly the evidence that tells a health check or a recovery path that recovery is needed, and it was being discarded at the only point where it is observable.

The failure mode is silence — no log line, no exception, no signal. If it happens, the first symptom is inconsistent data with nothing in the log to explain it.

The change

One catch. Control flow and the returned CommitResult are unchanged; the rollback failure is logged with the original commit failure as context, so both are visible together:

log($"Exception during rollback after failed commit of {this}: {e2} (commit failure: {e})");

A note on #541, since this adds a log() call

#541 documents that Catalog.log() does @Inject Console on every call, and that injector contention under concurrent load can form a permanent wait cycle. This change adds a log() on a path that reaches it.

I do not think it is a meaningful contribution to that pressure: Client.log dispatches asynchronously (catalog.log^(msg)), so the committing fiber does not block on it, and this line only runs when a commit and its compensating rollback have both failed. But it is worth naming rather than leaving for a reviewer to notice, and #541's own first recommendation — caching the injected Console per Catalog service instance — would remove the question entirely.

Testing, stated honestly

JsondbClientRollbackFailureTest fails on unmodified master and passes after:

org.opentest4j.AssertionFailedError: rollback failure after commit failure
must not be swallowed ==> expected: <false> but was: <true>

It is a source-shape test — it reads Client.x off disk and asserts on the shape of the error path, rather than provoking a real rollback failure. I would rather say that plainly than let it read as a behavioural reproduction. Inducing a genuine TxManager.rollback failure underneath a genuine commit failure needs fault injection that does not exist in this codebase today; the defect is a discarded exception whose absence is not otherwise observable. :javatools:test goes from 348 to 349 tests with the same 40 pre-existing environment-gated skips, and xdk:installDist compiles lib_jsondb clean.

Not included

There are nine other catch (… ignore) sites across lib_json, lib_jsondb/tools, lib_web, lib_xenia, lib_xunit_engine and manualTests. They are unrelated to this path and would be a separate tidy — the house idiom for a genuinely unused catch variable is catch (X _), which Client.x already uses at line 218.

[ggleyzer]

Marcus, did you write this test? I don't think so.

My next question is: did you read this test?

[ggleyzer]

A commit that fails and whose rollback also fails is different in kind — the client no longer knows the transaction's disposition, and it may be partially applied

I don't think this is a correct statement. A failed rollback doesn't leave a partially "applied" transaction. The only value in logging the exception is for developer's sake