One of our clients has just developed a set of coding standards for their
..NET and SQL Server application development. For the SQL Server standards,
they say the following regarding error handling:

-----------start of quote---------------
Error Handling
- Should be used in every stored procedure, with an Error Handling section
at the bottom of the procedure
- Stored procedures should return the success or failure of the stored
procedure by returning 0 (zero) upon success, or return the error number upon
error.
- @@ERROR should be checked after Every INSERT or UPDATE to deterimine the
success or failure of that action.
- @@ROWCOUNT should be checked after every INSERT or UPDATE if at least one
or more records were expected to be affected.

  DECLARE @Error int, @RowCount int

T-SQL here….

  RETURN 0
EH:
  RAISERROR(@Error, 16, 1)
  RETURN @Error

-----------end of quote---------------

Does this sound like a good idea? Having an error handler in every stored
procedure seems excessive.

Also, I don't think the RETURN @Error statement ever executes because the
RAISERROR causes the sproc to immediately exit.

NeoRev wrote:
Yes, it is a good idea to have error handling in every stored procedure.

Yes, it will execute. RAISERROR will raise an error but the next
statement will execute. Try this:

CREATE PROCEDURE Test
AS
   SELECT 1;
   RAISERROR('Error!', 16, 1);
   SELECT @@ERROR;

GO

EXEC Test;

Two great articles on error handling by Erland Sommarskog (the second is
more specific for stored procedures, but both are worth reading):
http://www.sommarskog.se/error-handling-I.html
http://www.sommarskog.se/error-handling-II.html

If this is SQL Server 2005/2008, then error handling should be done by
using TRY..CATCH rather than the old @@ERROR method.

Thanks for the articles.  I will try to read them tonight or over the weekend.

Do you also believe that it is a good idea to have error handling in every
application method (whether VB.NET or C#), too?

Show quoteHide quote

Yes, in my opinion it is best practice to implement error handling in
every application method. Otherwise you always leave a chance that
something can go wrong and unhandled.

Actually, that's a bad practice.  I'll try to dig up some articles for you to
read.

Show quoteHide quote

Here's that article that I recommended.  It's old and was written back in
classic VB days, but the conceptual information is still valid.  I'd read up
to 'Method 1 (Low Tech)":

http://web.archive.org/web/20010215021254/www.keysound.com/html/frameworks.htm

I've actually contacted the author to see if he'd update it for .NET.  If a
lot of people are writing exception handlers in all their .NET methods,
they're following bad practices. 
Show quoteHide quote

The way I read the article it actually advertises to use error handling
in all routines, even in the same "Method 1 (Low Tech)" section shows
how to create standard templates:

-------------------------------------------
Standard Templates

A good start to any project is to code the following standard templates
in most significant functions and all event handlers. This gives a basic
error reporting infrastructure that can be expanded by adding code to
the Select statement to deal with specific errors. Although the Catch
function described earlier will work for classes as well as forms, we
may need to consider how desirable it is to display a Message Box from a
class module, and perhaps recode Catch accordingly.

for functions:

ExitBlock:
     Exit Sub
ErrorBlock:
     Select Case Err.Number
         ` Add specific error handling here as required.
         Case Else
             GError.Throw TypeName(Me), "FuncName"
     End If
End Sub

for event handlers:

ExitBlock:
     Exit Sub
ErrorBlock:
     Select Case Err.Number
         ` Add specific error handling here as required.
         Case Else
             GError.Catch TypeName(Me), "FuncName"
     End If
End Sub

-------------------------------------------

You should read more serious studies on the topic, for example like this
one (it requires ACM membership, but the abstract is clear enough):
A Systems Engineering Approach to Exception Handling
http://portal.acm.org/citation.cfm?id=1396765

There have been also numerous studies by NASA and other organization
that have achieved the highest quality of coding practices and they all
indicate exception handling at all levels is a must.

Yes, but he's only doing that to build a call stack.  .NET keeps track of the
call stack for automatically, so this is no longer needed.  (This is one of
the things that needs to be updated.)

My first job out of school I worked on a very buggy application so error
handling became a topic I researched a lot.  In fact, I wrote and marketed an
error reporting DLL that was featured on DevX and VBtotheMax.com.  When I
coded in VB6, I always put an error handler in every routine.  But given that
..NET now keeps track of the call stack and even allows you to create a
centralized error handler (at least for Windows Forms app), all this boiler
plate code is simply no longer necessary. 

Honestly, error handling for the sake of error handling is a waste of time
and only serves to obsficate your code.  The only reason why you should
handle an error is if you're going to do something meaningful with the
exception. For example, you know what the error is and how to fix it.  Or
maybe you want to log it to the database.  Or you need to rollback a
transaction.  But if all you're going to do is catch it and raise it up the
call stack, that's pointless code because the error will bubble up the
callstack anyway. 

Show quoteHide quote

On Nov 20, 1:59 pm, NeoRev <Neo***@discussions.microsoft.com> wrote:
Show quoteHide quoteAlso you need to unit test your error handling:

http://www.simple-talk.com/sql/t-sql-programming/close-these-loopholes---reproduce-database-errors/

OK, I've finally had a chance to read both of those articles.  Neither really
contradict what I'm saying.  In fact, in the second article, the author
states that he doesn't recommend checking @@error after SELECT statements and
rarely does so himself.  He also states:

"There are situations when checking @@error is unnecessary, or even
meaningless."

Now, if you have a stored procedure that executes multiple statements, sure
it makes sense to have an error handler in the stored procedure.  But if all
you're doing is a single statement, it's probably best to let unanticipated
errors bubble up the call stack.

Show quoteHide quote

You have to decide what makes best sense to your application. But in
general I would prefer to handle the error as it occurs, especially in
stored procedures.

Again, if you use SQL Server 2005/2008, do not even waste time on
@@ERROR. TRY..CATCH is far more superior.

Also we need to realize that TRY... CATCH does not catch all the
errors - some errors (such as timeout) will not be caught.

Are there any other errors TRY...CATCH doesn't catch?  Is this documented
anywhere?

Show quoteHide quote

TRY CATCH will catch all execution errors with severity greater than 10
that do not terminate the database connection. Errors that terminate the
database connection (usually with severity from 20 through 25) are not
handled by the CATCH block because execution is aborted when the
connection terminates. But if the connection is not terminated errors
with severity higher than 20 will also be handled.

Errors with severity 0 to 10 are informational messages.

The best place is to review the Books Online documentation for TRY CATCH:
http://msdn.microsoft.com/en-us/library/ms175976.aspx
http://msdn.microsoft.com/en-us/library/ms179296.aspx

More on connection termination errors:
http://www.sommarskog.se/error-handling-I.html#connection-termination

So is that any different from old style error handling using @Error?  IOW, is
there anything that @Error will 'catch' that TRY...CATCH won't?

Show quoteHide quote

Yes, TRY CATCH it is much different that the old style using @@ERROR.
Since @@ERROR is reset after each statement you have to place error
checking code after each statement. With TRY CATCH you just wrap all
statements in a single error handling construct.

Also, on statement abort error, with @@ERROR the client still gets error
message, and with TRY CATCH you completely handle the error server side.
Here is one example:
http://blogs.msdn.com/sqlprogrammability/archive/2006/03/30/565141.aspx

I'm sorry.  I meant, are there any errors that @Error will find that
TRY...CATCH won't (besides informational messages)?

Show quoteHide quote

None that I am aware of. The opposite is true, here is example of error
which @@ERROR does not catch but can be handled using TRY CATCH:

CREATE TABLE Foo (
  col1 INT);

INSERT INTO Foo VALUES(1);
INSERT INTO Foo VALUES(2);
INSERT INTO Foo VALUES(1);

GO

PRINT 'Error handling with @@ERROR';

CREATE UNIQUE INDEX ix_Foo
ON Foo(col1);

PRINT @@ERROR;

GO

PRINT 'Error handling with TRY CATCH';

BEGIN TRY

  CREATE UNIQUE INDEX ix_Foo
  ON Foo(col1);

END TRY
BEGIN CATCH

  PRINT ERROR_MESSAGE();

END CATCH

GO

DROP TABLE Foo;

Hmmm...interesting.

When I run the code, the line that says PRINT @@ERROR; is not executed.  It
looks like this error aborts the batch.

Show quoteHide quote

Correct, that was the point. With TRY CATCH you can handle it.

On Dec 1, 9:01 am, NeoRev <Neo***@discussions.microsoft.com> wrote:
Here:

http://www.sommarskog.se/error-handling-I.html
http://www.sommarskog.se/error-handling-II.html

To put it another way, how is this code (spErrorHandlingTest1) any
functionally different than this code (spErrorHandlingTest2) other than the
error number being returned by the RETURN statement but we can get that from
the Exception object in our client code anyway:

CREATE PROCEDURE dbo.spErrorHandlingTest1
AS
BEGIN

DECLARE @Error INT
DECLARE @myInt INT

SELECT @myInt = 1/0

SELECT @Error = @@ERROR

IF @Error <> 0 GOTO EH

RETURN 0
EH:
    RAISERROR(@Error, 16, 1)
    RETURN @Error

END


CREATE PROCEDURE dbo.spErrorHandlingTest2
AS
BEGIN

DECLARE @myInt INT

SELECT @myInt = 1/0

END

Wait a second!  Does this error handling strategy even work?  When I execute
the following stored procedure, I get an error on the RAISERROR statement. 

CREATE PROCEDURE dbo.spErrorHandlingTest1
AS
BEGIN

DECLARE @Error INT
DECLARE @myInt INT

SELECT @myInt = 1/0

SELECT @Error = @@ERROR

IF @Error <> 0 GOTO EH

RETURN 0
EH:
    RAISERROR(@Error, 16, 1)
    RETURN @Error

END

Msg 8134, Level 16, State 1, Procedure spErrorHandlingTest1, Line 8
Divide by zero error encountered.
Msg 2732, Level 16, State 1, Procedure spErrorHandlingTest1, Line 16
Error number 8134 is invalid. The number must be from 13000 through
2147483647 and it cannot be 50000.

Show quoteHide quote

User-defined error message number should be greater than 50000 (you
cannot raise the original system error number):
http://msdn.microsoft.com/en-us/library/ms178592.aspx
http://msdn.microsoft.com/en-us/library/ms177497.aspx

Thanks for all your help.

Show quoteHide quote

A couple other things that I didn't like with the client's standard was that
they were hardcoding the severity as 16 and the state as 1.  What happens if
you have an error with a severity of 11 or a state of 0?  It ends up passing
incorrect information to the client.  They should be using ERROR_SEVERITY()
and ERROR_STATE().

So, putting this all together, the following should be a good template for
error handling (although I still think that having an error hander in every
sproc is overkill).

BEGIN TRY
    --Do something that might cause an error such as SELECT 1/0
END TRY
BEGIN CATCH
    DECLARE @errorMessage NVARCHAR(4000)
    DECLARE @errorSeverity INTEGER
    DECLARE @errorState INTEGER

    SELECT @errorMessage = ERROR_MESSAGE(),
           @errorSeverity = ERROR_SEVERITY(),
           @errorState = ERROR_STATE()

    RAISERROR (@errorMessage, @errorSeverity, @errorState)
END CATCH

Actually, I'm not too crazy for this code either because it loses the error
number.

Show quoteHide quote

There doesn't seem to be a way to set the error number.  I guess you could do
something like this but the error message formatting should probably go into
its own fuction.

ALTER PROCEDURE [dbo].[spErrorHandlingTest2]
AS
BEGIN

BEGIN TRY
    SELECT 1/0
END TRY
BEGIN CATCH
    DECLARE @errorNumber INTEGER
    DECLARE @errorMessage NVARCHAR(4000)
    DECLARE @errorSeverity INTEGER
    DECLARE @errorState INTEGER
    DECLARE @errorProcedure NVARCHAR(128)
    DECLARE @errorLine INTEGER

    SELECT @errorNumber = ERROR_NUMBER(),
           @errorMessage = ERROR_MESSAGE(),
           @errorSeverity = ERROR_SEVERITY(),
           @errorState = ERROR_STATE(),
           @errorProcedure = ERROR_PROCEDURE(),
           @errorLine = ERROR_LINE()

    RAISERROR('Procedure ''%s'' failed on line number ''%u'' with message
''%s'' - (error number: ''%u'', severity: ''%u'', state: ''%u'').',
@errorSeverity, @errorState, @errorProcedure, @errorLine, @errorMessage,
@errorNumber, @errorSeverity, @errorState)

END CATCH

Show quoteHide quote

The TRY CATCH topic in SQL Server Books Online shows examples of
wrapping the error handling in a separate stored procedure:
http://msdn.microsoft.com/en-us/library/ms179296.aspx

Cool.  That's actually the direction I was headed.  My boiler plate code was
getting too long.

Show quoteHide quote

In case anyone else is reading this thread (and for posterity's sake),
Microsoft's usp_RethrowError stored procedure has a couple bugs in it. 

1) It is not always possible to rethrow an error with the same severity as
was caught. Only a member of the sysadmin role can raise an error with a
severity greater than or equal to 19.

2) RAISERROR only generates errors with state from 1 through 127. Because
the Database Engine may raise errors with state 0, it should check the error
state returned by ERROR_STATE before passing it as a value to the state
parameter of RAISERROR.

Show quoteHide quote

OK, here's my 'final' code:

-- Verify that stored procedure does not exist.
IF OBJECT_ID (N'usp_RethrowError',N'P') IS NOT NULL
    DROP PROCEDURE usp_RethrowError;
GO
/******************************************************************************
* Original Auther:
* Creation Date: 12/02/2008>
* Purpose: To rethrow an error.  The original error information is used to
* construct the msg_str for RAISERROR.
* Last Revised By: <Last Revised By Name>
* Last Revised Date: <mm/dd/yyyy> *******************************************************************************/

CREATE PROCEDURE uspRethrowError AS

    -- Return if there is no error information to retrieve.
    IF ERROR_NUMBER() IS NULL
        RETURN;

    DECLARE
        @errorMessage          NVARCHAR(4000),
        @errorNumber           INT,
        @errorSeverity         INT,
        @errorState            INT,
        @errorLine             INT,
        @errorProcedure        NVARCHAR(200),
        @adjustedErrorState    INTEGER,
        @adjustedErrorSeverity INTEGER;

    -- Assign variables to error-handling functions that capture information
for
    -- RAISERROR.
    SELECT
        @errorNumber = ERROR_NUMBER(),
        @errorSeverity = ERROR_SEVERITY(),
        @errorState = ERROR_STATE(),
        @errorLine = ERROR_LINE(),
        @errorProcedure = ISNULL(ERROR_PROCEDURE(), '-');

    -- Build the message string that will contain original error information.
    SELECT @errorMessage =
        N'Error %d, Level %d, State %d, Procedure %s, Line %d, '
        + 'Message: ' + ERROR_MESSAGE();

    -- It is not always possible to rethrow an error with the same severity
as
    -- was caught. Only a member of the sysadmin role can raise an error
with a
    -- severity greater than or equal to 19.
    IF @errorSeverity < 19
        BEGIN
            SET @adjustedErrorSeverity = @errorSeverity
        END
    ELSE
        BEGIN
            SET @adjustedErrorSeverity = 18
        END

    -- RAISERROR only generates errors with state from 1 through 127. Because
    -- the Database Engine may raise errors with state 0, we need to check
the
    -- error state returned by ERROR_STATE before passing it as a value to
the
    -- state parameter of RAISERROR.
    IF @errorState <> 0
        BEGIN
            SET @adjustedErrorState = @errorState
        END
    ELSE
        BEGIN
            SET @adjustedErrorState = 1
        END

    -- Raise an error: msg_str parameter of RAISERROR will contain the
original
    -- error information.
    RAISERROR
        (
        @errorMessage,
        @adjustedErrorSeverity,
        @adjustedErrorState,              
        @errorNumber,           -- parameter: original error number.
        @errorSeverity,         -- parameter: original error severity.
        @errorState,            -- parameter: original error state.
        @errorProcedure,        -- parameter: original error procedure name.
        @errorLine              -- parameter: original error line number.
        );
GO

Show quoteHide quote

Actually, there's a bug in the previous code.  According to Microsoft's
documentation, "RAISERROR only generates errors with state from 1 through
127. Because the Database Engine may raise errors with state 0, we recommend
that you check the error state returned by ERROR_STATE before passing it as a
value to the state parameter of RAISERROR."

Here's my revised boiler plate error handler.

ALTER PROCEDURE [dbo].[spErrorHandlingTest2]
AS
BEGIN

BEGIN TRY
    --Do something that might cause an error such as SELECT 1/0
END TRY
BEGIN CATCH
    DECLARE @errorNumber INTEGER
    DECLARE @errorMessage NVARCHAR(4000)
    DECLARE @errorSeverity INTEGER
    DECLARE @errorState INTEGER
    DECLARE @errorProcedure NVARCHAR(128)
    DECLARE @errorLine INTEGER
    DECLARE @adjustedErrorState INTEGER

    SELECT @errorNumber = ERROR_NUMBER(),
           @errorMessage = ERROR_MESSAGE(),
           @errorSeverity = ERROR_SEVERITY(),
           @errorState = ERROR_STATE(),
           @errorProcedure = ERROR_PROCEDURE(),
           @errorLine = ERROR_LINE()

    /*
    RAISERROR only generates errors with state from 1 through 127. Because
the Database Engine
    may raise errors with state 0, we need to check the error state returned
by ERROR_STATE before
    passing it as a value to the state parameter of RAISERROR.
    */
    IF @errorState <> 0
    BEGIN
        SET @adjustedErrorState = @errorState
    END
    ELSE
    BEGIN
        SET @adjustedErrorState = 1
    END

    --RAISERROR (@errorMessage, @errorSeverity, @errorState,
@errorProcedure, @errorLine)
    RAISERROR('Procedure ''%s'' failed on line number ''%u'' with message
''%s'' - (error number: ''%u'', severity: ''%u'', state: ''%u'').',
@errorSeverity, @errorState, @errorProcedure, @errorLine, @errorMessage,
@errorNumber, @errorSeverity, @errorState)

END CATCH

END

Show quoteHide quote

Transforming Rows to Columns
DELETE and WHERE Clause SQL Help
Covering indexes question
Problem accessing SQL Server 2000 from SQL Server 2005 64-bit
Transaction (Process ID 411) was deadlocked on thread
bcp command specifying output file in path with a space
help on bcp delimiter
64bit to 32bit??
Reporting Services - Service Error
I need to copy data from table to another