T-SQL and Error Handling

[wbodger]

OK, I have never been good about putting error handling in my code. I'm not proud of that, I know it is not best practice, so my question is: Will this do what I am thinking it will? I am thinking this code will insert from the raw table into the history table and then, as long as there are no errors it will delete the rows from the raw table.

ALTER PROCEDURE [ProcessCoreWithNoDetails]
	@ThisISN int
AS
SET XACT_ABORT, NOCOUNT ON
BEGIN TRY
	BEGIN TRANSACTION
	BEGIN TRY
		INSERT INTO dbo.History (ThisISN, EntityISN, AccountISN, TransactionCodeId, [EntityType_CategoryTypeId], Amount, TransactionDate, Importdate, PacketId, StatusId, PacketIdGL)
		SELECT core.ThisISN, ea.EntityISN, ea.AccountISN, TransactionCodeId, EntityType_CategoryTypeId, ISNULL(CAST(Credit AS MONEY), CAST(Debit AS MONEY) * -1), CONVERT(DATETIME, PacketDate + ' ' + PacketHour + ':00:00', 120), GETDATE(), PacketId, 4, PacketIdGL
		FROM CoreRaw core JOIN TransactionCode code
			ON core.TxCode = code.TxCode JOIN dbo.EntityAccounts ea
			ON (ea.AccountNumber = concat(core.BaseAccountNumber,core.AccountTypeNumber) OR ea.AccountNumber = core.BaseAccountNumber)
		WHERE code.HasChildData = 0
		AND core.ThisISN = @ThisISN

		DELETE core FROM CoreRaw core JOIN TransactionCode code
			ON core.TxCode = code.TxCode JOIN dbo.EntityAccounts ea
			ON ea.AccountNumber = concat(core.BaseAccountNumber,core.AccountTypeNumber) OR ea.AccountNumber = core.BaseAccountNumber
		WHERE code.HasChildData = 0
		AND core.ThisISN = @ThisISN
	END TRY
	BEGIN CATCH
		IF @@trancount > 0 ROLLBACK TRANSACTION
		;THROW
	END CATCH
COMMIT TRANSACTION
END TRY
BEGIN CATCH
	IF @@trancount > 0 ROLLBACK TRANSACTION
	;THROW
END CATCH

Thanks!
wb

 

[SaltyTheFrog]

What do your test results show? By the way, I've never heard that error handling was "is not best practice". I think it is a very good idea.

 

[wbodger]

I meant, not putting error handling in is not best practice, i.e. that fact that I have typically not done much error handling is not best practice.

Thanks,
wb

 

[Olaf Doschke]

It also often depends on the client side. If an application executes ad hoc scripts it often is bad, if SQL Server already handles and swallows errors. You can easily detect a select query not working, but no error coming back from an insert would mean the client side application will interpret no error coming back as successful insert. So the only place for error handling is with errors you know you can mend. That's a reason you often don't see TRY..CATCH implemented in SQL. It surely has more place in stored procs and user defined functions, any permanent server side code.

Bye, Olaf.

 

[wbodger]

Yes, I am looking only at server side code

 

[Olaf Doschke]

In your example above I wonder about the necessety of the double nested TRY..CATCH. I see you want to get past the COMMIT TRANSACTION, when you get into the CATCH. That's much simpler done by putting both the BEGIN TRANSACTION and the COMMIT TRANSACTION into the first TRY block. If the TRY block succeeds fully, your manual transaction is closed again, if it fails - at any stage - even because your transaction nesting got too deep, the CATCH block rolllbacks a transaction, if you started one. That's complete.

Otherwise it's fine, including the THROW back of the same exception to the caller.

Another thing I dislike is a matter of taste, though - the semicolon positioning.

The statement before the THROW statement must be followed by the semicolon ( statement terminator.

To follow that rule in code you execute after any other developers code your only chance may be to put a semicolon at the start of your code, but even then you can stop the nonsense of people thinking certain commands have to start with a semicolon by putting it standalone in a previous line. In your own code, you can simply adhere to the rule and terminate the previous statement with the semicolon.

Overall:

ALTER PROCEDURE [ProcessCoreWithNoDetails]
	@ThisISN int
AS
SET XACT_ABORT, NOCOUNT ON
BEGIN TRY
	BEGIN TRANSACTION
	INSERT INTO dbo.History ...

	DELETE core FROM CoreRaw core ...
	COMMIT TRANSACTION
END TRY
BEGIN CATCH
	IF @@trancount > 0 ROLLBACK TRANSACTION;
	THROW
END CATCH

Edit: Being paranoid, you could save the initial @@trancount to see whether it got incremented by the BEGIN TRANSACTION, but I don't know of a transaction nesting limit, there is a stored procedure call @@nestlevel limitation of 32, but no transaction nest limitation, so you could even spare the @@trancount>0 check, if the catch block is activated by an exception, then you can be sure it's not caused by the BEGIN TRANSACTION.

Bye, Olaf.

 

[wbodger]

Thanks!