It’s not a good idea.
I recently encountered a situation involving a stored procedure that was failing with the following error:
The EXECUTE permission was denied on the object 'xp_logevent', database 'mssqlsystemresource', schema 'sys'
It turns out, a developer though he or she was being pretty clever. They had very well formatted and structured code in their stored procedure. All of their DML statements were wrapped in TRY/CATCH blocks with error handling code. The problem was they clearly never tested their error handling, or if they did, it was probably on a local dev instance and running under the context of a sysadmin. The result was that an error in the TRY block resulted in an error in the CATCH block, and the originating error message was lost in the process.
The problem with their approach was twofold. First, xp_logevent requires membership in the db_owner role of the master database or membership in sysadmin. Neither of these permissions are something you should be granting to your application service accounts or users. Secondly, xp_logevent does not give any indication to the client that an error has occurred. It simply writes an event to the Windows Event Log and SQL Server error log, then continues execution.
How could this have been better handled? Well, to quote MSDN:
When you send messages from Transact-SQL procedures, triggers, batches, and so on, use the RAISERROR statement instead of xp_logevent. xp_logevent does not call a message handler of a client or set @@ERROR. To write messages to the Windows Event Viewer and to the SQL Server error log file within an instance of SQL Server, execute the RAISERROR statement.
This is a slightly dated recommendation, as it’s now generally suggested to use THROW instead of RAISERROR (for SQL Server 2012+), but the concept is essentially the same. Both of these solutions will return an error message to the client without requiring any special permissions or role membership. However, RAISERROR will not log to the Windows Event Log or SQL Server error log as described above unless the caller is a member of sysadmin and the WITH LOG option of RAISERROR is specified. THROW is not capable of logging to either of those locations.
My personal belief is that non-fatal errors inside of a stored procedure are an application error and should be logged as such; return the error message to the client and log it in the application’s error log. Keep the SQL Server error log and Windows Event Log reserved for errors that pertain to failures in the SQL Server engine itself. Logging application errors on the database server is of marginal use to application administrators anyway, since there’s a good chance they won’t have permissions to view the event log or SQL Server error log.
While there are certainly edge cases where it would be desired in an application to silently log errors locally on the database server, more often than not you’re going to want your application to know that an error occurred. Had the call to xp_logevent succeeded in the example above, there would have been no indication of a problem to the client. This can result in the types of phantom errors that are difficult to troubleshoot. Perhaps a user clicks a Save button, then goes back to the record later and finds that their changes were not saved. An administrator might get the help desk ticket, check the application error log, see nothing abnormal, and tell the user they must have made a mistake. Only after the situation repeats itself might a deeper dive into the connected systems occur, eventually uncovering the error after much unnecessary effort from multiple team members. All of this could be easily avoided by returning an error to the client and logging the details where they belong – with the application.