Is it okay that I sometimes sink my exceptions?

Question

I have a best practices question. I realize this is subjective but wanted to ask people smarter than me if this is a common programming practice.

If you have a method of something NON-CRITICAL that you don't want to interfere with the important functioning of your application, is it common to use an error sink like this?

Try 
    'do stuff.  not important if it fails.

Catch ex as exception
    'sink.  do nothing.
End Try

If you were thinking of hiring me and you were reading some of my code and saw this...would you?

Seth

EDIT Wow! Thanks for your answers. I think the consensus is that should never be done or it should be very rare.

I thought I would give you context for this question. First, I have a strong familiarity with the Karl Sequin article and have followed that pattern for years.

But today on the project I was working on, I was working through the change list and was faced with the addition of a simple feature. (If you care to know...it is adding context menu support to a Rich Text Box.)

The attached note said, "if it takes longer than 15 mins...drop it."

So I am faced with adding what is a potentially useful feature but the don't really have the time to test that it won't break working features. For the record, our exception handler for this system DOES have a mechanism for handling and sinking or logging these errors. But what if I was working on a system that did not have a robust error handling system. Would it be okay to add this feature and if an error occurs...nothing is really lost.

That was my thinking. But I have taken your message to heart...that basically this is a bad idea.

Seth

Answer 1

You should never hide Exception. I might hire you anyway, but you wouldn't do that while you worked for me :)

Seriously, though, in Java (for instance) Exceptions are used for certain cases that you may choose to ignore, like FileNotFoundException (like you did, with a comment). But if you ever catch a type of Exception -- like IOException -- you cannot just hide it. If the Exception means nothing to you in particular, wrap it in a RuntimeException and throw it up to the client.

Somewhere along the way, Exception should be handled, but never hidden.

Answer 2

You shouldn't be catching all exceptions here. It may, on a rare occasion, be okay to ignore a particular, expected exception. However, catching all exceptions and swallowing them will mean that you try to continue after a OutOfMemoryException and other irrecoverable exceptions

Answer 3

As a rookie assigned to doing bug fixes, I discovered that the particular bug I had been assigned was printing (to debug) "This should never happen." After getting over my fear of working in an error condition that apparently was impossible, I finally managed to discover that it was caused by a subtle race condition. If the author of that code originally had simply sunk the exception without at least that meager debug statement, it would have been much more difficult to track down the race condition.

Since then I've become a huge proponent of either dealing with exceptions or propegating them until the error condition is shown in a user friendly way to the end-user, a step futher from simply logging a debug statement.

Answer 4

You should make a distinction between ignorable exceptions and fatal exceptions. I'm not sure how this is done in VB, but in Java (for example), the exceptions derived from Error should never be ignored. This includes things like class loader failures, out of memory conditions, stack overflow, etc.

Answer 5

No, that's not OK in my opinion.

Your system might not be critical, but presumably you do want it to run otherwise you wouldn't have written it in the first place. Now if one day it stops running, the people who have to maintain it have no idea why it's suddenly not working, as you're completely hiding the error. In fact it might even take them a long time to realize where the error is coming from or even that there is an error in the first place. It could waste a lot of developer time, whereas it wouldn't have taken much time to log the error instead of discarding it.

And you're catching exceptions like OutOfMemoryException that you really don't want to catch and discard. If that code is non-critical but is consuming far too much memory and making your whole system slow, you want to be told that so you can fix it.

Answer 6

Generally I'd say it's bad practice--but let's say the method in question was a logging method. I don't want my app to crash because the EventLog is full. In a case like that, I'd say it's okay. I'd still output it to the Debug window though.