MemberJunction / MemberJunction/MJ
mj sync push: race between an in-flight request and the transaction commit discards a fully successful push
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## What happens
`mj sync push` intermittently fails at the very end, after every metadata step has succeeded, with a driver-level transaction error. The whole push then rolls back.
```
{"event":"step-done","label":"[84/84] metadata/view-types — 6 records, no changes"}
TransactionError: Can't commit transaction. There is a request in progress.
⚠️ Rolling back database transaction due to error...
Failed to rollback transaction: Error: No active transaction to rollback
{"event":"step-failed","label":"Push failed","detail":"Failed to commit transaction: Can't commit transaction. There is a request in progress."}
```
All 84 steps reported done, including real writes (`1 created`, `1 updated` on several collections). The failure is only in the commit.
Observed in the Integration Tier: [run 34305908887](https://github.com/MemberJunction/MJ/actions/runs/34305908887/job/102322475420), on `fix/codeql-tooling-scripts` (PR #4314), 2026-09-09. Re-running the identical job on the same commit passed, so it is timing dependent.
## Why this is a real defect and not CI noise
- The failing commit is a merge of `next` carrying no changes near metadata sync. The diff from the previous passing merge touches nothing in `MetadataSync`, `GenericDatabaseProvider`, `SQLServerDataProvider`, or any transaction code.
- Every other commit on that branch passed the same job, and `next` passed in the same minute.
- Each integration run creates its own SQL Server container and a fresh `test` database, so there is no cross-run contamination.
- The error appears once in the last 60 Integration Tier runs. Rare, not benign: when it hits, an otherwise fully successful metadata push is discarded.
The secondary error is worth noting on its own. The rollback that follows fails with `No active transaction to rollback`, because the failed commit already abandoned the handle and cleared state. The push then logs both "Rolling back database transaction due to error" and "Database transaction rolled back successfully" around a rollback that did not happen.
## Where
- `packages/GenericDatabaseProvider/src/GenericDatabaseProvider.ts` — `WithTransactionLock` (~5522), `CommitTransaction` (~5574), `commitTransactionCore` (~5675)
- `packages/SQLServerDataProvider/src/SQLServerDataProvider.ts` — `CommitPhysicalTransaction` (~2342)
## Mechanism, as far as the code shows
`mssql` raises `Can't commit transaction. There is a request in progress.` when `transaction.commit()` is called while a request on that transaction is still active.
`WithTransactionLock` serializes begin, commit and rollback **against each other**. It does not serialize them against ordinary data requests. `BeginPhysicalTransaction` already acknowledges this, in its own comment:
> Assign `_transaction` only after begin() resolves so concurrent ExecuteSQL never sees an un-begun handle.
Commit has no equivalent guard. `CommitPhysicalTransaction` calls `this._transaction.commit()` with no check that no request is outstanding on that transaction. So a single `ExecuteSQL` that is issued but not awaited to completion before the push finishes its steps is enough to lose this race, and it will only lose it when that request is still in flight at the moment commit runs. That matches both the rarity and the fact that every step reported success.
Two directions worth investigating, in order:
1. Find the unawaited request in the push path. A floating promise in a step that reports done before its SQL settles is the direct cause, and the honest fix. `@memberjunction/metadata-sync` push is the place to look.
2. Make the provider fail loudly rather than racily. Track outstanding requests per transaction and either await them before `commit()` or throw a diagnostic naming the in-flight statement. Today the driver's message says nothing about which request was still running, which is why this is hard to chase from a CI log.
While in there, the rollback path should not report success when `rollbackTransactionCore` threw `No active transaction to rollback`.
## Impact
A metadata push that has done all its work is thrown away, and the operator sees a message that points at the transaction layer rather than at anything they did. In CI it looks like a flaky integration job, which is how it has been read so far. On a developer machine or a release run it is a failed `mj sync push` that succeeds on retry, which is the kind of thing that trains people to retry without reading.
## Reproducing
No deterministic repro. It is timing dependent and has surfaced once in 60 runs of the deterministic integration tier. Instrumenting `CommitPhysicalTransaction` to log outstanding requests, or adding the tracking in direction 2 above, would turn the next occurrence into a named statement instead of a generic driver error.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor guide
Research direction
Start in packages/GenericDatabaseProvider/src/GenericDatabaseProvider.ts at WithTransactionLock, CommitTransaction, and commitTransactionCore, then inspect packages/SQLServerDataProvider/src/SQLServerDataProvider.ts at CommitPhysicalTransaction and the @memberjunction/metadata-sync push path. Run the Integration Tier or add instrumentation around the transaction commit to identify outstanding requests. Done means the push no longer loses successful work at commit, and rollback does not report success after failing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100