Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I've found a piece of code like this in one of our projects:

    SomeClass QueryServer(string args)
    {
        try
        {
            return SomeClass.Parse(_server.Query(args));
        }
        catch (Exception)
        {
            return null;
        }
    }

As far as I understand, suppressing errors like this is a bad practice, as it destroys useful information from the original server's exception and makes the code continue when it actually should terminate.

When is it appropriate to completely suppress all errors like this?

share|improve this question
13  
Eric Lippert wrote a fine blog post called vexing exceptions where he classifies exceptions into 4 different categories and suggests what the right approach is for each one. Written from a C# perspective but applicable across languages, I believe. – Damien_The_Unbeliever Feb 23 at 14:32
3  
I believe that code violates "Fail-fast" principle. There is high probability that actual bug is hidden in it. – Euphoric Feb 23 at 14:46
7  
3  
You're catching to much. You'd be catching bugs, too, such as NRE, array index out of bounds, configuration error, ... That's prevents you from finding those bugs and they will continually cause damage. – usr Feb 23 at 17:02
1  
up vote 49 down vote accepted

Imagine code with thousands files using a bunch of libraries. Imagine all of them are coded like this.

Imagine, for example, an update of your server causes one configuration file disappear; and now all you have is a stack trace is a null pointer exception when you try using that class: how would you resolve that? It could take hours, where at least just logging the raw stack trace of the file not found [file path] may enable you to resolve momentarily.

Or even worse: a failure in one off the libraries you use after an update that makes your code crash later on. How can you track this back to the library ?

Even without robust error handling just doing

throw new IllegalStateException("THIS SHOULD NOT HAPPENING")

or

LOGGER.error("[method name]/[arguments] should not be there")

may save you hours of time.

But there are some cases where you might really want to ignore the exception and return null like this (or do nothing). Especially if you integrate with some badly designed legacy code and this exception is expected as a normal case.

In fact when doing this you should just wonder if you really are ignoring the exception or "properly handling" for your needs. If returning null is "properly handling" your exception in that given case, then do it. And add a comment why this is the proper thing to do.

Best practices are things to follow in most of cases, maybe 80%, maybe 99%, but you'll always find one edge case where they don't apply. In that case, leave a comment why you're not following the practice for the others (or even yourself) who will read your code months later.

share|improve this answer
7  
+1 for explaining why you think you need to do this in a comment. Without an explanation, exception swallowing would always raise alarm bells in my head. – jpmc26 Feb 23 at 18:48
    
My problem with that is that the comment is stuck inside that code. As a consumer of the function, I might never get to see that comment. – corsiKa Feb 24 at 0:31
    
@jpmc26 If the exception is handled (for example by returning a special value), that isn't exception-swallowing at all. – Deduplicator Feb 24 at 2:34
2  
@corsiKa a consumer don't need to know the implementation detail if the function is doing its job properly. maybe they're some of this in apache libraries, or spring and you don't know it. – Walfrat Feb 24 at 7:49
    
@Deduplicator yes but using a catch as a part of your algorithm isn't supposed to be a good pratice too :) – Walfrat Feb 24 at 7:49

There are cases where this pattern is useful - but they are typically used when the exception generated should never have been (ie when exceptions are used for normal behaviour).

For example, imagine you have a class that opens a file, stores it in the object that is returned. If the file does not exist, you may consider that to not be an error case, you return null and let the user create a new file instead. The file-open method may throw an exception here to indicate no file present, so silently catching it may be a valid situation.

However, its not often you'd want to do this, and if the code is littered with such a pattern you'd want to deal with it now. At the very least I'd expect such a silent catch to write a log line saying this is what has happened (you have tracing, right) and a comment to explain this behaviour.

share|improve this answer
2  
"used when the exception generated should never have been" no such thing. The POINT of exception is to signalize something went horribly wrong. – Euphoric Feb 23 at 14:44
2  
@Euphoric But sometimes the writer of a library doesn't know the intentions of the end user of that library. One person's "horribly wrong" may be someone else's easily recoverable condition. – Simon B Feb 23 at 15:03
2  
@Euphoric, it depends on what your language considers "horribly wrong": Python folks like exceptions for things that are very close to flow-control in (e.g., ) C++. Returning null might be defensible in some situations--e.g., reading from a cache where items could be suddenly and unpredictably evicted. – Matt Krause Feb 23 at 15:15
8  
@Euphoric, "File not found is not a normal exception. You should first check if file exists if there is possibility it might not exist." No! No! No! No! That is classic wrong thinking around atomic operations. The file might be deleted between you checking if it exists and accessing it. The only correct way to handle such situations is to access it and handle the exceptions if they occur, eg by returning null if that's the best your chosen language can offer. – David Arno Feb 23 at 16:30
3  
@Euphoric: So, write the code to handle not being able to access the file at least twice? That violates DRY, and also unexercised code is broken code. – Deduplicator Feb 23 at 19:29

This is 100% context dependent. If the caller of such function has the requirement to display or log errors somewhere, then it is obviously nonsense. If the caller ignored all errors or exception messages either, it would not make much sense to return the exception or its message. When the requirement is to make the code only terminate without displaying any reason, then a call

   if(QueryServer(args)==null)
      TerminateProgram();

would probably be sufficient. You have to consider if this will make it hard to find the cause for the error by the user - if that is the case, this is the wrong form of error handling. However, if the calling code looks like this

  result = QueryServer(args);
  if(result!=null)
      DoSomethingMeaningful(result);
  // else ??? Mask the error and let the user run into trouble later

then you should have an argument with the developer during code review. If someone implements such a form of non-error handling just because he is too lazy to find out about the correct requirements, then this code should not go into production, and the author of the code should be taught about the possible consequences of such laziness.

share|improve this answer

In the years I have spent programming and developing systems, there are only two situations where I found the pattern in question useful (in both cases the supression contained also logging of the thrown exception, I do not consider plain catch and null return as a good practice).

The two situations are the following:

1. When the exception was not considered an exceptional state

This is when you do an operation on some data, which may throw, you know it may throw but you still want your application to keep running, because you do not need the processed data. If you receive them, it's good, if you do not, it is also good.

Some optional attributes of a class may come in mind.

2. When you're providing a new (better, faster?) implementation of a library using an interface already used in an application

Imagine you have an application using some sort of an old library, which did not throw exceptions but returned null on error. So you created an adapter for this library, pretty much copying the original API of the library, and are using this new (still non-throwing) interface in your application and handling the null checks yourself.

A new version of the library comes, or perhaps a completely different library offering the same functionality, which, instead of returning nulls, throws exceptions and you want to use it.

You do not want to leak the exceptions to your main application, so you supress and log them in the adapter you create to wrap this new dependency.


The first case is not an issue, it is the desired behaviour of the code. In the second situation, however, if everywhere the null return value of the library adapter really means an error, refactoring the API to throw an exception and catching it instead of checking for null may be (and code-wise usually is) a good idea.

I personally use exception supression only for the first case. I have only used it for the second case, when we did not have the budget to make the rest of the application work with the exceptions instead of nulls.

share|improve this answer

While it seems logical to say that programs should only catch exceptions they know how to handle, and can't possibly know how to handle exceptions which weren't anticipated by the programmer, such a claim ignores the fact that many operations can fail in a nearly limitless number of ways which have no side-effects, and that in many cases the proper handling for the vast majority of such failures will be identical; the exact details of the failure will be irrelevant, and consequently it shouldn't matter whether the programmer anticipated them.

If, for example, the purpose of a function is to read a document file into a suitable object and either create a new document window to show that object or report to the user that the file couldn't be read, an attempt to load an invalid document file shouldn't crash the application--it should instead show a message indicating the problem but let the rest of the application continue running normally unless for some reason the attempt to load the document has corrupted the state of something else in the system.

Fundamentally, the proper handling of an exception will often depend less upon the exception type than the location where it was thrown; if a resource is guarded by a read-write lock, and an exception is thrown within a method that has acquired the lock for reading, the proper behaviour should generally be to release the lock since the method can't have done anything to the resource. If an exception is thrown while the lock is acquired for writing, the lock should often be invalidated, since the guarded resource may be in an invalid state; if the locking primitive doesn't have an "invalidated" state, one should add a flag to track such invalidation. Releasing the lock without invalidating it is bad because other code may see the guarded object in an invalid state. Leaving the lock dangling, however, isn't a proper solution either. The right solution is to invalidate the lock so that any pending or future attempts at acquisition will immediately fail.

If it turns out that an invalidate resource gets abandoned before there's any attempt to use it, there's no reason it should bring down the application. If an invalidated resource is critical to an application's continued operation, the application will need to be brought down but invalidating the resource will likely make that happen. The code which received the original exception will often have no way of knowing which situation applies, but if it invalidates the resource it can ensure that the correct action will end up being taken in either case.

share|improve this answer
2  
This fails to answer the question because handling an exception without knowing the exception details != silently consuming a (generic) exception. – Taemyr Feb 24 at 11:10
    
@Taemyr: If the purpose of a function is to "do X if possible, or else indicate that it wasn't", and X wasn't possible, it will be often impractical to list all the kinds of exceptions that neither the function nor the caller will care about beyond "It wasn't possible to do X". Pokemon exception handling is often the only practical way to make things work in such cases, and need not be nearly as evil as some people make out if code is disciplined about invalidating things which become corrupted by unexpected exceptions. – supercat Feb 24 at 16:11
    
Exactly. Every method should have a purpose, if it can fullfill it's purpose regardless of whether some method it calls throws an exception or not, then swallowing the exception, whatever it is, and doing it's job, is the right thing to do. Error handling is fundamentally about completing the task after an error. That is what matters, not what error occurred. – jmoreno Mar 3 at 7:08
    
@jmoreno: What makes things difficult is that in many situations there will be a large number of exceptions, many of which a programmer wouldn't particularly anticipate, which would not indicate a problem, and a few exceptions, some of which a programmer wouldn't particularly anticipate, which do indicate a problem, and no defined systematic way to distinguish them. The only way I know of to sanely deal with that is to have exceptions invalidate things that were corrupted, so if they matter they get noticed and if they don't matter they get ignored. – supercat Mar 3 at 14:56

Swallowing this sort of error isn't particularly useful to anyone. Yes, the original caller may not care about the exception but someone else might.

Then what do they do? They'll add code to handle the exception to see what flavour of exception they're getting back. Great. But if the exception handler is left in, the original caller no longer gets null back and something breaks elsewhere.

Even if you're aware that some upstream code could throw an error and thus return null, it would be seriously inert of you not to at least try to head off the exception in the calling code IMHO.

share|improve this answer
    
"seriously inert" - did you perhaps mean "seriously inept"? – Esoteric Screen Name Feb 23 at 15:36
    
@EsotericScreenName Pick one... ;-) – Robbie Dee Feb 23 at 17:42

I've seen examples where third-party libraries had potentially useful methods, except that they throw exceptions in some cases that should work for me. What can I do?

  • Implement exactly what I need myself. Can be difficult on a deadline.
  • Modify the third-party code. But then I have to look for merge conflicts with every upgrade.
  • Write a wrapper method to turn the exception into an ordinary null pointer, a boolean false, or whatever.

For example, the library method

public foo findFoo() {...} 

returns the first foo, but if there is none it throws an exception. But what I need is a method to return the first foo or a null. So I write this one:

public foo myFindFoo() {
    try {
        return findFoo() 
    } 
    catch (NoFooException ex) {
        return null;
    }
}

Not neat. But sometimes the pragmatic solution.

share|improve this answer
1  
What do you mean "throw exceptions where they should work for you"? – corsiKa Feb 24 at 0:32
    
@corsiKa, the example that comes to my mind are methods searching through a list or similar data structure. Do they fail when they find nothing or do they return a null value? Another example are waitUntil methods which fail on timeout rather than return a boolean false. – o.m. Feb 24 at 6:14

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.