Ignore exception when closing a resource such as a file?

Question

From Effective Java 2e by Joshua Bloch,

An example of the sort of situation where it might be appropriate to ignore an exception is when closing a FileInputStream. You haven’t changed the state of the file, so there’s no need to perform any recovery action, and you’ve already read the information that you need from the file, so there’s no reason to abort the operation in progress. Even in this case, it is wise to log the exception, so that you can investigate the matter if these exceptions happen often.

Is it wise to always ignore that exception, as suggested. The code then would look like,

FileInputStream stream = null;
try {
    stream = new FileInputStream("foo.txt");
    // do stuff
} finally {
    if (null != stream) {
        try {
            stream.close();
        } catch (IOException e) {
            LOG.error("Error closing file. Ignoring this exception.", e);
        }
    }

which is much more verbose than

try (FileInputStream stream = new FileInputStream("foo.txt")) {
    // do stuff
}

But is the extra verbosity worth ignoring the exception from closing the file? Is it better to ignore the exception since there's nothing to be done? Should the same approach be used with resources other than a file, such as a database connection?

Answer 1

The reason, which as I recall Bloch explains, is that the interesting Exception is the original one thrown from within the try block. Any Exception thrown within the finally block will hide the original Exception.

Imagine that the ur-Exception is because the File is on a network and the network is down. That's the more informative Exception that you want to throw. But that problem also affects the close() , and you don't want the 2nd exception to "get in the way".

Note this explanation was in V1 of his book...

Added The following code proves the point:

try {
  throw new Exception("in try, here is the ur-Exception");
}
finally {
  throw new Exception("in finally");
}

The second, "in finally" Exception, is the one you will see, hiding the original Exception.

Answer 2

For argument’s sake, I can’t imagine how this operation could fail, and what could go wrong if it fails.

Therefore, I would do nothing except logging in production. And make sure that the app runs into a breakpoint when running on a developers machine. If this ever fails, I want to know about it, and why it happened, so I can figure out more correctly how to handle it.

Answer 3

In general you should expect your program to run without ever throwing an exception.

Therefore, when an exception is thrown, its unusual that you would want to ignore it.

However, it's very usual that you don't want your program to crash. So people put top level try catch do nothing blocks to stop stuff they haven't thought of from crashing their applications completely.

In this case though really you do want to log the exception you hadn't thought of, so that you can update your code to deal with it properly.

In the FileInputStream example, yes it's not really clear why this would ever throw an exception (perhaps the file has been deleted?) or what you would do about it if it did.

Potentially I suppose if the close fails to complete successfully you might have a memory leak. So if its happening a lot you will want to know about it.

Answer 4

then and now, very sometimes. Yes. But always? By God - never!

First note: you should be careful about over-generalizing a rule from a precise example.

Compare

void close(Closable target) {
    try {
        target.close();
    } catch (IOException e) {
        ???
    }
}

We don't have nearly enough information to dismiss this exception as unimportant. There are many exceptional situations which prevent a resource from closing successfully.

void close(FileInputStream target) {
    try {
        target.close();
    } catch (IOException e) {
        ???
    }
}

Here, we have the additional context that we are reading data (no worries about being corrupted), and that the caller had already obtained the expected data (the caller is invoking close()). In other words, although we are in an exceptional situation, the circumstances don't indicate that the caller will be unable to ensure its explicit postcondition.

We shouldn't confuse a case where we accrue business value but leak a file handle with a case where there are indications of actual data loss.

I personally wouldn't get too hung up on verbosity; you can use another Closable to do the work

try( ClosePolicy closer = new LoggingClosePolicy(new FileInputStream(...))) {
    FileInputStream stream = closer.get();
    // ...
}

which I think is a bit better on the intention revealing scale: the default strategy for exceptions during close isn't satisfactory, so we are replacing it.

With that debris cleared, I think you can then start to address your original question: should we always...? It's a checked exception, so we cannot passively ignore the possibility.

I see two possibilities; if the local method cannot establish its postcondition in the face of the exception, then I think you need to propagate it in some form.

But I think it's much more common that the local method will be able to establish the observable portions of the post condition, in which case log-and-move-on seems like a reasonable way to accrue business value when the exceptional circumstances don't require immediate remediation.

Answer 5

Note that this problem is not limited to the Java language, or for that matter languages that have exceptions. You can face exactly the same problem when programming in C. Do you check the return value from fclose()? Or from close()?

I have seen both approaches: check and not check. Personally, I try to code finalizers so that they don't return an error code, just silently ignoring all errors. In some cases when I'm interested about catching bugs, I fail early in a big way, by calling abort() if I see that some important precondition is violated. The core dump left behind explains everything. Needless to say, in the final program the abort() function is never called. Don't use abort() if e.g. a network connection or some external resource fails; it's only for preconditions!

In the case of files, you probably won't fail closing it. But however, in the case of some network streams, there may be perhaps an error condition.

Also, you could consider logging the exception and then ignoring it silently. If you see lots of such exceptions in the logs, you can turn off logging for the biggest log size offenders.

Answer 6

To me there's one simple rule about exceptions:

If a method doesn't fulfill its contract, it throws an exception.

Now there are explicit and implicit aspects of a method's contract. The explicit contract typically (hopefully) correlates with its name. Implicit contract parts can be leaving the file system in the same state as before the method call, not creating resource leaks, and so on. It's a design decision, which implicit duties a given method has.

What's the contract of your method?

• E.g. to read some data from a configuration file.

What's the effect of a failure when closing the stream?

• The file has been read successfully, so the explicit contract has been fulfilled.
• Failing to close a FileInputStream results in some resource leak. For a single configuration file read once in the application's run-time, that's hardly a problem, so typically doesn't violate the implicit contract.
• Not closing a FileInputStream might (depending on the environment: OS, file system, ...) leave a lock on the file, inhibiting later modification by some other process. As configuration files typically aren't modified often (and in our case need an application restart when modified), that also is acceptable behaviour of our read method.

So yes, in this case of reading a single configuration file, I'd log the exception with a WARN level and return without an exception.

But if the method were the fileCopy() function of a file manager application, then not closing the source file with the risk to leave a lock on the file would definitely violate the implicit contract, so needing an exception.

Answer 7

The 3rd edition of Effective Java contains a new section with the title "Prefer try-with-resources to try-finally". In it, Bloch writes, "Always use try-with-resources in preference to try-finally when working with resources that must be closed," and provides a rationale which includes the 3 following points:

• An exception in the finally block will hide the exception in the try block, exactly as user949300 describes in his answer.

• try-finally is difficult to read when used with more than one resource.

• try-finally is often used incorrectly. "In fact, two-thirds of the uses of the close method in the Java libraries were wrong in 2007."

So to answer the question, Bloch seems to think that try-with-resources is worth it.

Answer 8

Bloch proposes to swipe the garbage under the rug and hide it from the user by pretending it is not there, which I am certain an honest programmer (or program) will never do. I should make the program fail and crash real hard in on order to detect all possible errors at their first occurrences. Error recovery is another matter, though.

There also seems to be a redundancy in your code—If you create the stream outside the try block, you will not have to check whether it is null in the finally because it will always have been created.