How to properly handle exceptions when working with files in C#

Question

I've read many blogs/articles/book chapters about proper exception handling and still this topic is not clear to me. I will try to illustrate my question with following example.

Consider the class method that has following requirements:

1. receive list of file paths as parameter
2. read the file content for each file or skip if there is any problem trying to do that
3. return list of objects representing file content

So the specs are straightforward and here is how I can start coding:

    public class FileContent
    {
        public string FilePath { get; set; }
        public byte[] Content { get; set; }

        public FileContent(string filePath, byte[] content)
        {
            this.FilePath = filePath;
            this.Content = content;
        }
    }

    static List<FileContent> GetFileContents(List<string> paths)
    {
        var resultList = new List<FileContent>();

        foreach (var path in paths)
        {
            // open file pointed by "path"
            // read file to FileContent object
            // add FileContent to resultList
            // close file
        }

        return resultList;
    }

Now note that the 2. from the specs says that method should "skip any file which content can't be read for some reason". So there could be many different reasons for this to happen (eg. file not existing, file access denied due to lack of security permissions, file being locked and in use by some other application etc...) but the point is that I should not care what the reason is, I just want to read file's content if possible or skip the file if not. I don't care what the error is...

So how to properly implement this method then?

OK the first rule of proper exception handling is never catch general Exception. So this code is not good then:

    static List<FileContent> GetFileContents(List<string> paths)
    {
        var resultList = new List<FileContent>();

        foreach (var path in paths)
        {
            try
            {
                using (FileStream stream = File.Open(path, FileMode.Open))
                using (BinaryReader reader = new BinaryReader(stream))
                {
                    int fileLength = (int)stream.Length;
                    byte[] buffer = new byte[fileLength];
                    reader.Read(buffer, 0, fileLength);

                    resultList.Add(new FileContent(path, buffer));
                }
            }
            catch (Exception ex)
            {
                // this file can't be read, do nothing... just skip the file
            }
        }

        return resultList;
    }

The next rule of proper exception handlig says: catch only specific exceptions you can handle. Well I do not I care about handling any specific exceptions that can be thrown, I just want to check if file can be read or not. How can I do that in a proper, the best-practice way?

Answer 1

Your requirements are clear - skip files that cannot be read. So what is the problem with the general exception handler? It allows you to perform your task in a manner that is easy, clean, readable, scalable and maintainable.

If at any future date you want to handle the multiple possible exceptions differently, you can just add above the general exception the catch for the specific one(s).

So you'd rather see the below code? Note, that if you add more code to handle the reading of files, you must add any new exceptions to this list. All this to do nothing?

try
{
    // find, open, read files
}
catch(FileNotFoundException) { }
catch(AccessViolation) { }
catch(...) { }
catch(...) { }
catch(...) { }
catch(...) { }
catch(...) { }
catch(...) { }

Conventions are guidelines and great to try to adhere to to create good code - but do not over-complicate code just to maintain some odd sense of proper etiquette.

To me, proper etiquette is to not talk in bathrooms - ever. But when the boss says hello to you in there, you say hello back. So if you don't care to handle multiple exceptions differently, you don't need to catch each.

---

Edit: So I recommend the following

try
{
    // find, open, read files
}
catch { } // Ignore any and all exceptions

The above tells me to not care which exception is thrown. By not specifying an exception, even just System.Exception, I've allowed .NET to default to it. So the below is the same exact code.

try
{
    // find, open, read files
}
catch(Exception) { } // Ignore any and all exceptions

Or if you're going to log it at least:

try
{
    // find, open, read files
}
catch(Exception ex) { Logger.Log(ex); }  // Log any and all exceptions

Answer 2

Although it's generally not considered to be good practice to catch and swallow non-specific exceptions, the risks are often overstated.

After all, ASP.NET will catch a non-specific exception that is thrown during processing of a request, and after wrapping it in an HttpUnhandledException, will redirect to an error page and continue happily on it's way.

In your case, if you want to respect the guideline, you need a complete list of all exceptions that can be thrown. I believe the following list is complete:

UnauthorizedAccessException IOException FileNotFoundException DirectoryNotFoundException PathTooLongException NotSupportedException (path is not in a valid format). SecurityException ArgumentException

You probably won't want to catch SecurityException or ArgumentException, and several of the others derive from IOException, so you'd probably want to catch IOException, NotSupportedException and UnauthorizedAccessException.