The Exception handling seems designed to obfuscate rather than reveal

[devilgate]

I've just been struggling with some new code that was throwing an SQLException. But no matter what I did I could not get the details of that Exception to show in the log. In the end I had to debug the code and find the cause that way.

The problem is that the code has so many catch statements (sometimes within finally blocks, which is a serious code smell). At every point it tries to do something clever with the caught Exception. That would be fine if it were something that we could recover from in the code, but in this case, as in many, it is not. In such cases the Exception should just percolate to the highest level -- and, most importantly, it should appear in the log.

This particular instance was in SmoothResultsSubmissionStep, but I've seen similar situations all over the code. I feel it needs a root-and-branch approach, maybe introducing a few more specific Exceptions, rather than just having one, and including some unchecked Exceptions where appropriate.

[peterhambly]

I could not agree more. Really user defined exceptions should only be used for business rule failures; try catch for catching exceptions that have to be caught unless defined and for the purposes of error recovery logic (e.g. the print subsystem creates an error file for the front end to pick up in another REST call; this also stops it being run > once). What is needed is a good generic handler for the rest call top level that is consistently applied. That way we have one message in the log.