Best practices for committing a transaction in SQL Server where TRY CATCH is used

Question

In a SQL Server code block, what is the best place to place the commit transaction? Inside the try catch block or outside it?.

For example, is option A or option B the correct approach or are they subjective choices?

Option A

CREATE PROCEDURE DummyProc 
BEGIN TRY
      BEGIN TRANSACTION
      INSERT sometable(a, b) VALUES (@a, @b)
      INSERT sometable(a, b) VALUES (@b, @a)
      COMMIT TRANSACTION
   END TRY
   BEGIN CATCH
      IF @@trancount > 0 ROLLBACK TRANSACTION
      DECLARE @msg nvarchar(2048) = error_message()  
      RAISERROR (@msg, 16, 1)
      RETURN 55555
   END CATCH   

Option B

CREATE PROCEDURE DummyProc 
BEGIN TRY
      BEGIN TRANSACTION
      INSERT sometable(a, b) VALUES (@a, @b)
      INSERT sometable(a, b) VALUES (@b, @a)

   END TRY
   BEGIN CATCH
      IF @@trancount > 0 ROLLBACK TRANSACTION
      DECLARE @msg nvarchar(2048) = error_message()  
      RAISERROR (@msg, 16, 1)
      RETURN 55555
   END CATCH
   IF @@trancount > 0  COMMIT TRANSACTION

In Option B, is there a possibility of some error happening when its doing a commit outside the TRY-CATCH block ?

Answer 1

The best way I have found to do this is the following code:

SET XACT_ABORT ON;

BEGIN TRY
    BEGIN TRANSACTION

    /*
        Code goes here
    */

    COMMIT TRANSACTION
END TRY
BEGIN CATCH

    DECLARE @ErrorMessage NVARCHAR(4000);
    DECLARE @ErrorSeverity INT;
    DECLARE @ErrorState INT;

    SELECT 
        @ErrorMessage = ERROR_MESSAGE(),
        @ErrorSeverity = ERROR_SEVERITY(),
        @ErrorState = ERROR_STATE();

    RAISERROR (@ErrorMessage, -- Message text.
               @ErrorSeverity, -- Severity.
               @ErrorState -- State.
               );

    -- If >= SQL 2012 replace all code in catch block above with
    -- THROW;

    WHILE @@TRANCOUNT > 0
    BEGIN
        ROLLBACK TRANSACTION;
    END

END CATCH

Note the use of XACT_ABORT to ensure the errors are caught effectively, the fact that both the BEGIN and COMMIT statements are within the TRY block, and the WHILE for @@Trancount - this should ensure nested transactions are rolled back (not always applicable)

The THROW statement can also replace RAISERROR for SQL versions above 2012, to rethrow the caught exception / error.

As Dan Guzman states in his comment, XACT_ABORT is useful for catching errors that the TRY/CATCH construct won't, including timeouts, runtime collation errors etc

Answer 2

Option A is the correct choice. It is possible for all statements in a transaction to work and then the actual COMMIT to fail, so you keep the COMMIT inside your TRY block so that any failure of the COMMIT will be caught and you can gracefully handle this error and rollback.

See this answer on SE. In your example code, if we assume other users are running this procedure concurrently it might be possible for the TRY block to complete successfully and then fail on the COMMIT due to changes made by other sessions.