sql try/catch rollback/commit - preventing erroneous commit after rollback

I am trying to write an MS sql script that has a transaction and a try/catch block. If it catches an exception, the transaction is rolled back. If not, the transaction is committed. I have seen a few different websites saying to do it like this:

begin transaction
begin try
    --main content of script here
end try
begin catch
    rollback transaction
end catch

commit transaction

But won't we still hit the "commit transaction" line even in the case of catching an exception? Won't this lead to a SQL error because the transaction has already been rolled back? I think it should be done like this:

declare @success bit = 1

begin transaction
begin try
    --main content of script here
end try
begin catch
    rollback transaction
    set @success = 0
end catch

if(@success = 1)
begin
    commit transaction
end

Howcome the commonly-posted solution does not include the @success variable? Is there no sql error that happens as a result of committing a transaction that has already been rolled back? Am I incorrect in saying that the "commit transaction" line of the first code example will still be hit in the case of catching an exception?

I always thought http://www.4guysfromrolla.com/webtech/041906-1.shtml">this was one of the better articles on the subject. It includes the following example that I think makes it clear and includes the frequently overlooked @@trancount which is needed for reliable nested transactions

PRINT 'BEFORE TRY'
BEGIN TRY
    BEGIN TRAN
     PRINT 'First Statement in the TRY block'
     INSERT INTO dbo.Account(AccountId, Name , Balance) VALUES(1, 'Account1',  10000)
     UPDATE dbo.Account SET Balance = Balance + CAST('TEN THOUSAND' AS MONEY) WHERE AccountId = 1
     INSERT INTO dbo.Account(AccountId, Name , Balance) VALUES(2, 'Account2',  20000)
     PRINT 'Last Statement in the TRY block'
    COMMIT TRAN
END TRY
BEGIN CATCH
    PRINT 'In CATCH Block'
    IF(@@TRANCOUNT > 0)
        ROLLBACK TRAN;

    THROW; -- raise error to the client
END CATCH
PRINT 'After END CATCH'
SELECT * FROM dbo.Account WITH(NOLOCK)
GO

In your first example, you are correct. The batch will hit the commit transaction, regardless of whether the try block fires.

In your second example, I agree with other commenters. Using the success flag is unnecessary.

I consider the following approach to be, essentially, a light weight best practice approach.

If you want to see how it handles an exception, change the value on the second insert from 255 to 256.

CREATE TABLE #TEMP ( ID TINYINT NOT NULL );
INSERT  INTO #TEMP( ID ) VALUES  ( 1 )

BEGIN TRY
    BEGIN TRANSACTION

    INSERT  INTO #TEMP( ID ) VALUES  ( 2 )
    INSERT  INTO #TEMP( ID ) VALUES  ( 255 )

    COMMIT TRANSACTION
END TRY
BEGIN CATCH
    DECLARE 
		@ErrorMessage NVARCHAR(4000),
		@ErrorSeverity INT,
		@ErrorState INT;
    SELECT 
        @ErrorMessage = ERROR_MESSAGE(),
        @ErrorSeverity = ERROR_SEVERITY(),
        @ErrorState = ERROR_STATE();
    RAISERROR (
		@ErrorMessage,
		@ErrorSeverity,
		@ErrorState    
		);
    ROLLBACK TRANSACTION
END CATCH

SET NOCOUNT ON

SELECT ID
FROM #TEMP

DROP TABLE #TEMP

I used below ms sql script pattern several times successfully which uses Try-Catch,Commit Transaction- Rollback Transaction,Error Tracking.

Your TRY block will be as follows

 BEGIN TRY
 BEGIN TRANSACTION T
 ----
 //your script block
 ----
 COMMIT TRANSACTION T 
 END TRY

Your CATCH block will be as follows

BEGIN CATCH
DECLARE @ErrMsg NVarChar(4000), 
        @ErrNum Int, 
        @ErrSeverity Int, 
        @ErrState Int, 
        @ErrLine Int, 
        @ErrProc NVarChar(200)
 SELECT @ErrNum = Error_Number(), 
       @ErrSeverity = Error_Severity(), 
       @ErrState = Error_State(), 
	   @ErrLine = Error_Line(), 
	   @ErrProc = IsNull(Error_Procedure(), '-')
 SET @ErrMsg = N'ErrLine: ' + rtrim(@ErrLine) + ', proc: ' + RTRIM(@ErrProc) + ', 
       Message: '+ Error_Message()

Your ROLLBACK script will be part of CATCH block as follows

IF (@@TRANCOUNT) > 0 
BEGIN
PRINT 'ROLLBACK: ' + SUBSTRING(@ErrMsg,1,4000)
ROLLBACK TRANSACTION T
END
ELSE
BEGIN
PRINT SUBSTRING(@ErrMsg,1,4000);   
END

END CATCH

Above different script blocks you need to use as one block. If any error happens in the TRY block it will go the the CATCH block. There it is setting various details about the error number,error severity,error line ..etc. At last all these details will get append to @ErrMsg parameter. Then it will check for the count of transaction (@@TRANCOUNT >0) , ie if anything is there in the transaction for rollback. If it is there then show the error message and ROLLBACK TRANSACTION. Otherwise simply print the error message.

We have kept our COMMIT TRANSACTION T script towards the last line of TRY block in order to make sure that it should commit the transaction(final change in the database) only after all the code in the TRY block has run successfully.

Transaction counter

--@@TRANCOUNT = 0
begin try
--@@TRANCOUNT = 0
BEGIN TRANSACTION tran1
 --@@TRANCOUNT = 1
	
		--your code
		-- if failed  @@TRANCOUNT = 1
		-- if success @@TRANCOUNT = 0

COMMIT TRANSACTION tran1

end try

begin catch
    print 'FAILED'
end catch

Below might be useful.

Source: https://msdn.microsoft.com/en-us/library/ms175976.aspx

BEGIN TRANSACTION;

BEGIN TRY
	-- your code --
END TRY
BEGIN CATCH
	SELECT 
		ERROR_NUMBER() AS ErrorNumber
		,ERROR_SEVERITY() AS ErrorSeverity
		,ERROR_STATE() AS ErrorState
		,ERROR_PROCEDURE() AS ErrorProcedure
		,ERROR_LINE() AS ErrorLine
		,ERROR_MESSAGE() AS ErrorMessage;

	IF @@TRANCOUNT > 0
		ROLLBACK TRANSACTION;
END CATCH;

IF @@TRANCOUNT > 0
	COMMIT TRANSACTION;
GO