Is having an error handler in every single stored procedure excessive?
-
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. Honestly, I'm of the opinion that 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. Do you agree or disagree? Why or why not?
Seems pretty much like the standards everywhere I've contracted in the last few years. If you get an error you can enhance the message to help track down the problem, so no, I don't think its a waste of time. My biggest gripe is that you can't (pre 2005) trap ALL errors so you can handle them. I've spent hours trying to track down such elusive messages as 'string or binary data would be truncated' :((
Bob Ashfield Consultants Ltd
-
Seems pretty much like the standards everywhere I've contracted in the last few years. If you get an error you can enhance the message to help track down the problem, so no, I don't think its a waste of time. My biggest gripe is that you can't (pre 2005) trap ALL errors so you can handle them. I've spent hours trying to track down such elusive messages as 'string or binary data would be truncated' :((
Bob Ashfield Consultants Ltd
> If you get an error you can enhance the message to help track down the problem What sort of information would you include to enhance the error message? For example, what would you change about this stored procedure? --air code CREATE PROCEDURE GetCustomerCount AS SELECT COUNT(*) FROM CUSTOMERS RETURN 0 EH: RAISERROR(@Error, 16, 1) RETURN @Error
-
> If you get an error you can enhance the message to help track down the problem What sort of information would you include to enhance the error message? For example, what would you change about this stored procedure? --air code CREATE PROCEDURE GetCustomerCount AS SELECT COUNT(*) FROM CUSTOMERS RETURN 0 EH: RAISERROR(@Error, 16, 1) RETURN @Error
Obviously, in this example there is nothing, but if you have anything more complex, for example a proc which has several update statements, then it is useful to know where the error originated. Another example would be a series of nested procs - by including the proc name in the error you know where it originated. As with any set of standards there are cases for and against - for example, stylecop in c# insists that the using statements come after the namespace declaration, but if you create a new class they are generated before. As a long time contractor I have worked at many companies, they all have slightly different standards, some good, some not so good, but at the end of the day they are called standards for a reason - consistency. The overhead they cause is minimal and they make it easy for someone to pick up the code and see what is happening. If you think the standard is not good you need to raise it with the powers that be, but in your original post you state "One of our clients..." and at the end of the day they call the tune.
Bob Ashfield Consultants Ltd
-
Obviously, in this example there is nothing, but if you have anything more complex, for example a proc which has several update statements, then it is useful to know where the error originated. Another example would be a series of nested procs - by including the proc name in the error you know where it originated. As with any set of standards there are cases for and against - for example, stylecop in c# insists that the using statements come after the namespace declaration, but if you create a new class they are generated before. As a long time contractor I have worked at many companies, they all have slightly different standards, some good, some not so good, but at the end of the day they are called standards for a reason - consistency. The overhead they cause is minimal and they make it easy for someone to pick up the code and see what is happening. If you think the standard is not good you need to raise it with the powers that be, but in your original post you state "One of our clients..." and at the end of the day they call the tune.
Bob Ashfield Consultants Ltd
A lot of stored procedures are simple, one-statement CRUD operations that probably don't require error handling, which is why I think that mandating that every single sproc have an error handler is excessive. These coding standards are brand new. If we get the contract, this will be the very first project to use them. So, consistency with existing applications is not an issue.
-
A lot of stored procedures are simple, one-statement CRUD operations that probably don't require error handling, which is why I think that mandating that every single sproc have an error handler is excessive. These coding standards are brand new. If we get the contract, this will be the very first project to use them. So, consistency with existing applications is not an issue.
-
Its up to you and your client. I still maintain they are called standards for a reason.
Bob Ashfield Consultants Ltd
-
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. Honestly, I'm of the opinion that 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. Do you agree or disagree? Why or why not?
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 ENDMsg 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. -
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 ENDMsg 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.I think it works as expected. Since you are passing forward the original error (8134) as message id in a custom RAISERROR, you'll have trouble. From Books Online: msg_id Is a user-defined error message number stored in the sys.messages catalog view using sp_addmessage. Error numbers for user-defined error messages should be greater than 50000. When msg_id is not specified, RAISERROR raises an error message with an error number of 50000.
The need to optimize rises from a bad design.My articles[^]
-
I think it works as expected. Since you are passing forward the original error (8134) as message id in a custom RAISERROR, you'll have trouble. From Books Online: msg_id Is a user-defined error message number stored in the sys.messages catalog view using sp_addmessage. Error numbers for user-defined error messages should be greater than 50000. When msg_id is not specified, RAISERROR raises an error message with an error number of 50000.
The need to optimize rises from a bad design.My articles[^]
-
Had to re-read my post and yes that's exactly what I meant... and yes the answer was confusing since I didn't point out what I was referring to. Sorry about that :) But the problem is that you're using reserved error numbers in custom RAISERROR call. If you change the first argument to for example 50001 you'll see it behaves differently. Mika
The need to optimize rises from a bad design.My articles[^]
-
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. Honestly, I'm of the opinion that 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. Do you agree or disagree? Why or why not?
Sorry to revisit an old thread, but I happened to stumble across an article that relates to this topic. According to Microsoft's Design Guidelines for Exceptions: "Do not overuse catch. Exceptions should often be allowed to propagate up the call stack." http://msdn.microsoft.com/en-us/library/ms229005.aspx Of course, this guideline is in reference to .NET code and not SQL, but it's still good advice.