Is it ok to catch all exception types if you rethrow them wrapped another exception?

Question

I know you're not suppose to write code that caches all exception types like this.

try
{
  //code that can throw an exception
}
catch
{
   //what? I don't see no
}

Instead you're suppose to do something more like the code below allowing any other exception that you didn't expect to bubble up.

try
{
//code that can throw an exception
}
catch(TypeAException)
{
   //TypeA specific code
}

catch(TypeBException)
{
   //TypeB specific code
}

But is it ok to catch all exception types if you are wrapping them with another exception? Consider this Save() method below I am writing as part of a Catalog class. Is there anything wrong with me catching all exception types and returning a single custom CatalogIOException with the original exception as the inner exception?

Basically I don't want any calling code to have to know anything about all the specific exceptions that could be thrown inside of the Save() method. They only need to know if they tried to save a read only catalog (CatalogReadOnlyException), the catalog could not be serialized (CatalogSerializationException), or if there was some problem writing to the file (CatalogIOException).

Is this a good or bad way to handle exceptions?

/// <summary>
/// Saves the catalog
/// </summary>
/// <exception cref="CatalogReadOnlyException"></exception>
/// <exception cref="CatalogIOException"></exception>
/// <exception cref="CatalogSerializingExeption"></exception>
public void Save()
{
    if (!this.ReadOnly)
    {
        try
        {
            System.Xml.Serialization.XmlSerializer serializer = new XmlSerializer(typeof(Catalog));
            this._catfileStream.SetLength(0); //clears the file stream
            serializer.Serialize(this._catfileStream, this);
        }
        catch (InvalidOperationException exp)
        {
            throw new CatalogSerializationException("There was a problem serializing the catalog", exp);
        }
        catch (Exception exp)
        {
            throw new CatalogIOException("There was a problem accessing the catalog file", exp);
        }
    }
    else
    {
        throw new CatalogReadOnlyException();
    }
}

---

Update 1

Thanks for all the responses so far. It sounds like the consensus is I shouldn't be doing this, and I should only be catching exceptions if I actually have something to do with them. In the case of this Save() method there really isn't any exception that may be thrown that I want to handle in the Save() method itself. Mostly I just want to notify the user why they were not able to save.

I think my real problem is I'm using exceptions as a way to notify the user of problems, and I'm letting this inform how I am creating and handling exceptions a little too much. So instead should it sounds like it would be better to not catch any exceptions and let the UI layer figure out how to notify the user, and or crash. Is this correct? Consider the Save Menu event handler below.

    private void saveCatalogToolStripMenuItem_Click(object sender, EventArgs e)
    {
        //Check if the catalog is read only
        if (this.Catalog.ReadOnly)
        {
            MessageBox.Show("The currently opened catalog is readonly and can not be saved");
            return;
        }

        //attempts to save
        try
        {
            //Save method doesn't catch anything it can't deal with directly
            this.Catalog.Save(); 
        }
        catch (System.IO.FileNotFoundException)
        {
            MessageBox.Show("The catalog file could not be found");
        }
        catch (InvalidOperationException exp)
        {
            MessageBox.Show("There was a problem serializing the catalog for saving: " + exp.Message);
        }
        catch (System.IO.IOException exp)
        {
            MessageBox.Show("There was a problem accessing the catalog file: " + exp.Message);
        }
        catch (Exception exp)
        {
            MessageBox.Show("There was a problem saving the catalog:" + exp.Message);
        }
    }

Update 2

One more thing. Would the answer change at all if the Save() method was part of a public API vs internal code? For example if it was part of a public API then I'd have to figure out and document all the possible exceptions that Save() may throw. This would be a lot easier if knew that Save() could only possibly throw one of my three custom exceptions.

Also if Save() was part of a public API wouldn't security also be a concern? Maybe I would want to let the consumer of the API know that the save wasn't successful, but I don't want expose anything about how Save() works by letting them get at the exceptions that may have been generated.

Answer 1

Doing a generic catch-all and rethrowing as a new type of exception does not really solve your problem and does not give you anything.

What you really need to do is to catch the exceptions that you can handle and then handle them (at the appropriate level - this is where rethrowing may be useful). All other exceptions either need to be logged so you can debug why they happened, or shouldn't be happening in the first place (for example - make sure you validate user input, etc.). If you catch all exceptions, you'll never really know why you're getting the exceptions you're getting and, thus, cannot fix them.

Updated Response

In response to the update of your question (particularly in how you want to handle the save case), my question to you would be - why are you using exceptions as a means of determine the path your program takes? For example, let's take the "FileNotFoundException." Obviously that can happen at times. But, instead of letting a problem happen and notifying the user about it, before saving (or doing whatever) with the file, why not check first that the file can be found. You still get the same effect, but you aren't using exceptions to control program flow.

I hope this all makes sense. Let me know if you have any additional questions.