Closed ThomasShih closed 4 weeks ago
Hi, the goal of this library is to abstract away transactions, so that the code executing the query doesn't have to care whether it's run within a transaction or not, so while you could access queryRunner through this.tx.queryRunner?.rollbackTransaction();
, it would throw an error in case there was no transaction, so I would not recommend this approach.
The nestjs-cls
library doesn't attempt to add any more abstraction over transactions than necessary, so it does the bare minimum to
1) put a transaction manager/reference into the context
2) expose the transaction manager/reference via a typed accessor
3) leave any and all rollback mechanisms of the underlying database libraries via adapters.
All the "official" adapters try to be as simple as possible - usually, throwing an error within a transaction
callback rolls back the transaction, so the implementation of the TypeORM (and most other) adapter is essentially just these few lines:
optionsFactory = (dataSource: DataSource) => ({
wrapWithTransaction: async (
options: TypeOrmTransactionOptions,
fn: (...args: any[]) => Promise<any>,
setClient: (client?: EntityManager) => void,
) => {
return dataSource.transaction(options?.isolationLevel, (trx) => {
setClient(trx); // store transaction in the CLS
return fn(); // execute the decorated function
// if this throws, the transaction is automatically rolled back by TypeORM
});
},
getFallbackInstance: () => dataSource.manager,
});
So, for your use-case, I would recommend solving it at the adapter level by writing a custom result-aware adapter by extending TransactionalAdapterTypeOrm
and overriding optionsFactory
with something like the following:
optionsFactory = (dataSource: DataSource) => ({
wrapWithTransaction: async (
options: TypeOrmTransactionOptions,
fn: (...args: any[]) => Promise<any>,
setClient: (client?: EntityManager) => void,
) => {
- return dataSource.transaction(options?.isolationLevel, (trx) => {
+ return dataSource.transaction(options?.isolationLevel, async (trx) => {
setClient(trx);
- return fn()
+ const result = await fn();
+ if (result.isErr()) {
+ await trx.queryRunner?.rollbackTransaction()
+ }
+ return result
});
},
getFallbackInstance: () => dataSource.manager,
});
(you'll probably want to wrap rollbackTransaction
in a result, too, since it can potentially throw)
This should ensure that any time a method decorated with @Transactional()
returns a result of Err
, the transaction rolls back and the original result is returned.
Thank you!
We have tried both implementations already and neither seems to be the smartest thing to do.
Our use of neverthrow contain errors that may not want to be handled, and may just be a logical decision making step, we can state that all .isErr() === true
returns at the transaction level must be hard "ejects, handle like throws", but that seems like a way too overly broad statement to assume with a broad stroke.
The tx.queryRunner?.rollbackTransaction()
is an option using the built in adaptor, we can write a generic function to verify the transaction exists, and rollback only if. But it seems that there is a check missing from the lib because calling rollbackTransaction means that no transaction exist, and if we don't throw an error the lib still attempts a commit afterwards:
I'm not actually sure how we can avoid the commit attempt given that we are using typeorm's dataSource.transaction(
to wrap everything, which is a bit annoying.
Once solution, which I hate, is the idea of writing a custom adapter that sandwiches the .transaction
, but I don't think this would be correct in any way and hence didn't do it:
wrapWithTransaction: async (
options: TypeOrmTransactionOptions,
fn: (...args: any[]) => Promise<any>,
setClient: (client?: EntityManager) => void,
) => {
try {
return dataSource.transaction(options?.isolationLevel, async (trx) => {
setClient(trx);
const result = await fn();
// Check if transaction still not active and result.isErr() is true, if true, throw custom error with the original message
return result
});
} catch (e) {
// If e is some custom flag, convert back to err()
}
},
Ah, I see, that is the nature of the callback-style transaction, in which the only way to rollback is to throw an error.
In that case, you could write an adapter with manual transaction control.
Like this:
wrapWithTransaction: async (
options: TypeOrmTransactionOptions,
fn: (...args: any[]) => Promise<any>,
setClient: (client?: EntityManager) => void,
) => {
const queryRunner = dataSource.createQueryRunner()
try {
await queryRunner.connect()
await queryRunner.startTransaction(options?.isolationLevel)
setClient(queryRunner.manager)
const result = await fn()
if(result.isErr()) {
await queryRunner.rollbackTransaction()
} else {
await queryRunner.commitTransaction()
}
return result;
} catch (e) {
await queryRunner.rollbackTransaction()
return err( /*...*/ )
} finally {
await queryRunner.release()
}
}
This way the transaction not committed if Err
is returned.
But if you'd like to explicitly rollback the transaction within the wrapped function, then I'm afraid you can't use this library without inventing a hacky solution.
Yes I think thats our conclusion too!
Thank you for your help.
For anyone else that might be wondering about this in the future, the two solutions that I'm currently debating is:
if(result.isErr() && result.err.triggerTransactionRollback)
queryRunner
in the ASL store, and having downstream functions fetch the queryRunner directly to get entity repos and to trigger an explicit rollback.Both have drawbacks
Nothing to change in the repo. Thank you @Papooch!
I'm currently using neverthrow to wrap all errors in my codebase. This means that I don't raise any errors and therefore the current implementation (typeorm adapter) won't rollback.
I know that we can do a manual rollback with queryRunner.rollbackTransaction, but I'm not sure how to access that within a request context.
Assistance would be appreciated!