Friday, August 25, 2006

finally {} is not a good place to commit your transactions

I just fixed some code (not written by me!) that went something like this:

transaction = connection.BeginTransaction();
try {
try
{
//***.... database operations here...
} finally {
transaction.Commit();
}
} catch {
transaction.RollBack();
}

First of all, there is a bug there, if an exception is ever raised, it will raise a new exception because it cannot rollback a transaction that was committed (the message will not be that clear, but that's what it means, the exception says something about unable to complete the operation due to the current state of the object), and you'll lose the original exception that you got


Second, it's just wrong, it defeats the purpose of using a transaction, is like you are saying


//*** delete some records


//*** update some records


//*** some db operation that caused some exception


//*** finally, commit what I had done so far


leaving your data in an unknown state because you commited all the operations up until it failed


hint: always use "using" for any DB related stuff (connections, commands, transactions, etc)

No comments: