CODEWITHSUNDEEP
sql-serverstored-procedureserror-handlingraiseerror
Leon
Leon

Reputation: 63

Raiserror not returning expected value

I am using SQL Server and trying to return an error from a stored procedure to a caller. However it does not seem to work. When debugging the returned value is null.

Error handling:

begin try
    if not exists (select * from dbo.Employees where IsCEO = 1)
    begin
        insert into dbo.Employees (FirstName, LastName, Salary, IsCEO, IsManager, ManagerId, EmployeeRank)
        values (@FirstName, @LastName, @Salary, @IsCEO, @IsManager, @ManagerId, @EmployeeRank)              
    end
end try
begin catch
    return raiserror ('CEO already exists', 15, 1)
end catch

Upvotes: 0

Views: 489

Answers (2)

Aarlaneth
Aarlaneth

Reputation: 530

Your issue here is that you never encounter error in this code.

Logic of your code says if CEO doesn't exist, insert employee in the table. If it exists, do nothing. And then you check for errors in this and say CEO already exists which makes no sense.

Seem to me you wanted to do follwing.

begin try
if not exists (select * from dbo.Employees where IsCEO = 1)
begin
    insert into dbo.Employees (FirstName, LastName, Salary, IsCEO, IsManager, ManagerId, 
    EmployeeRank)
    values (@FirstName, @LastName, @Salary, @IsCEO, @IsManager, @ManagerId, 
    @EmployeeRank)              
end
ELSE
BEGIN
raiserror ('CEO already exists', 15, 1)
END
end try
begin catch
    --some code that handles possible errors in code above
end catch

Upvotes: 1

Thom A
Thom A

Reputation: 95544

To repeat what I state in the comments, firstly your RETURN in the CATCH doesn't make sense. You don't RETURN an error, RETURN provides back an int value, and (historically) is used to denote the success of the procedure (0 meaning success, anything else meaning failure). Their use, however, is somewhat more historical, especially now with things like THROW and OUTPUT parameters.

Speaking of THROW, you really should be using that too. As the documentation on RAISERROR states:

Note

The RAISERROR statement does not honor SET XACT_ABORT. New applications should use THROW instead of RAISERROR.

Finally, your error and your IF conflict. Your IF checks to see if a CEO does not exist, however, the error you raise states that they do.

I suspect therefore, you likely want something like like this, which doesn't need the TRY...CATCH, and just an IF...ELSE:

BEGIN
    IF NOT EXISTS (SELECT * FROM dbo.Employees WHERE IsCEO = 1)
        INSERT INTO dbo.Employees (FirstName,
                                   LastName,
                                   Salary,
                                   IsCEO,
                                   IsManager,
                                   ManagerId,
                                   EmployeeRank)
        VALUES (@FirstName, @LastName, @Salary, @IsCEO, @IsManager, @ManagerId, @EmployeeRank);
    ELSE
        THROW 50001, N'CEO does not exist', 16; --Use an error number (and state) appropriate for your environment
    --This will only be reached if the ELSE was not entered
    RETURN 0;
END;

Upvotes: 3

Related Questions