views:

137

answers:

3

What is the best way to implement error handling for a SqlTransaction RollBack that already exists within a catch clause? My code is roughly like this:

using (SqlConnection objSqlConn = new SqlConnection(connStr)) {
  objSqlConn.Open();

  using (SqlTransaction objSqlTrans = objSqlConn.BeginTransaction()) {
    try {
      // code
      // more code
      // and more code
    }
    catch (Exception ex) {
      // What happens if RollBack() has an exception?
      objSqlTrans.Rollback();
      throw ex;
    }
  }
}

I believe that my application had an exception in the try block, which in turn was caught in the catch block and then the RollBack was attempted. However, the error that I'm seeing says something about a SqlTransaction.ZombieCheck(), which is making me wonder if the RollBack() itself threw an exception as well. So, do I need to implement some type of error handling at the RollBack()? How do I do that and manage to hold on to the exception that put the execution into the catch block in the first place?

EDIT - All of my code:

using (SqlConnection objSqlConn = new SqlConnection(connStr)) {

    objSqlConn.Open();

    // Begin Transaction
    using (SqlTransaction objSqlTrans = objSqlConn.BeginTransaction()) {

        try {
            // Create file in db (which in turn creates it on disk according to where the 
            // ...FileStream points)
            SqlCommand objSqlCmd = new SqlCommand("usp_FileAdd", objSqlConn, objSqlTrans);
            objSqlCmd.CommandType = CommandType.StoredProcedure;

            // Sql parameter - report name
            SqlParameter objSqlParam1 = new SqlParameter("@ObjectID", SqlDbType.Int);
            objSqlParam1.Value = objID;

            // Sql out parameter - returns the file path
            SqlParameter objSqlParamOutput = new SqlParameter("@filepath", SqlDbType.VarChar, -1);
            objSqlParamOutput.Direction = ParameterDirection.Output;

            // Add Sql parameters to command obj
            objSqlCmd.Parameters.Add(objSqlParam1);
            objSqlCmd.Parameters.Add(objSqlParamOutput);

            // Execute command object
            objSqlCmd.ExecuteNonQuery();

            // Path to the FileStream
            string path = objSqlCmd.Parameters["@filepath"].Value.ToString();

            // Reset command object to get FileStream
            objSqlCmd = new SqlCommand(
                "SELECT GET_FILESTREAM_TRANSACTION_CONTEXT()",
                objSqlConn,
                objSqlTrans);

            // Execute command object
            Object obj = objSqlCmd.ExecuteScalar();

            if (obj != DBNull.Value) {
                // Byte array representing the FileStream
                byte[] fsBytes = (byte[])obj;

                SqlFileStream sqlFS = new SqlFileStream(path, fsBytes, FileAccess.Write);

                using (FileStream fs = fi.OpenRead()) {
                    //byte[] b = new byte[1024];
                    byte[] b = new byte[4096];
                    int read;

                    fs.Seek(0, SeekOrigin.Begin);

                    while ((read = fs.Read(b, 0, b.Length)) > 0) {
                        sqlFS.Write(b, 0, read);
                    }
                }

                sqlFS.Close();
            }

            // Commit the transaction
            objSqlTrans.Commit();
        }
        catch (Exception ex) {
            objSqlTrans.Rollback();
            throw ex;
        }
    }
}
A: 

That should also probably be a throw; rather than a throw ex;

Wyatt Barnett
+1  A: 

This fragment should read like so:

using (SqlConnection objSqlConn = new SqlConnection(connStr)) {
 objSqlConn.Open();

 using (SqlTransaction objSqlTrans = objSqlConn.BeginTransaction()) {
   try {
     // code
     // more code
     // and more code
   }
   catch (Exception ex) {
     // What happens if RollBack() has an exception?
     try {
        objSqlTrans.Rollback();
     } catch (Exception ex2) {
        /* can't roll back -- db gone? db will do it for us since we didn't commit. */
     }
     throw;
   }
 }
}

EDIT: Come to think of it there's no need for the whole try/catch at all in this particular case as closing a connection with an uncommitted transaction rolls back the transaction, so the block can look like this:

using (SqlConnection objSqlConn = new SqlConnection(connStr)) {
 objSqlConn.Open();

 using (SqlTransaction objSqlTrans = objSqlConn.BeginTransaction()) {
  // code
  // more code
  // and more code
 }
}
Joshua
I think you missed the catch keyword in that 2nd try block. One thing that I'm confused about - if Rollback() works just fine and doesn't have an error, then I'll never actually get to see the original exception since the throw only exists in the 2nd catch block, right?
Jagd
Fixed newlines to make it obvious the throw is at the end of the first catch block.
Joshua
A: 

You've already got

using (SqlTransaction objSqlTrans = objSqlConn.BeginTransaction())

This will cause the Transaction to be rolled back when it the using block ends if it hasn't been commited.

So I would remove the catch block entirely.

As for what happens when the rollback fails I would start by recognizing this as a very bad situation to be in and follow Eric Lippert's advice on a similar problem. here

Conrad Frix
I think you may have nailed it. I found another question on SO that dealt with using blocks for SqlTransaction, and I noticed that within their example they don't use the Rollback either. I don't think my question is a duplicate of that one though. http://stackoverflow.com/questions/1127830/why-use-a-using-statement-with-a-sqltransaction
Jagd