In this post I use the categorization of exceptions by @Eric Lippert which you can find here: Vexing exceptions
The most important ones in this case:
Boneheaded exceptions are your own darn fault, you could have prevented them and therefore they are bugs in your code. You should not catch them; doing so is hiding a bug in your code. Rather, you should write your code so that the exception cannot possibly happen in the first place, and therefore does not need to be caught.
Exogenous exceptions appear to be somewhat like vexing exceptions except that they are not the result of unfortunate design choices. Rather, they are the result of untidy external realities impinging upon your beautiful, crisp program logic. Always handle exceptions that indicate unexpected exogenous conditions; generally it is not worthwhile or practical to anticipate every possible failure. Just try the operation and be prepared to handle the exception.
Like every developer probably experienced it is not possible to avoid boneheaded exceptions to 100% in a large enterprise software.
In the unfortunate situation that a boneheaded exceptions is thrown, i want to inform the user, so that he will report the bug to us (third level support). In addition, I want to log a message with log level "Error" in this case.
For exogenous exceptions i want to show the user a more specific message with some hints, because he can probably fix the problem himself (maybe with help of first or second level support)
The way I currently achieve this, is by catching ONLY exogenous exceptions explicitly in low level components and wrapping them into custom exceptions. In the top layer (in my case the ViewModel of a MVVM WPF application) I then explicitly catch the custom exception, to show the warning. In a second catch block I catch general Exceptions to show the error.
Is this a common and good practices to distinguish between boneheaded exceptions and exogenous exceptions in enterprise applications? Is there a better approach? Or is it not necessary at all?
After reading this article dotnetpro - Implementierungsausnahmen I am also wondering, if I should wrap all (also boneheaded) exceptions into custom exceptions to provide more context information when logging them?
About wrapping all exceptions I found following posts: stackoverflow - Should I catch and wrap general Exception? and stackoverflow - Should I catch all possible specific exceptions or just general Exception and wrap it in custom one? It seams to be pretty controversial and depended on the use-case, so I am not sure in my case.
Example for a high level catch handler in a ViewModel:
public class MainWindowViewModel
{
private readonly ICustomerRepository _customerRepository;
public MainWindowViewModel(ICustomerRepository customerRepository)
{
_customerRepository = customerRepository;
PromoteCustomerCommand = new DelegateCommand(PromoteCustomer);
}
public ICommand PromoteCustomerCommand { get; }
private void PromoteCustomer()
{
try
{
Customer customer = _customerRepository.GetById(1);
customer.Promote();
}
catch (DataStoreLoadException ex)
{
// A expected exogenous exception. Show a localized message with some hints and log as warning.
Log(LogLevel.Warning, ex);
ShowMessage("Unable to promote customer. It could not be loaded. Try to...", ex);
}
catch (Exception ex)
{
// A unexpected boneheaded exception. Show a localized message, so that the users contacts the support and log as error.
Log(LogLevel.Error, ex);
ShowMessage("Unable to promote customer because of an unknown error. Please contact support@example.com", ex);
}
}
}
Example for a low level exception wrapping:
public class SqlCustomerRepository : ICustomerRepository
{
public Customer GetById(long id)
{
try
{
return GetFromDatabase(id);
}
catch (SqlException ex)
{
// Wrap the exogenous SqlException in a custom exception. The caller of ICustomerRepository should not depend on any implementation details, like that the data is stored in a SQL database.
throw new DataStoreLoadException($"Unable to get the customer with id {id} from SQL database.", ex);
}
// All other exceptions bubble up the stack without beeing wrapped. Is it a good idea, or should I do something like this to provide additional context? (Like the id in this case)
/*catch (Exception ex)
{
throw new DataStoreException($"Unknown error while loading customer with id {id} from SQL database.", ex);
}*/
}
}