Issue Creation Checklist
Bug Description
Three compounding bugs in the savepoint path break nested-transaction workflows that wrap per-unit work inside an outer (typically top-level) transaction. All three are confirmed on sequelize@6.37.8 + Postgres + cls-hooked. Bug 1 is still present on main / v7; Bug 2 is already fixed there (via the explicit savepointName option); Bug 3 does not apply to v7 because v7 dropped CLS.
I believe these are the root causes behind the symptom reported in #12762.
Bug 1 — Savepoint name collision across nesting levels
v6.37.8/src/transaction.js:30-36:
this.parent = this.options.transaction;
if (this.parent) {
this.id = this.parent.id; // <-
this.parent.savepoints.push(this);
this.name = `${this.id}-sp-${this.parent.savepoints.length}`; // <-
}
Every descendant of a top-level transaction inherits the same id, and the savepoints counter is per parent, so the first child at every depth gets …-sp-1:
global : id=abc, savepoints=[wrapper]
wrapper: id=abc, name=abc-sp-1, savepoints=[child]
child: id=abc, name=abc-sp-1 ← collision
Postgres' ROLLBACK TO SAVEPOINT abc-sp-1 resolves to the most recently declared savepoint with that name, so a later rollback of the wrapper lands on the child instead (or fails outright, depending on order). Writes done at the wrapper level between the child's lifetime and the wrapper's rollback are silently committed when the top-level transaction eventually commits.
v7 main still carries the same pattern in packages/core/src/transaction.ts:
if (this.parent) {
this.id = this.parent.id;
this.#name = `${this.id}-sp-${this.parent.#savepoints.size}`;
this.parent.#savepoints.set(this.#name, this);
}
Fix idea: either use a top-level-rooted counter (walk up to the root and use its total savepoint count), or include a depth/path segment in the name, or just generate a fresh id per savepoint instead of inheriting from the parent.
Bug 2 — Parent .name mutated on every child create/rollback (v6 only)
v6.37.8/src/dialects/abstract/query-interface.js:519-527 and :552-566:
async startTransaction(transaction, options) {
...
options = { ...options, transaction: transaction.parent || transaction };
options.transaction.name = transaction.parent ? transaction.name : void 0; // <-
...
}
options.transaction is an alias for the parent object, not a copy. The next line mutates parent.name to the child's name. Same pattern in rollbackTransaction.
Effect: creating a savepoint inside a wrapper permanently stomps the wrapper's .name. Any caller that cached the wrapper's original name (e.g. to later issue a manual ROLLBACK TO SAVEPOINT <cachedName>) ends up targeting the child instead.
This is already fixed in v7 — the name is threaded through an explicit option (savepointName / transactionName) rather than mutated onto the parent. Asking for a v6 backport.
Bug 3 — CLS not cleared on savepoint rollback (v6 only)
v6.37.8/src/transaction.js:117-124:
cleanup() {
if (this.parent || this.connection.uuid === void 0) {
return; // <- savepoints early-return
}
this._clearCls();
this.sequelize.connectionManager.releaseConnection(this.connection);
this.connection.uuid = void 0;
}
The early-return is correct for the connection-release half (savepoints share the parent's connection) but also skips _clearCls(). After savepoint.rollback(), CLS still points at the finished savepoint, so any subsequent CLS-routed query throws:
Error: rollback has been called on this transaction <id>-sp-1, you can no longer use it.
Suggested fix: reset CLS even for savepoints, only skip the connection release:
cleanup() {
if (this.parent) {
const clsNs = this.sequelize.constructor._cls;
if (clsNs && clsNs.get('transaction') === this) {
clsNs.set('transaction', this.parent);
}
return;
}
if (this.connection.uuid === void 0) return;
this._clearCls();
this.sequelize.connectionManager.releaseConnection(this.connection);
this.connection.uuid = void 0;
}
This is N/A in v7 (no CLS), so only asking for the v6 fix.
Reproducible Example
Public gist: https://gist.github.com/kessiler/658a6dc64a4c2754c2b2a907ef17fec3
bug-repro.js — self-contained, ~120 lines, needs sequelize@6.37.8 + pg + cls-hooked + a reachable postgres
// Demonstrates three compounding bugs in sequelize@6.37.8 nested savepoints:
// 1. Savepoint name collision across nesting levels (transaction.js:30-36)
// 2. Parent .name mutation on child create/rollback (query-interface.js:523-524, 560-561)
// 3. CLS not cleared on savepoint rollback (transaction.js:117-124)
const cls = require('cls-hooked');
const { Sequelize } = require('sequelize');
const namespace = cls.createNamespace('sq-repro');
Sequelize.useCLS(namespace);
const sequelize = new Sequelize(
process.env.DATABASE_URL || 'postgres://postgres:postgres@localhost:5432/postgres',
{ logging: false },
);
async function main() {
// cls-hooked requires an active context before set() works
namespace.enter(namespace.createContext());
// ---------------------------------------------------------------
// BUG 1 + BUG 2a: pure flow, no workarounds
// ---------------------------------------------------------------
console.log('===== PART 1: pure flow (no workarounds) =====\n');
const globalA = await sequelize.transaction({ autocommit: false });
const globalANameAtCreation = globalA.name;
console.log('globalA.id =', globalA.id);
console.log('globalA.name at creation =', globalANameAtCreation);
const wrapperA = await sequelize.transaction({
autocommit: false,
transaction: globalA,
});
const wrapperANameAtCreation = wrapperA.name;
console.log('wrapperA.id =', wrapperA.id);
console.log('wrapperA.name at creation =', wrapperANameAtCreation);
console.log();
console.log('=== BUG 2a: creating wrapperA stomped globalA.name ===');
console.log('globalA.name was :', globalANameAtCreation);
console.log('globalA.name now :', globalA.name);
console.log('mutated ? ', globalA.name !== globalANameAtCreation);
console.log();
const childA = await sequelize.transaction({
autocommit: false,
transaction: wrapperA,
});
console.log('childA.id =', childA.id);
console.log('childA.name =', childA.name);
console.log();
console.log('=== BUG 1: savepoint name collision ===');
console.log('wrapperA.name (at creation) :', wrapperANameAtCreation);
console.log('childA.name :', childA.name);
console.log('collision ? ', wrapperANameAtCreation === childA.name);
console.log();
// Clean up part 1
try { await childA.rollback(); } catch (e) { /* noop */ }
try { await wrapperA.rollback(); } catch (e) { /* noop */ }
try { await globalA.rollback(); } catch (e) { /* noop */ }
// ---------------------------------------------------------------
// BUG 2b + BUG 3: workaround Bug 1 so the others are unambiguous
// ---------------------------------------------------------------
console.log('===== PART 2: Bug 2b + Bug 3 (with Bug 1 worked around) =====\n');
const globalB = await sequelize.transaction({ autocommit: false });
const wrapperB = await sequelize.transaction({
autocommit: false,
transaction: globalB,
});
// Force a unique wrapperB.id so child names don't collide with wrapperB.name
wrapperB.id = `wrap-${Date.now()}`;
const wrapperBNameAtCreation = wrapperB.name;
const childB = await sequelize.transaction({
autocommit: false,
transaction: wrapperB,
});
console.log('=== BUG 2b: creating childB stomped wrapperB.name ===');
console.log('wrapperB.name was :', wrapperBNameAtCreation);
console.log('wrapperB.name now :', wrapperB.name);
console.log('mutated ? ', wrapperB.name !== wrapperBNameAtCreation);
console.log();
// Bug 3: put childB into CLS (what sequelize.transaction(callback) does
// automatically, and what test frameworks like ours do explicitly), then
// roll it back and inspect CLS.
namespace.set('transaction', childB);
console.log('cls tx before childB.rollback(): is childB ?',
namespace.get('transaction') === childB);
await childB.rollback();
const clsTx = namespace.get('transaction');
console.log('=== BUG 3: CLS not cleared on savepoint rollback ===');
console.log('cls tx after childB.rollback():');
console.log(' is childB ?', clsTx === childB);
console.log(' is wrapperB ?', clsTx === wrapperB);
console.log(' is null ?', clsTx === null);
console.log(' finished flag on it :', clsTx && clsTx.finished);
console.log();
try {
await sequelize.query('SELECT 1', { transaction: namespace.get('transaction') });
console.log('subsequent query via CLS tx: OK');
} catch (e) {
console.log('subsequent query via CLS tx threw:');
console.log(' ', e.message);
}
console.log();
try { await wrapperB.rollback(); } catch (e) { /* noop */ }
try { await globalB.rollback(); } catch (e) { /* noop */ }
await sequelize.close();
}
main().catch((e) => {
console.error('unexpected error:', e);
process.exit(1);
});
Run it:
mkdir sq-bug-repro && cd sq-bug-repro
npm init -y
npm i sequelize@6.37.8 pg cls-hooked
DATABASE_URL=postgres://user:pass@localhost:5432/postgres node bug-repro.js
What do you expect to happen?
- Each savepoint gets a unique name (at minimum, distinguishable from its parent).
parent.name stays stable across child savepoint creation and rollback.
cls.get('transaction') after savepoint.rollback() points at the parent transaction (or null), never at the rolled-back savepoint.
What is actually happening?
Actual output of the repro above against sequelize@6.37.8 + pg + Postgres 14:
===== PART 1: pure flow (no workarounds) =====
globalA.id = 6459c59a-b454-427f-a26b-d4c0bba74989
globalA.name at creation = undefined
wrapperA.id = 6459c59a-b454-427f-a26b-d4c0bba74989
wrapperA.name at creation = 6459c59a-b454-427f-a26b-d4c0bba74989-sp-1
=== BUG 2a: creating wrapperA stomped globalA.name ===
globalA.name was : undefined
globalA.name now : 6459c59a-b454-427f-a26b-d4c0bba74989-sp-1
mutated ? true
childA.id = 6459c59a-b454-427f-a26b-d4c0bba74989
childA.name = 6459c59a-b454-427f-a26b-d4c0bba74989-sp-1
=== BUG 1: savepoint name collision ===
wrapperA.name (at creation) : 6459c59a-b454-427f-a26b-d4c0bba74989-sp-1
childA.name : 6459c59a-b454-427f-a26b-d4c0bba74989-sp-1
collision ? true
===== PART 2: Bug 2b + Bug 3 (with Bug 1 worked around) =====
=== BUG 2b: creating childB stomped wrapperB.name ===
wrapperB.name was : 07b3b893-1250-4397-8f69-ff4615cf18e3-sp-1
wrapperB.name now : wrap-1775745107157-sp-1
mutated ? true
cls tx before childB.rollback(): is childB ? true
=== BUG 3: CLS not cleared on savepoint rollback ===
cls tx after childB.rollback():
is childB ? true
is wrapperB ? false
is null ? false
finished flag on it : rollback
subsequent query via CLS tx threw:
rollback has been called on this transaction(wrap-1775745107157), you can no longer use it.
Environment
- Sequelize version:
6.37.8
- Node.js version:
v24.14.0
- If TypeScript related: n/a (plain JS repro)
- Database & Version: PostgreSQL 14 (also reproduced on 16)
- Connector library & Version:
pg@8.x
- CLS library:
cls-hooked@4.2.2
Would you be willing to resolve this issue by submitting a Pull Request?
Happy to open PRs for each of the three once I have maintainer steer on:
- Bug 1: preferred naming strategy (global counter vs. fresh id per savepoint vs. depth-aware template).
- Bug 2: confirmation that backporting the v7
savepointName shape to v6's query-interface.js is the desired fix.
- Bug 3: confirmation of the cleanup split above, or an alternative hook point maintainers prefer.
Issue Creation Checklist
Bug Description
Three compounding bugs in the savepoint path break nested-transaction workflows that wrap per-unit work inside an outer (typically top-level) transaction. All three are confirmed on
sequelize@6.37.8+ Postgres +cls-hooked. Bug 1 is still present onmain/ v7; Bug 2 is already fixed there (via the explicitsavepointNameoption); Bug 3 does not apply to v7 because v7 dropped CLS.I believe these are the root causes behind the symptom reported in #12762.
Bug 1 — Savepoint name collision across nesting levels
v6.37.8/src/transaction.js:30-36:Every descendant of a top-level transaction inherits the same
id, and thesavepointscounter is per parent, so the first child at every depth gets…-sp-1:Postgres'
ROLLBACK TO SAVEPOINT abc-sp-1resolves to the most recently declared savepoint with that name, so a later rollback of the wrapper lands on the child instead (or fails outright, depending on order). Writes done at the wrapper level between the child's lifetime and the wrapper's rollback are silently committed when the top-level transaction eventually commits.v7 main still carries the same pattern in
packages/core/src/transaction.ts:Fix idea: either use a top-level-rooted counter (walk up to the root and use its total savepoint count), or include a depth/path segment in the name, or just generate a fresh id per savepoint instead of inheriting from the parent.
Bug 2 — Parent
.namemutated on every child create/rollback (v6 only)v6.37.8/src/dialects/abstract/query-interface.js:519-527and:552-566:options.transactionis an alias for the parent object, not a copy. The next line mutatesparent.nameto the child's name. Same pattern inrollbackTransaction.Effect: creating a savepoint inside a wrapper permanently stomps the wrapper's
.name. Any caller that cached the wrapper's original name (e.g. to later issue a manualROLLBACK TO SAVEPOINT <cachedName>) ends up targeting the child instead.This is already fixed in v7 — the name is threaded through an explicit option (
savepointName/transactionName) rather than mutated onto the parent. Asking for a v6 backport.Bug 3 — CLS not cleared on savepoint rollback (v6 only)
v6.37.8/src/transaction.js:117-124:The early-return is correct for the connection-release half (savepoints share the parent's connection) but also skips
_clearCls(). Aftersavepoint.rollback(), CLS still points at the finished savepoint, so any subsequent CLS-routed query throws:Suggested fix: reset CLS even for savepoints, only skip the connection release:
This is N/A in v7 (no CLS), so only asking for the v6 fix.
Reproducible Example
Public gist: https://gist.github.com/kessiler/658a6dc64a4c2754c2b2a907ef17fec3
bug-repro.js— self-contained, ~120 lines, needssequelize@6.37.8+pg+cls-hooked+ a reachable postgresRun it:
What do you expect to happen?
parent.namestays stable across child savepoint creation and rollback.cls.get('transaction')aftersavepoint.rollback()points at the parent transaction (or null), never at the rolled-back savepoint.What is actually happening?
Actual output of the repro above against
sequelize@6.37.8+pg+ Postgres 14:Environment
6.37.8v24.14.0pg@8.xcls-hooked@4.2.2Would you be willing to resolve this issue by submitting a Pull Request?
Happy to open PRs for each of the three once I have maintainer steer on:
savepointNameshape to v6'squery-interface.jsis the desired fix.