Best practice to corrupt data
A common practice for handling errors in PL/SQL procedures is to catch all errors in the top-most database layer and convert them into error codes and human readable messages for client applications. This technique is a relict from the past and, in fact, a very bad practice from today's perspective, since it can lead to data corruption.
Procedures build by this rule are easy to spot. They typically have one or two additional OUT parameters on the end – error code and message. Here is a typical example:
build_references ( start_ref IN reference_tbl.idref%TYPE, start_date IN DATE := SYSDATE, error_code OUT INTEGER, error_msg OUT VARCHAR2);
This type of error handling really was a best practice several years ago, and is even published in Steven Feuersteins book “PL/SQL Best Practices” (EXC-03: Catch all exceptions and convert to meaningful return codes before returning to non-PL/SQL host programs). O'Reilly published that book in 2001, at the time when popular client platforms did not know what do to with database exceptions or how to handle them, so they were given an error code and message through output parameters. Feuerstein mentions three client platforms as typical examples: Powerbuilder, Visual basic and Java (a bit strange, since Java has proper exception handling) . The situation changed significantly since then, but the practice lives on, and is still followed with a religious zeal in some companies.
Other than dealing with platforms that do not convert Oracle exceptions into host exceptions, I've come across several explanations for this “best practice”:
- dealing with all possible errors in the database, and not having to bother with errors in web/client application code
- giving clients a single, clear and human readable message instead of a strange stack trace
The original premise, client platforms which don't know what to do with database exceptions, no longer holds. Most client platforms now support exception handling, and even Visual basic will transform exceptions from ODBC drivers into it's embedded Err object. Database exceptions will just be wrapped into a SQLException (Java), DbException (.Net) or whichever class is appropriate. But, on the end, you will get a proper host exception, with the database stack trace included. In fact, Java will make you handle a checked SQLException, whether you like it or not.
The other two explanations, as far as I am concerned, never were valid. First of all, you will never be able to take care of all the errors in the database. Never. Some edge cases will still give you problems - network glitch will cause web to disconnect from the database, connection pool will decide to recycle and Oracle will not be able to start a dedicated listener process, or the database will run out of storage. These problems are not frequent, and represent a very irregular situation - an Exception. You might explain to yourself that all the problems were handled in the database, but your client could not care less about that. His customer placed a bet on a winning horse, but there is no record of the bet in the database.
The “strange stack trace” concept was never clear to me. I like to have as much troubleshooting information as I can. Most login operations will fail because the password was incorrect, but if someone added a trigger to the session table that cannot execute because it's mutating, having an 'Incorrect password' as the only information in application logs does not help me very much. I think that the third explanation is just a sign of laziness – front end programmers not wanting to get the message out of the exception. On the end, exceptions in most platforms carry both the error code and string message – instead of dumping the stack trace, front end can show only the message (and log the trace in any case).
So there are no real benefits of the approach. However, the story does not end here – serious problems can be caused by the assumed “best practice”.
Feuerstein suggests maintaining an additional error handling layer as a wrapper around original procedures/functions, and then using the underlying layer in the database, and the wrapper when connecting from outside. But, this is rarely the case. Most of such procedures I've seen are used both from the database and from the outside world. So, this approach effectively cancels all benefits of exceptions in the database. Your application logic becomes mixed with error handling code, and you have to check for errors after all operations. Error codes are easy to ignore which is often the case in code written under tight deadlines. This can lead to errors that are extremely hard to troubleshoot. All of this also applies to client code, where error codes will have to be checked after every operation. In fact, most of the properly written clients would check for the error code and throw an exception in case of error. So additional code is being written on the client side to throw the exception, but the original cause of problems was lost along with the database stack trace.
If web developers are promised that no errors will get thrown from the database, they might just choose to ignore any unexpected problems, and blame you for everything.
If cluttered code and passing the buck are not enough for you – here is a common scenario with a really big, real and serious problem that will make you think twice before writing those two additional output parameters again:
Imagine a batch update method that changed half the records and then ran into a problem (for example, money is taken from account but one of requested books is no longer available). The method could not handle the problem at that level, and threw an exception. Outer Pl/Sql procedure or block handles the exception by setting the error code parameter and returning. The web displays the message and asks the user kindly to choose another book and start again. A relatively common scenario – I'm sure that you can find a similar example in your applications. What you don't expect is that the account changes would get committed along the way – which is exactly what will happen if the client was in autocommit mode.
Most modern database connectivity drivers support “autocommit” mode, executing each call as an implicit transaction which is committed or rolled back depending on whether the call completed successfully or raised an exception. In the account example, the call completed successfully - there was no exception and the driver could not care less about your custom error message. The problem becomes even more interesting because autocommit mode is default for .Net and Java clients. So, in order for your code to be used properly, clients must explicitly turn off autocommit mode, begin and complete transactions.
To those of you who are now thinking "I don't care about the web code" or "it's not so hard to write a commit", you are not doing your job if it's harder to use your code properly than to misuse it. And yes, it's not hard to call the commit method, but wrapping exceptions into error messages has no real benefit, and requires additional code both in the database (to catch the exception) and in web (to throw it again, commit or rollback).
A workaround might be to commit or rollback directly in stored procedures, but then you are implying that they will never be used in a wider scope, which is, in my experience, a sure a sure ticket to disaster. Anyway, you are writing and maintaining more code than you should.
The solution is quite easy - follow the way Samurai Principle - "Return victorious, or not at all". All popular client platforms now support exceptions, and will wrap the underlying database exception properly, so it's actually less code for you, less code for your clients, and less chance to get an urgent support call at 3 AM on a Saturday.
To conclude, wrapping exceptions into error messages:
- clutters code both on client and in the database
- requires more effort to write and maintain additional code
- cancels all benefits of exceptions in database
- makes it very hard to troubleshoot strange edge cases
- makes it easy to ignore errors, causing problems that are hard to diagnose and troubleshoot
- may lead to data corruption
and should not be considered a best practice by any standard. Use exceptions for error handling - that's what they are intended for. You will write less code and the system will be much easier to maintain.