Try Catch alters behaviour of existing procedures

  • this code shows that calling an existing stored procedure inside a try block alters its behaviour

    that seems like a bug / mistake to me

    go

    create procdbo.spTryTestA

    as

    --

    raiserror('spTryTestA:1', 0, 1)

    raiserror('spTryTestA:2', 16, 1)

    raiserror('spTryTestA:3', 0, 1)

    --(end)

    go

    create procdbo.spTryTestB

    as

    --

    begin try

    raiserror('spTryTestB:1', 0, 1)

    exec dbo.spTryTestA

    raiserror('spTryTestB:2', 0, 1)

    end try

    begin catch

    raiserror('spTryTestB:3', 0, 1)

    end catch

    go

    exec dbo.spTryTestA

    exec dbo.spTryTestB

    go

    spTryTestA:1

    Msg 50000, Level 16, State 1, Procedure spTryTestA, Line 6

    spTryTestA:2

    spTryTestA:3 << codepath reaches here

    spTryTestB:1

    spTryTestA:1 << oh, codepath exits

    spTryTestB:3

    this means that all SPs need to be redesigned to detect whether they are called from inside try block or not!

    in my databases - in most cases it is far easier and cleaner to avoid try catch than to use it

    for example:

    old style:

    if @@error <> 0 or @@rowcount < 1 goto label_rollback

    new style:

    if @@rowcount < 1 raiserror('@@rowcount < 1', 16, 1)

    overall the try catch actually increases the amount of code you need to write

    and can decrease the readability and reliability

    as I have 100s of SPs from before TRY CATCH I say it is too risky to use try catch and there is nothing to be gained anyway

    it's another T-SQL fail - keeps us in work though *-)

  • Perhaps I am being dense, but the entire purpose of TRY...CATCH is to change behavior. Now, I could understand if you don't like HOW it changes behavior, but I am having a hard time understanding what you'd expect it to do if it doesn't change behavior.

    I do find it has some flaws, but still - it seems to be to be a huge step forward from continuously having to check @@ERROR after each operation lest you miss a single-non-breaking exception.

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • the problem is that it changes the behaviour of OTHER pre-existing stored procedures

    that already have a defined and tested behaviour

    it breaks them

    a simple example:

    if @@error <> 0 goto label_do_something_else

    won't work if the sp is called from a TRY block

    but will work if called normally

    so a pre-defined behaviour has changed in a subtle way

    as an example, in csharp, calling a method from within a try block or not does not change the called method

    because that would be insane!

  • Matt Miller (#4) (7/6/2010)I do find it has some flaws, but still - it seems to be to be a huge step forward from continuously having to check @@ERROR after each operation lest you miss a single-non-breaking exception.

    but you still have to check

    if @@rowcount < 1

    after updates and deletes

    and there isn't much difference between

    if @@error <> 0 or @@rowcount < 1 goto label_rollback

    and

    if @@rowcount < 1 raiserror('oh no', 16, 1)

    plus you still need to check XACT_STATE

    and even need to check for @@trancount > 1

    and I find that once a big SP has been converted to use TRY CATCH

    it is usually more verbose and less readable

    and why no THROW?

    instead I use this:

    create proc dbo.spThrowError

    as

    --

    declare@errmsgnvarchar(2048)

    declare@errnoint

    declare@errseverityint

    declare@errstateint

    declare@errlineint

    declare@errprocnvarchar(126)

    --

    select@errmsg= error_message()

    ,@errno= error_number()

    ,@errseverity= error_severity()

    ,@errstate= error_state()

    ,@errline= error_line()

    ,@errproc= coalesce(error_procedure(), '-')

    --

    raiserror(

    @errmsg-- msg

    ,@errseverity-- severity

    ,1-- state

    ,@errno-- argument

    ,@errseverity-- argument

    ,@errstate-- argument

    ,@errproc-- argument

    ,@errline-- argument

    )

    --(end)

    is there anyway to fix the tabs on this website - the combination of proportional font in the editor

    and 8 space tabs in the preview is a nightmare - the default for TSQL has been 4 spaces for years

  • But if you have pre-existing procs with established and functioning logic, why are you changing them to use TRY/CATCH at all? I would only suggest rewriting them as needed and migrating them to TRY/CATCH, and yes, updating the logic then.

    TRY/CATCH does work differently than @@error and thank the gods that it does. For example, please show me how to catch a deadlock error and resubmit the query without using TRY/CATCH.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • Grant Fritchey (7/7/2010)


    But if you have pre-existing procs with established and functioning logic, why are you changing them to use TRY/CATCH at all? I would only suggest rewriting them as needed and migrating them to TRY/CATCH, and yes, updating the logic then.

    TRY/CATCH does work differently than @@error and thank the gods that it does. For example, please show me how to catch a deadlock error and resubmit the query without using TRY/CATCH.

    Why are people struggling to understand this simple post?

    - I have an EXISTING stored procedure that WORKS FINE "dbo.spWorks"

    - if I call this stored procedure "dbo.spWorks" from OUTSIDE a TRY block - it works OK

    - if I call this stored procedure "dbo.spWorks" from INSIDE a TRY block - it does not work any more

    I am NOT changing anything - just CALLING the SAME, UNCHANGED procedure from both INSIDE and OUTSIDE TRY blocks

    Excuse the caps but I am getting frustrated when people are missing such a simple point

  • You're dealing with a fundamental change in behavior. If you're raising errors inside the procedure, the errors are recognized by the TRY/CATCH construct, as designed. That's how it's supposed to work. Your previous construct worked for you, but it's not functional within the new construct.

    Things change. You can't use *= style joins in 2008 now. Code behaviors get deprecated and if you want to move on with new versions of the software, you have to deal with it.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • Grant Fritchey (7/7/2010)


    You're dealing with a fundamental change in behavior. If you're raising errors inside the procedure, the errors are recognized by the TRY/CATCH construct, as designed. That's how it's supposed to work. Your previous construct worked for you, but it's not functional within the new construct.

    Things change. You can't use *= style joins in 2008 now. Code behaviors get deprecated and if you want to move on with new versions of the software, you have to deal with it.

    Yes - that is the point I was trying to make:

    watch out! using try catch will change, even break, *existing* stored procedures

    that isn't obvious - in fact nowhere have I seen it mentioned except in my posts

    in all the other languages I use a procedure is a predefined behaviour

    the only way to modify its behaviour is by changing the parameters

    this means if you have a library of stored procedures - you have to rewrite all of them, in case any of them are called from within a try block

    the way MS should have done it:

    that existing procedures that are not using try blocks should behave the SAME

    but if, upon exit, @@error <> 0 THEN catch the error in the parent try block

    that would be sensible - the current approach makes no sense

  • doobya (7/7/2010)


    Grant Fritchey (7/7/2010)


    You're dealing with a fundamental change in behavior. If you're raising errors inside the procedure, the errors are recognized by the TRY/CATCH construct, as designed. That's how it's supposed to work. Your previous construct worked for you, but it's not functional within the new construct.

    Things change. You can't use *= style joins in 2008 now. Code behaviors get deprecated and if you want to move on with new versions of the software, you have to deal with it.

    Yes - that is the point I was trying to make:

    watch out! using try catch will change, even break, *existing* stored procedures

    that isn't obvious - in fact nowhere have I seen it mentioned except in my posts

    in all the other languages I use a procedure is a predefined behaviour

    the only way to modify its behaviour is by changing the parameters

    this means if you have a library of stored procedures - you have to rewrite all of them, in case any of them are called from within a try block

    the way MS should have done it:

    that existing procedures that are not using try blocks should behave the SAME

    but if, upon exit, @@error <> 0 THEN catch the error in the parent try block

    that would be sensible - the current approach makes no sense

    Absolutely not starting a fight, but I do disagree. I think your approach to have multiple errors raised and expect to get and see multiple errors, on purpose, is the issue, not the implementation of TRY/CATCH. That's not an approach I'd take or advocate. If you're trying to communicate messages back to the calling application, there are better ways to do it rather than RAISERROR.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • I'm sorry to have to say it - but you still do not understand

    OO

  • I understand. But you have control of the behavior based on the error severity level.

    From Books Online (the section on RAISERROR):

    When RAISERROR is run with a severity of 11 or higher in a TRY block, it transfers control to the associated CATCH block. The error is returned to the caller if RAISERROR is run:

    Outside the scope of any TRY block.

    With a severity of 10 or lower in a TRY block.

    With a severity of 20 or higher that terminates the database connection.

    In your example, change the error severity to 9 instead of 16 and you get the consistent behavior you are after. The various behaviors based on error severity are by design. In some cases a procedure cannot and should not proceed after a severe error.

    __________________________________________________

    Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
    Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills

  • The Dixie Flatline (7/7/2010)In your example, change the error severity to 9 instead of 16 and you get the consistent behavior you are after. The various behaviors based on error severity are by design. In some cases a procedure cannot and should not proceed after a severe error.

    I cannot control the severity level of errors

    that example is contrived to prove that the codepath is altered

    in real life it will be a DML statement that is raising the error

    update dbo.Table

    set [Field] = 'value'

    where [OtherField] = 'otherValue'

    if @@error <> 0 or @@rowcount <> 1 goto label_do_something --<< when this SP is called from another SP within a TRY block it may not reach this line

  • doobya,

    Let me give you a few suggestions:

    #1. Consider that maybe it's you who isn't understanding what's being said. You're arguing with someone who writes top notch books on SQL Server. I'm personally not inclined to think that he's the one that doesn't get it.

    #2. Take a deep breath and understand how TRY...CATCH blocks are designed to work, not how you think they should work had you designed SQL Server yourself. The whole point of the block is to exit to the CATCH when the error is encountered rather than continuing to process. What you're doing is like complaining about how your TV no longer works after you blow the circuit by purposely overloading it. Of course it doesn't work, it's designed specifically not to work.

    If you want 'Y' to happen regardless of what happens with 'X' then don't put it in the same TRY.

    └> bt



    Forum Etiquette: How to post data/code on a forum to get the best help[/url]

  • bteraberry (7/7/2010)If you want 'Y' to happen regardless of what happens with 'X' then don't put it in the same TRY.

    that is what I *am* doing! X and Y are in a SP with NO TRY BLOCK

    but the CALLING procedure IS using a TRY block and therefore breaks the tested and working logic

    my very first post explains this with working code

    what MS has done is a complete mess, I have described the correct approach that they should have taken

    which would allow procedures both with and without TRY blocks to work together perfectly, which I will explain again:

    for code in a procedure that is outside of a LOCAL TRY block

    that procedure *should* behave as it was written - no modification to existing behaviour

    whether or not the CALLING procedure is using a LOCAL TRY block or not

    IF the calling procedure IS using a LOCAL TRY block and calls a procedure which returns with @@error <> 0

    (and severity level > 10) THEN control should be passed to the CATCH block

    that is the correct, predictable and reliable way to implement it

    I doubt Microsoft would have made such a terrible mistake without reason - maybe there is some insurmountable

    hurdle to implementing it correctly - and they had no choice - who knows

    but the purpose of my post is to warn people - beware - this behaviour is not what the majority of developers would expect

    and WILL CATCH YOU OUT (no pun intended) especially if you have an existing library of stored procedures that WILL BREAK if they are called with a call stack that includes a TRY block at any level

    you can not simply convert a SQL 2000 database to SQL 2005 and expect to get away with it

    the only safe approach is to rewrite ALL existing stored procedures to work whether called within a TRY block or not ...

    Is that common knowledge or not?

    If it is common knowledge please supply a link to an explanation and I apologise

    Otherwise please thank me for pointing it out

    But please don't miss the point

  • Also, you can take a look at what happens in your example when you change the proc dbo.spTryTestA:

    create proc dbo.spTryTestA

    as

    --

    begin try

    raiserror('spTryTestA:1', 0, 1)

    end try

    begin catch

    raiserror('spTryTestA:1 failed', 0, 1)

    end catch

    begin try

    raiserror('spTryTestA:2', 16, 1)

    end try

    begin catch

    raiserror('spTryTestA:2 failed', 0, 1)

    end catch

    begin try

    raiserror('spTryTestA:3', 0, 1)

    end try

    begin catch

    raiserror('spTryTestA:3 failed', 0, 1)

    end catch

    --(end)

    go

    This returns the same results either way. The issue in your example is that you've got a call to a ghetto proc within a TRY block. Now, there is nothing to be ashamed of ... we all have old code around that doesn't truly have error handling (checking @@ERROR is not proper error handling), but you can't make calls from that code from within a TRY block if you don't want the errors in the code to bubble up to the TRY. If you want to do execute legacy code, it's a pretty simple matter to move the call outside the TRY block.

    Now, if your original stated opinion was that TRY...CATCH doesn't work well to make calls to archaic procs then I would wholly agree.

    └> bt



    Forum Etiquette: How to post data/code on a forum to get the best help[/url]

Viewing 15 posts - 1 through 15 (of 81 total)

You must be logged in to reply to this topic. Login to reply