SQL Server Forum / DB Engine / SQL Server / December 2008
Best practices for error handling in SQL Server procs
|
|
Thread rating:  |
NeoRev - 20 Nov 2008 18:15 GMT 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.
Plamen Ratchev - 20 Nov 2008 18:56 GMT > Does this sound like a good idea? Having an error handler in every stored > procedure seems excessive. Yes, it is a good idea to have error handling in every stored procedure.
> Also, I don't think the RETURN @Error statement ever executes because the > RAISERROR causes the sproc to immediately exit. 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.
 Signature Plamen Ratchev http://www.SQLStudio.com
NeoRev - 20 Nov 2008 19:59 GMT 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?
> > Does this sound like a good idea? Having an error handler in every stored > > procedure seems excessive. [quoted text clipped - 24 lines] > If this is SQL Server 2005/2008, then error handling should be done by > using TRY..CATCH rather than the old @@ERROR method. Plamen Ratchev - 20 Nov 2008 20:08 GMT 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.
 Signature Plamen Ratchev http://www.SQLStudio.com
NeoRev - 20 Nov 2008 20:51 GMT Actually, that's a bad practice. I'll try to dig up some articles for you to read.
> 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. NeoRev - 20 Nov 2008 22:26 GMT 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.
> Actually, that's a bad practice. I'll try to dig up some articles for you to > read. > > > 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. Plamen Ratchev - 20 Nov 2008 22:48 GMT 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.
 Signature Plamen Ratchev http://www.SQLStudio.com
NeoRev - 20 Nov 2008 23:21 GMT 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.
> 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 [quoted text clipped - 45 lines] > that have achieved the highest quality of coding practices and they all > indicate exception handling at all levels is a must. Alex Kuznetsov - 23 Nov 2008 01:12 GMT > Thanks for the articles. I will try to read them tonight or over the weekend. > [quoted text clipped - 33 lines] > > Plamen Ratchev > >http://www.SQLStudio.com Also you need to unit test your error handling:
http://www.simple-talk.com/sql/t-sql-programming/close-these-loopholes---reprodu ce-database-errors/
NeoRev - 22 Nov 2008 21:10 GMT 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.
> > Does this sound like a good idea? Having an error handler in every stored > > procedure seems excessive. [quoted text clipped - 24 lines] > If this is SQL Server 2005/2008, then error handling should be done by > using TRY..CATCH rather than the old @@ERROR method. Plamen Ratchev - 23 Nov 2008 04:34 GMT 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.
 Signature Plamen Ratchev http://www.SQLStudio.com
Alex Kuznetsov - 23 Nov 2008 16:58 GMT Also we need to realize that TRY... CATCH does not catch all the errors - some errors (such as timeout) will not be caught.
NeoRev - 01 Dec 2008 15:01 GMT Are there any other errors TRY...CATCH doesn't catch? Is this documented anywhere?
> Also we need to realize that TRY... CATCH does not catch all the > errors - some errors (such as timeout) will not be caught. Plamen Ratchev - 01 Dec 2008 15:36 GMT 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
 Signature Plamen Ratchev http://www.SQLStudio.com
NeoRev - 01 Dec 2008 17:00 GMT 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?
> TRY CATCH will catch all execution errors with severity greater than 10 > that do not terminate the database connection. Errors that terminate the [quoted text clipped - 11 lines] > More on connection termination errors: > http://www.sommarskog.se/error-handling-I.html#connection-termination Plamen Ratchev - 01 Dec 2008 17:57 GMT 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
 Signature Plamen Ratchev http://www.SQLStudio.com
NeoRev - 01 Dec 2008 18:17 GMT I'm sorry. I meant, are there any errors that @Error will find that TRY...CATCH won't (besides informational messages)?
> 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 [quoted text clipped - 5 lines] > Here is one example: > http://blogs.msdn.com/sqlprogrammability/archive/2006/03/30/565141.aspx Plamen Ratchev - 01 Dec 2008 18:41 GMT 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;
 Signature Plamen Ratchev http://www.SQLStudio.com
NeoRev - 01 Dec 2008 19:04 GMT Hmmm...interesting.
When I run the code, the line that says PRINT @@ERROR; is not executed. It looks like this error aborts the batch.
> 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: [quoted text clipped - 34 lines] > > DROP TABLE Foo; Plamen Ratchev - 01 Dec 2008 19:12 GMT Correct, that was the point. With TRY CATCH you can handle it.
 Signature Plamen Ratchev http://www.SQLStudio.com
Alex Kuznetsov - 01 Dec 2008 16:43 GMT > Are there any other errors TRY...CATCH doesn't catch? Is this documented > anywhere? > > > Also we need to realize that TRY... CATCH does not catch all the > > errors - some errors (such as timeout) will not be caught. Here:
http://www.sommarskog.se/error-handling-I.html http://www.sommarskog.se/error-handling-II.html
NeoRev - 20 Nov 2008 21:22 GMT 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
NeoRev - 01 Dec 2008 19:11 GMT 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.
> 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, [quoted text clipped - 28 lines] > Also, I don't think the RETURN @Error statement ever executes because the > RAISERROR causes the sproc to immediately exit. Plamen Ratchev - 01 Dec 2008 19:24 GMT 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
 Signature Plamen Ratchev http://www.SQLStudio.com
NeoRev - 01 Dec 2008 19:56 GMT Thanks for all your help.
> 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 NeoRev - 01 Dec 2008 20:50 GMT 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
NeoRev - 02 Dec 2008 16:33 GMT Actually, I'm not too crazy for this code either because it loses the error number.
> 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 [quoted text clipped - 20 lines] > RAISERROR (@errorMessage, @errorSeverity, @errorState) > END CATCH NeoRev - 02 Dec 2008 16:54 GMT 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
> Actually, I'm not too crazy for this code either because it loses the error > number. [quoted text clipped - 23 lines] > > RAISERROR (@errorMessage, @errorSeverity, @errorState) > > END CATCH Plamen Ratchev - 02 Dec 2008 17:13 GMT 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
 Signature Plamen Ratchev http://www.SQLStudio.com
NeoRev - 02 Dec 2008 17:56 GMT Cool. That's actually the direction I was headed. My boiler plate code was getting too long.
> 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 NeoRev - 02 Dec 2008 18:13 GMT 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.
> Cool. That's actually the direction I was headed. My boiler plate code was > getting too long. > > > 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 NeoRev - 02 Dec 2008 20:14 GMT 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
> 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. [quoted text clipped - 14 lines] > > > wrapping the error handling in a separate stored procedure: > > > http://msdn.microsoft.com/en-us/library/ms179296.aspx NeoRev - 02 Dec 2008 17:23 GMT 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
> 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 [quoted text clipped - 56 lines] > > > RAISERROR (@errorMessage, @errorSeverity, @errorState) > > > END CATCH
|
|
|