Repository navigation
fix(sql): propagate a failed transaction begin as a typed error - #7236
Merged
tim-smart merged 3 commits intoAug 13, 2026
Merged
Conversation
`makeWithTransaction` wrapped the `begin` step together with the transaction body in `Effect.exit`, so a failed `BEGIN` took the rollback branch. No transaction was active, the `ROLLBACK` failed, and its `Effect.orDie` wrapper replaced the original typed `SqlError` with a defect (`cannot rollback - no transaction is active`), so callers could no longer classify the failure as retryable. Commit and rollback now run only after `begin` or `savepoint` succeeds. A failed `begin` or `savepoint` fails with its original `SqlError`, leaves the wrapped effect unexecuted, and still closes the acquired connection scope. fixes Effect-TS#7235 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 543dfc9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
The unit tests for `makeWithTransaction` drive the transaction control flow with stubs. Add the driver-level counterpart from issue Effect-TS#7235: a second client whose `BEGIN IMMEDIATE` cannot take the write lock must fail `withTransaction` with a typed `SqlError`, not a rollback defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merged
roninjin10
added a commit
to smithersai/flows-proto
that referenced
this pull request
Aug 14, 2026
…7235 Two DurableWriter contract cases (NodeDatabase two-connection harness only) and one journal durable-emission case fail on "cannot rollback - no transaction is active": Effect's SqlClient.makeWithTransaction issues ROLLBACK even when BEGIN itself failed under real write-lock contention. Root cause tracked upstream at Effect-TS/effect#7235, fixed unreleased by Effect-TS/effect#7236. Mark with it.fails so each flips loudly red the moment flows depends on an effect release containing the fix.
17 tasks
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #7235
What was broken
makeWithTransactioninpackages/effect/src/unstable/sql/SqlClient.tswrapped thebegin/savepointstep together with the transaction body inEffect.exit. A failedBEGINtherefore landed in the failure branch, which issues aROLLBACK. No transaction was active at that point, so theROLLBACKfailed too, and itsEffect.orDiewrapper discarded the original typedSqlErrorand replaced it with a defect:Callers can no longer classify the failure as busy/locked, so busy-retry logic never sees it.
Why it surfaced now
The
makeWithTransactioncode is unchanged from the betas, but the branch was unreachable: the older sqlite client set neitherbeginTransactionnorbusy_timeout, so a plain deferredBEGINtook no lock and could not fail. The current sqlite client setsbeginTransaction: "BEGIN IMMEDIATE"plusPRAGMA busy_timeoutfor writable connections, soBEGINitself can fail withSQLITE_BUSYand reach the latent branch.The fix
begin/savepointnow runs outside theEffect.exitregion. Commit and rollback handling applies only after it succeeds. A failedbegin/savepointpropagates as its original typedSqlError, and the acquired connection scope is still closed viaEffect.onError. Everything else is unchanged:uninterruptibleMask/restorebehaviour, span events,Effect.orDieon the commit/rollback that follow a successful begin, and scope closure on every other path.Tests
New
packages/effect/test/unstable/sql/SqlClient.test.tsdrivesmakeWithTransactionwith stub transaction commands (no driver needed). The stubs mirror the driver contract:rollback/rollbackSavepointfail when nothing is active.beginpropagates typed, the wrapped effect never runs, androllbackis not calledbeginstill rolls back and propagates its typed errorsavepointin a nested transaction propagates typed withoutrollbackSavepointbeginfailsAll five fail-first: before the fix the begin and savepoint cases fail with the
cannot rollback - no transaction is activedefect.Verified end-to-end against
@effect/sql-sqlite-nodewith the reproduction from the issue (two connections,busyTimeout: 0). Before:hasDies: true,UnknownError: cannot rollback - no transaction is active. After: a typedSqlErrorwhose reason isLockTimeoutErrorwithisRetryable: true.Added in follow-up:
packages/sql/sqlite-node/test/Client.test.tsnow carries the driver-level counterpart, so the reported scenario is guarded in CI rather than only checked by hand. A contending client withPRAGMA busy_timeout = 1runswithTransactionwhile another client holds the write lock; the test asserts the cause carries no defect and that the typed error is aSqlErrorreportingdatabase is locked. It fails onmainwithexpected a typed failure but the cause contains a defectand passes with this change.