Skip to content

fix(jdbc): propagate rollback failures and keep transaction state - #58

Merged
magnusp merged 1 commit into
mainfrom
fix/30-rollback-failures
Oct 21, 2026
Merged

magnusp merged 1 commit into
mainfrom
fix/30-rollback-failures

Conversation

@magnusp

@magnusp magnusp commented Oct 21, 2026

Copy link
Copy Markdown
Owner

Closes #30. Part of #54.

What changed

  • commit() and rollback() now delegate to one shared endTransaction path, matching xerial 3.53.4.0 SQLiteConnection.java:435-451.
  • A StreamResult.Error from COMMIT/ROLLBACK, or a transport LibsqlException, is mapped to SQLException instead of being ignored.
  • The baton is left in place when the transaction command fails, so the caller can retry against the same transaction stream.
  • Added LibsqlConnectionTest covering server and transport failures for both COMMIT and ROLLBACK.
  • Added a Toxiproxy integration test that breaks the connection during rollback(), asserts the SQLException, reconnects, retries the rollback, and verifies the insert was rolled back.

Contract decision

Parity with xerial 3.53.4.0: commit and rollback share one error path, a failed ROLLBACK throws, and transaction state is not cleared on failure.

Deviations

Transport-only: COMMIT/ROLLBACK is sent as its own pipeline request, and the stream Close is sent only after a successful transaction command. Sending Execute + Close in one pipeline would close the Hrana stream even when the command failed, so the driver could not preserve transaction state for a retry. The observable JDBC behavior matches xerial; the extra request is forced by the Hrana stream model.

Evidence

Failing before the fix (old LibsqlConnection):

$ ./mvnw -q -pl libsql-jdbc -am -Dtest=LibsqlConnectionTest -Dsurefire.failIfNoSpecifiedTests=false test
Tests run: 4, Failures: 4, Errors: 0, Skipped: 0
  serverErrorIsPropagatedAndTransactionStateIsKept[1]  Expecting code to raise a throwable
  serverErrorIsPropagatedAndTransactionStateIsKept[2]  Expecting code to raise a throwable
  transportErrorIsPropagatedAndTransactionStateIsKept[1]  Expected size: 2 but was: 1
  transportErrorIsPropagatedAndTransactionStateIsKept[2]  Expecting code to raise a throwable

Live failing before the fix (old driver installed, Toxiproxy run):

NetworkToxicityIT.testFailedRollbackThrowsAndKeepsTransactionForRetry:150
Expecting code to raise a throwable.
Tests run: 15, Failures: 1, Errors: 0, Skipped: 0

Passing after the fix:

$ ./mvnw -pl libsql-jdbc -am test
libSQL Hrana Codec ... SUCCESS (4 tests)
libSQL JDBC Driver ... SUCCESS (7 tests)
BUILD SUCCESS

$ DOCKER_CONFIG=/tmp/docker-config ./mvnw -pl libsql-itests -am verify
NetworkToxicityIT ... SUCCESS (5 tests)
libSQL Integration Tests ... SUCCESS (15 tests)
BUILD SUCCESS

$ ./mvnw spotless:apply
BUILD SUCCESS

DOCKER_CONFIG=/tmp/docker-config uses an empty public-image auth file because the sandbox blocks ~/.docker/config.json (AGENTS.md section 4). It does not change test behavior.

Out-of-scope defects

None newly found. The re-BEGIN-after-commit/rollback lifecycle gap is already tracked by #40.

@magnusp
magnusp force-pushed the fix/30-rollback-failures branch 2 times, most recently from 087a5d9 to 2c38e54 Compare October 21, 2026 21:45
Share commit and rollback error handling, throw SQLException when the
server rejects the transaction command, and keep the baton so callers
can retry without losing transaction state.
@magnusp
magnusp force-pushed the fix/30-rollback-failures branch from 2c38e54 to e1abd18 Compare October 21, 2026 21:51
@magnusp
magnusp merged commit ce15323 into main Oct 21, 2026
5 checks passed
@magnusp
magnusp deleted the fix/30-rollback-failures branch October 21, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rollback() swallows failures and reports success

1 participant