"The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION" I keep getting this error when I try to execute my stored procedure,Help me

ALTER PROCEDURE Add_Edit_Courses_new
    @CourseCode          VARCHAR,
    ... other params ...
AS
    BEGIN TRY
        DECLARE @ErrorCode INT =0, @ErrorMessage VARCHAR(25) = 'Action failed'

        IF @TaskType > 2
            BEGIN
                RAISERROR('Wrong action key',16,1)
            END
        ELSE 
            BEGIN TRANSACTION 
            BEGIN
                DECLARE @message VARCHAR(MAX)
                IF @TaskType = 1
                BEGIN
                    INSERT INTO Courses(...) VALUES(@CourseCode,...)
                    SET @message = 'Added Successfully'
                END
                ELSE IF @TaskType = 2
                BEGIN
                    UPDATE Courses SET CourseCode=@CourseCode,...;
                    SET @message = 'Modified Successfully'
                END                                                 
            END
            COMMIT TRANSACTION 
    END TRY
    BEGIN CATCH
      ROLLBACK TRANSACTION 
      SELECT ERROR_NUMBER() AS ErrorNumber, ...
    END CATCH

I wrote this stored procedure to insert and update and I used (1 & 2) to differentiate the task while using a try and catch, but every time I try to execute this stored procedure I keep getting that error, please can you help me with where I am wrong, I am just learning this principle for the first time.

Upvotes: 0

Views: 607

Answers (2)

SteveC
SteveC

Reputation: 6015

Imo there are some major issues with this code. First, if the intention is for the transaction to be atomic then SET XACT_ABORT ON should be specified. Per the Docs RAISERROR does not honor SET XACT_ABORT ON so it could be converted to use THROW. Second, in cases where the ELSE block in the code is hit then COMMIT will always be hit (regardless of what happens to the commitable state of the transaction).

Also, the code performs the ROLLBACK before the code:

 SELECT ERROR_NUMBER() AS ErrorNumber, ...

It's the ROLLBACK which clears the error messages and returns the state to normal. To catch the error metadata SELECT it before the ROLLBACK happens.

Upvotes: 1

Aaron Bertrand
Aaron Bertrand

Reputation: 280520

Why is BEGIN TRANSACTION before BEGIN? I feel like BEGIN TRANSACTION/COMMIT TRANSACTION should be inside the ELSE conditional. With some noise removed:

    IF @TaskType > 2
    BEGIN
        RAISERROR('Wrong action key',16,1);
    END
    ELSE 
    BEGIN -- moved this here

        BEGIN TRANSACTION;
        -- BEGIN -- removed this

          DECLARE @message varchar(max);

          IF @TaskType = 1
          BEGIN
            INSERT INTO Courses(...
            SET @message = 'Added Successfully';
          END

          IF @TaskType = 2 -- don't really need ELSE there
          BEGIN
            UPDATE Courses SET ...
            SET @message = 'Modified Successfully';
          END
                                                
        -- END -- removed this 
        COMMIT TRANSACTION;
        SELECT @message;
    END -- moved this here

In your catch you just blindly say:

ROLLBACK TRANSACTION;

This should be updated to:

IF @@TRANCOUNT > 0
BEGIN
  ROLLBACK TRANSACTION;
END

Note that if you have a conditional where multiple statements are not wrapped correctly in BEGIN / END, they won't execute like you think. Consider:

IF 1 = 0
  PRINT 'foo';
  PRINT 'bar';

You get bar output every time, regardless of the result of the conditional.

You had something similar:

IF 1 = 1
  -- do stuff
ELSE
  BEGIN TRANSACTION;
  BEGIN 
    -- do stuff
  END
  COMMIT TRANSACTION;

In that case, -- do stuff and the commit happened every time, even if the begin transaction did not, because that BEGIN/END wrapper (and anything that followed it) was not associated with the ELSE.

Perhaps a little dry and wordy if you're new to the topic, but Erland Sommarskog has a very comprehensive series on error handling here, might be worth a bookmark:

Upvotes: 2

Related Questions