How to ensure rollback from without explicit ROLLBACK

Question

If the ROLLBACK is out of scope because of the Catch block, assuming that is the cause, how am I supposed to resolve other than putting all the saves in a single method?

SqlConnection connection = new SqlConnection(DataAccess.ConnectionString);
connection.Open();
string sql = "USE IncidentReports; BEGIN TRANSACTION;";
SqlCommand cmd = new SqlCommand(sql, connection);
cmd.ExecuteNonQuery();
cmd.Dispose();

try
{
    //if anyone of these saves fails, I want to rollback everything. No partial saves
    SaveIncident(connection);
    if (SelectedStatus.Key != _previousStatus) { SaveStatusChange(connection); }
    SaveEmployeeData(connection);
    SaveNotes(connection);
    SaveChecklist(connection);
    SaveExpenses(connection);

    _previousStatus = SelectedStatus.Key;

    HasBeenSaved = true;
    NewIncident = false;

    sql = "USE IncidentReports; COMMIT TRANSACTION;";
    cmd = new SqlCommand(sql, connection);
    cmd.ExecuteNonQuery();
    cmd.Dispose();
}
catch (Exception ex)
{
    sql = "USE IncidentReports; ROLLBACK TRANSACTION;";
    cmd = new SqlCommand(sql, connection);
    cmd.ExecuteNonQuery();
    cmd.Dispose();

    var mbp2 = new MessageBoxParams()
    {
        Caption = StaticGlobals.MSGBOX_CAPTION,
        Image = MessageBoxImage.Exclamation,
        Button = MessageBoxButton.OK,
        Message = $"Incident was not saved, the following error occurred;{Environment.NewLine}{ex.Message}"
    };
    ShowMessageBox(mbp2);
    return;
}
finally
{
    cmd?.Dispose();
    connection?.Close();
}           

Answer 1

You shouldn't manually do transactions with BEGIN and COMMIT unless it is all in the same batch (along with SET XACT_ABORT ON).

If you want a client-side transaction (ie you want the transaction to begin before any SqlCommand, and continue until all commands are done), you should just use BeginTransaction on the SqlConnection object.

Then each command is created using that SqlTransaction object. And you call transaction.Commit(); at the end.

Furthermore:

• Put a using on the connection, command and transaction objects. Then you don't need the finally.
• Instead of USE, put the database you want to connect into the connection string.
• Do not block the calling thread with message boxes until you have closed the connection.
• Consider using async await in order to keep your UI responsive.

try
{
    using var connection = new SqlConnection(DataAccess.ConnectionString);
    connection.Open();
    using var tran = connection.BeginTransaction();

    SaveIncident(connection, tran);

    if (SelectedStatus.Key != _previousStatus)
    {
        SaveStatusChange(connection, tran);
    }

    SaveEmployeeData(connection, tran);
    SaveNotes(connection, tran);
    SaveChecklist(connection, tran);
    SaveExpenses(connection, tran);

    _previousStatus = SelectedStatus.Key;

    HasBeenSaved = true;
    NewIncident = false;

    tran.Commit();
}
catch (Exception ex)
{
    var mbp2 = new MessageBoxParams()
    {
        Caption = StaticGlobals.MSGBOX_CAPTION,
        Image = MessageBoxImage.Exclamation,
        Button = MessageBoxButton.OK,
        Message = $"Incident was not saved, the following error occurred;{Environment.NewLine}{ex.Message}"
    };
    ShowMessageBox(mbp2);
    return;
}