-
-
Notifications
You must be signed in to change notification settings - Fork 714
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG]: Transactions rollback doesn't work #1723
Comments
Is there any chance you can put together a reproduction repo? I would like to look into this. |
I am also experiencing this issue with
In the following example, step import { RDSDataClient } from '@aws-sdk/client-rds-data';
import { drizzle } from 'drizzle-orm/aws-data-api/pg';
const rdsClient = new RDSDataClient({});
export const db = drizzle(rdsClient, {
database: process.env.database,
resourceArn: process.env.resourceArn,
secretArn: process.env.secretArn
});
async function getOrCreateApplication({ userId, dealId }) {
return await db.transaction(async tx => {
let [application] = await tx
.select()
.from(applications)
.where(eq(applications.userId, userId));
// if already exists, return it
if (application) {
return application;
}
// 1. create application
application = await tx
.insert(applications)
.values({ userId })
.returning();
// 2. associate marketing entities
await tx
.insert(hs_deals)
.values({ dealId, applicationId: application.id });
return application;
});
} |
bumping this, this is a pretty important functionality to not be working or have a workaround. It's super simple to reproduce with the happy path of a rollback:
Will output:
|
turns out I was doing this wrong, works fine for postgres, you just have to be sure to throw within transaction and can't suppress it because it depends on catching the error in the underlying code in order to perform the rollback. So this works and rolls back:
but this won't:
Probably obvious, but just in case that trips anyone else up. |
This is not working on
Here is the test that fails import { test, expect } from "vitest";
import { db } from "./database";
import * as Schema from "./schema";
import {eq} from "drizzle-orm";
import { faker } from "@faker-js/faker";
test("Should rollback", async () => {
const name = faker.person.firstName();
await expect(() =>
addUser(name)
).rejects.toThrow();
const users = await db
.select()
.from(Schema.users)
.where(eq(Schema.users.name, name));
expect(users).toHaveLength(0);
});
async function addUser(name: string) {
await db.transaction(async tx => {
await tx.insert(Schema.users).values({
name
});
throw new Error("BOOM");
});
}
} |
I believe it is this line https://github.com/drizzle-team/drizzle-orm/blob/main/drizzle-orm/src/better-sqlite3/session.ts#L74 If |
Seems like it a deeper issue. At the moment |
Bettersqlite3 is only synchronous |
@Angelelz If that is the case then shouldn't the types for something like queries be synchronous as well? Since they are async it naturally follows that a developer would treat them as such and use an The docs for better-sqlite on the drizzle site show |
If anyone comes across this my work around was to just handle transaction logic. This is just repurposing the bulk of the logic in type Transaction = {
readonly db: Database; // This type is the return type of `drizzle`
readonly nestedIndex: number;
readonly savepointName: string;
transaction: <T>(tx: (t: Transaction) => Promise<T>) => Promise<T>;
rollback: () => void;
};
function createTransaction(
db: Database,
nestedIndex?: number,
savepointName?: string,
): Transaction {
const idx = nestedIndex ?? 0;
const name = savepointName ?? "sp0";
return {
db,
nestedIndex: idx,
savepointName: name,
transaction: async (tx) => {
db.run(sql.raw(`savepoint ${name}`));
const t = createTransaction(db, idx + 1, `sp${idx + 1}`);
try {
const txResult = await tx(t);
db.run(sql.raw(`release savepoint ${name}`));
return txResult;
} catch (e) {
db.run(sql.raw(`rollback to savepoint ${name}`));
throw e;
}
},
rollback: () => {
throw new Error("Rollback called. Reverting transaction");
},
};
}
const databaseTransaction = createTransaction(db); // pass your initialized drizzle instance
await databaseTransaction.transaction(async ({ db, transaction, rollback }) => {
await db.insert(users).values({...});
await transactions(async ({ db, transaction, rollback }) => {
// nested transaction
});
rollback(); // call roll back to throw and revert transaction
}); |
I'm getting this error with the latest version of drizzle (0.30.4). This is my setup with bun (v1.0.35) and vitest (all the latest and recent updated):
This is my simple test case with vitest describe('Contacts API', () => {
it('should work', async () => {
await db.transaction(async (tx) => {
const contact = await new ContactFactory(tx).create();
const results = await tx.select().from(contacts);
expect(contact.id).toBeDefined();
expect(results).toHaveLength(1);
tx.rollback(); // this doesn't return a promise so await here is irrelevant and also doesn't make different
console.log('Never reach this line!');
});
});
}); And this is the result I'm getting: bun test v1.0.35 (940448d6)
api/contacts/index.test.ts:
1 | import { entityKind } from "./entity.js";
2 | class DrizzleError extends Error {
3 | static [entityKind] = "DrizzleError";
4 | constructor({ message, cause }) {
5 | super(message);
^
DrizzleError: Rollback
at new DrizzleError (/Users/marco/projects/testings/project-x/node_modules/drizzle-orm/errors.js:5:9)
at new TransactionRollbackError (/Users/marco/projects/testings/project-x/node_modules/drizzle-orm/errors.js:13:5)
at rollback (/Users/marco/projects/testings/project-x/node_modules/drizzle-orm/pg-core/session.js:55:15)
at /Users/marco/projects/testings/project-x/api/contacts/index.test.ts:16:7
✗ Contacts API > should work [122.68ms]
0 pass
1 fail
2 expect() calls
Ran 1 tests across 1 files. [288.00ms] |
This seems to be the normal behaviour, Thats just my intuition though, as the transaction API in drizzle isn't really documented aside from basically mentioning that it exists. |
I get:
|
Did you find a workaround? Throwing an error inside the transaction function doesn't seem to trigger the auto-rollback. PS: using bun sqlite driver |
My workaround, for better-sqlite driver with better-sqlite-pool lib. import { BetterSQLite3Database, drizzle } from "drizzle-orm/better-sqlite3";
import { Pool } from "better-sqlite-pool";
import * as schema from "./schema";
export type DB = BetterSQLite3Database<typeof schema> & {
release: () => void;
};
export class ConnectionPool {
constructor(
dbPath: string,
) {
this.pool = new Pool(dbPath, {
onConnectionCreated: sqlite => {
// enabling Write-Ahead Logging for better performance
sqlite.pragma("journal_mode = WAL");
// enforce foreign key constraints
sqlite.pragma("foreign_keys = ON");
},
});
}
private readonly pool: Pool;
public acquire = async (): Promise<DB> => {
const connection = await this.pool.acquire();
const db = drizzle(connection, {
schema,
});
Object.assign(db, { release: () => connection.release() });
return db as DB;
};
}
export const runInReadMode =
<TResult, TArgs = void>(
connectionPool: ConnectionPool,
fn: (db: DB, args: TArgs) => Promise<TResult>
) =>
async (args: TArgs) => {
const db = await connectionPool.acquire();
try {
return await fn(db, args);
} finally {
db.release();
}
};
export const runInTransaction = <TResult, TArgs = void>(
connectionPool: ConnectionPool,
fn: (db: DB, args: TArgs) => Promise<TResult>
) => {
return async (args: TArgs) => {
const db = await connectionPool.acquire();
db.run(sql.raw(`BEGIN TRANSACTION`));
try {
const res = await fn(db, args);
db.run(sql.raw(`COMMIT TRANSACTION`));
return res;
} catch (e) {
db.run(sql.raw(`ROLLBACK TRANSACTION`));
throw e;
} finally {
db.release();
}
};
};
const main = async () => {
const connectionPool = new ConnectionPool(dbPath);
const readResult = await runInReadMode(connectionPool, readSomeData);
const writeResult = await runInTransaction(connectionPool, writeSomeData);
}
const readSomeData = async (db: DB) => {
const users = await db.select().from(usersTable);
const products = await db.select().from(productsTable);
return { products, users };
}
const writeSomeData = async (db: DB) => {
const user = await db.insert(usersTable).values({ name: "John" }).returning();
const product = await db.insert(productsTable).values({ name: "Apple" }).returning();
if (!product || !user) {
throw new Error("Some error");
}
return { user, product };
} I am using this approach for node app that has incoming requests, requests either read-only or read-write. So they are wrapped in corresponding wrappers. |
Same with me using 0.30.10 and postgres. |
Does anyone know a workaround for this in expo-sqlite? I have been trying to call tx.rollback() in a transaction but instead db.transaction(...) didn't roll back and just throw the error back at me. |
Still having the issue. No idea what caused it, it happened pretty much overnight and I started getting this error as soon as I upgraded my iOS simulator from 17.4 to 17.5. No code change. Also I don't manually call the rollback function. I am getting a "Failed to run the query" on a CREATE TABLE statement from useMigrations, but from what I see in the logger, the rollback is causing the error? I'm not quite sure what's happening, my apologies if I made a mistake. |
Running into the same issue today on 0.32.0. |
Also running into this issue, thought i was going crazy while trying to debug |
Same here driving me crazy.. :( const affectedRows = await db.transaction(async (tx) => {
// Perform the update within a transaction
const [updateResult] = await tx.execute(
sql`UPDATE ${projectsData} SET text_translated = REPLACE(text_translated, ${oldText}, ${newText})
WHERE text_translated LIKE ${'%' + oldText + '%'}`
);
// Get the number of affected rows
const affectedRows = updateResult.affectedRows - 1;
// Rollback the transaction to simulate without actual changes
if (dryRun) {
tx.rollback();
}
return affectedRows;
}); always drops to catch with error Rollback. Basically returning catch block. This is stupid... Bug is open for almost the year and still not fixed... :( |
Came across this when using Drizzle with sqlite. The problem is if you use an I posted an issue on So this is not really a Drizzle bug, but a better-sqlite3 bug. |
Not sure if this is helpful but I've been using a raw query to avoid this weird error await tx.execute(sql`ROLLBACK`); |
How do you do that? I wrapped my transaction code in try...catch, then on the catch I run trx.run(sql`ROLLBACK`); but I get an error from that line: Failed to run the query 'ROLLBACK'. This really sucks, as it's pretty basic for transactions to work as intended, sad there's no solution in sight |
for me was that I left "db" instead of convert all query with "tx"..... such a stupid error...😂 Btw I'm using neon serverless driver because the http driver of don't support transactions yet |
I'm also using |
What version of
drizzle-orm
are you using?0.29.2
What version of
drizzle-kit
are you using?0.20.9
Describe the Bug
I wrote a pretty simple transaction but when the error appears rollback doesn't work.
I have 3 steps and when the error happens on the second step, the first change is still in DB.
Expected behavior
I expect that if the error happens at some point in the transaction the entire transaction rolls back. But it doesn't.
Even when I try to run rolling back manually I get an error:
[DrizzleError: Rollback] { name: 'DrizzleError', cause: undefined }
Environment & setup
It is running on Vercel and the project is on Next.js 14.
The text was updated successfully, but these errors were encountered: