Nested stored procedures containing TRY CATCH ROLLBACK pattern?

Question

I'm interested in the side effects and potential problems of the following pattern:

CREATE PROCEDURE [Name]
AS
BEGIN
    BEGIN TRANSACTION
    BEGIN TRY
        [...Perform work, call nested procedures...]
    END TRY
    BEGIN CATCH
        ROLLBACK TRANSACTION
        RAISERROR [rethrow caught error using @ErrorNumber, @ErrorMessage, etc]
    END CATCH
END

To the best of my understanding this pattern is sound when used with a single procedure - the procedure will either complete all of its statements without error, or it will rollback all actions and report the error.

However when one stored procedure calls another stored procedure to do some sub-unit of work (with the understanding that the smaller procedure is sometimes called on its own) I see an issue coming about with relation to rollbacks - an informational message (Level 16) is issued stating The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION.. This I assume is because the rollback in the sub-procedure is always rolling back the outer-most transaction, not just the transaction started in the sub-procedure.

I do want the whole thing rolled back and aborted if any error occurs (and the error reported to the client as an SQL error), I'm just not sure of all the side effects that come from the outer layers trying to rollback a transaction that has already been rolled back. Perhaps a check of @@TRANCOUNT before doing a rollback at each TRY CATCH layer?

Finally there is the client end (Linq2SQL), which has it's own transaction layer:

try
{
    var context = new MyDataContext();
    using (var transaction = new TransactionScope())
    {       
            // Some Linq stuff
        context.SubmitChanges();
        context.MyStoredProcedure();
        transactionComplete();
    }
}
catch
{
    // An error occured!
}

In the event that a stored procedure, "MySubProcedure", called inside MyStoredProcedure raises an error, can I be sure that everything previously done in MyStoredProcedure will be rolled back, all the Linq operations made by SubmitChanges will be rolled back, and finally that the error will be logged? Or what do I need to change in my pattern to ensure the whole operation is atomic, while still allowing the child parts to be used individually (i.e. the sub-procedures should still have the same atomic protection)

Terminology

If your sub procedure is relying on something from your outer procedure (ie. transaction) then it seems to me like there's an awful lot of coupling where there shouldn't be. Edit: I think I read your issue wrong – Coenzyme

Answer 1

This is our template (error logging removed)

This is designed to handle

• Paul Randal's article "No such thing as a nested transaction in SQL Server"
• Error 266
• Trigger Rollbacks

Explanations:

• all TXN begin and commit/rollbacks must be paired so that @@TRANCOUNT is the same on entry and exit

• mismatches of @@TRANCOUNT cause error 266 because

  • BEGIN TRAN increments @@TRANCOUNT

  • COMMIT decrements @@TRANCOUNT

  • ROLLBACK returns @@TRANCOUNT to zero

• You can not decrement @@TRANCOUNT for the current scope
  This is what you'd think is the "inner transaction"

• SET XACT_ABORT ON suppresses error 266 caused by mismatched @@TRANCOUNT
  And also deals with issues like this "SQL Server Transaction Timeout" on dba.se

• This allows for client side TXNs (like LINQ) A single stored procedure may be part of distributed or XA transaction, or simply one initiated in client code (say .net TransactionScope)

Usage:

• Each stored proc must conform to the same template

Summary

• So don't create more TXNs than you need

The code

CREATE PROCEDURE [Name]
AS
SET XACT_ABORT, NOCOUNT ON

DECLARE @starttrancount int

BEGIN TRY
    SELECT @starttrancount = @@TRANCOUNT

    IF @starttrancount = 0
        BEGIN TRANSACTION

       [...Perform work, call nested procedures...]

    IF @starttrancount = 0 
        COMMIT TRANSACTION
END TRY
BEGIN CATCH
    IF XACT_STATE() <> 0 AND @starttrancount = 0 
        ROLLBACK TRANSACTION;
    THROW;
    --before SQL Server 2012 use 
    --RAISERROR [rethrow caught error using @ErrorNumber, @ErrorMessage, etc]
END CATCH
GO

Notes:

• The rollback check is actually redundant because of SET XACT_ABORT ON. However, it makes me feel better, looks odd without, and allows for situations where you don't want it on

• Remus Rusanu has a similar shell that uses save points. I prefer an atomic DB call and don't use partial updates like their article

Sentence

SET NOCOUNT ON doesn't seem necessary? – Eliaseliason

You should always use SET NOCOUNT ON imho. – Victoir

@Carl R: SET NOCOUNT ON is pretty much essential in code and triggers: so much client code breaks without it. Also, there it's possible to measure the time taken to process the "xx rows affected messages". See https://mcmap.net/q/47503/-set-nocount-on-usage/27535 (my question) please – Sentence

I thought the @@ROWCOUNT wasn't updated with it on, but I was wrong. Thanks! – Eliaseliason

Ah! I've been using RecordsAffected a lot in a previous project. Had completely forgot about that. – Eliaseliason

@Carl R: RecordsAffected would nbe Ok for a direct INSERT or UPDATE. Via a stored proc (or if there is a trigger, then this most likely be lost eg the INSERT into some audit table. Hence the breaks without using SET NOCOUNT ON – Sentence

@Sentence this pattern makes error handling on the client more difficult. Instead of catching SqlException, and switching on error number of the original exception, I am not getting the original error number. All I can do is parse the text, which is prone to errors. – Gerrigerrie

Yes, one downside. Denali fixes this with Throw. But until then you'd always lose the original error anyway in the catch block, with the trade off for predictabilty of error handlin – Sentence

Do I understand that this provides behavior different than Remus'? My understanding is that his will allow the outer transaction to complete even if the inner transaction fails (thus SAVE TRANSACTION), whereas this will roll back the entire operation if any nested procedure rolls back. Is that understanding correct? – Tropism

@EricJ. Correct. I roll it all back – Sentence

We don’t need transaction for inner stored procs. If we are using transaction in outer most stored procedure it will take care of all inner stored procedures, any error occured in inner stored procedures will be passed to outermost stored procedure and will casue a rollback.So we dont need nested transactions at all. – Kaja

@Niraj: Correct, but then you get error 266 unless you have SET XACT_ABORT ON. – Sentence

@gbn: I wouldn't, cause I am not using transactions in inner stored procs I am using transaction at only one place in the outer most stored procedure only. I raise error from inner stored procedure and catch them in outer transaction and I don't get an error on rollback. – Kaja

@Niraj: what if you decide to nest stored procedures deeper, or want to call the inner one directly. You are making assumptions. You can still get error 266 if the inner @@TRANCOUNT goes to zero by, say, a batch-aborting trigger error (if you don't have TRY/CATCH). What works for you in a controlled environment is not* the general case that works for everyone. My code is safe for this general case. – Sentence

@Sentence I agree your code is better for a scenario where my inner procedures can be called directly also by client apps. In my case it was not the case. In my case since I dont have transactions in inner procs and I use try catch always may be thats why I dont get error 266. Thanks for your sincere reply. – Kaja

Why use RAISERROR in your catch block instead of THROW? I thought THROW was the preferred option when using TRY/CATCH, and it seems simpler. – Asseveration

Never mind. I now see that this answer predates SQL Server 2012, so TRHOW was not available. But perhaps it should be updated to use THROW, and add a note to use RAISERROR instead if the server version predates SS 2012. – Asseveration

@gbn, could you please estimate performance impact or other drawbacks of this altenative to simple cases: https://mcmap.net/q/183171/-nested-stored-procedures-containing-try-catch-rollback-pattern? – Excavate

XACT_ABORT ON setting essentially means any code in the catch block will NOT execute. The SP above works because the rollback is superfluous as mentioned in the first note by the author. If you added any error logging in the catch block, then use of XACT_ABORT flag I think will invalidate it. Can we safely eliminate xact_abort on statement from the template above?social.technet.microsoft.com/wiki/contents/articles/… – Carman

Answer 2

I am not a Linq guy (and neither is Erland), but he wrote the absolute bibles on error handling. Outside of the complications Linq might add to your problem, all of your other questions should be answered here:

http://www.sommarskog.se/error_handling/Part1.html

(Old link: http://www.sommarskog.se/error_handling_2005.html)

Victoir

Answer 3

To solve the issue of returning the error number and line number mentioned by @AlexKuznetsov, one can raise the error as such:

DECLARE @ErrorMessage NVARCHAR(4000)
DECLARE @ErrorSeverity INT
DECLARE @ErrorState INT
DECLARE @ErrorLine INT
DECLARE @ErrorNumber INT

SELECT @ErrorMessage = ERROR_MESSAGE(),
@ErrorSeverity = ERROR_SEVERITY(),
@ErrorState = ERROR_STATE(),
@ErrorNumber = ERROR_NUMBER(),
@ErrorLine = ERROR_LINE()

RAISERROR (@ErrorMessage, @ErrorSeverity, @ErrorState, @ErrorNumber, @ErrorLine)

Palaeolithic

Answer 4

-- @Amanda method above doesnt return correct error number

DECLARE  
  @ErrorMessage   nvarchar(4000),  
  @ErrorSeverity   int,  
  @ErrorState int,  
  @ErrorLine  int,  
  @ErrorNumber   int  

BEGIN TRY  
 SELECT 1/0; -- CATCH me  
END TRY  

BEGIN CATCH  

  DECLARE @err int = @@ERROR  

  PRINT @err           -- 8134, divide by zero  
  PRINT ERROR_NUMBER() -- 8134  

  SELECT  
    @ErrorMessage  = ERROR_MESSAGE(),  
    @ErrorSeverity = ERROR_SEVERITY(),  
    @ErrorState    = ERROR_STATE(),  
    @ErrorNumber   = ERROR_NUMBER(),  
    @ErrorLine     = ERROR_LINE()  

  -- error number = 50000 :(  
  RAISERROR (@ErrorMessage, @ErrorSeverity, @ErrorState, @ErrorNumber, @ErrorLine)  

END CATCH  

-- error number = 8134  
SELECT 1/0

Radiotransparent

You are right, but this belongs as a comment on Amanda's answer. – Cassy

Answer 5

In case no special error handling needed in CATCH except rethrow and stored procs call chain isn't too long it may be suitable to use such simple template:

create procedure someNestedSP
as
SET XACT_ABORT ON
begin transaction
-- do some work or call some other similar SP
commit transaction

It would also rollback root transaction with all "nested" ones in case of any error but the code is shorter and more straightforward than @gbn's solution. Still XACT_ABORT takes care of most issues mentioned there.

There may be addiotional overhead for transaction nesting but it may be not too high, I guess.

Excavate

The TRY/CATCH blocks give more predictable error handling, especially where triggers are involved. Also, this pattern gives no option to trap, log or rethrow the error. Or to throw your own if data validation fails. And every one else uses a similar pattern to me google.ch/…* Your choice, but this code would never go live in my work place. – Sentence