I am confused because in quite a few places I've already read that the so-called 'boneheaded' exceptions (ones that result from bugs in code) are not supposed to be caught. Instead, they must be allowed to crash the application:
- Vexing exceptions, by Eric Lippert
- A comment under Eliding Async and Await, by Stephen Cleary
- Answer below Is it a good practice to use self-defined exception?, by Draco18s no longer trusts SE
At least two of the three above people are established authorities.
I am surprised. Especially for some (important!) use cases, like server side code, I simply can't see why is catching such an exception suboptimal and why the application must be allowed to crash.
As far as I'm aware, the typical solution in such a case is to catch the exception, return HTTP 500 to the client, have an automatic system that sends an emergency e-mail to the development team so that they can fix the problem ASAP - but do not crash the application (one request must fail, there's nothing we can do here, but why take the whole service down and make everyone else unable to use our website? Downtime is costly!). Am I incorrect?
Why am I asking - I'm perpetually trying to finish a hobby project, which is a browser based game in .net core. As far as I'm aware, in many cases the framework does for me out of the box the precise thing Eric Lippert and Stephen Cleary are recommending against! - that is, if handling a request throws, the framework automatically catches the exception and prevents the server from crashing. In a few places, however, the framework does not do this. In such places, I am wrapping my own code with try {...} catch {...}
to catch all possible 'boneheaded' exceptions.
One of such places, AFAIK, is background tasks. For example, I am now implementing a background ban clearing service that is supposed to clear all expired temporary bans every few minutes. Here, I'm even using a few layers of all-catching try
blocks:
try // prevent server from crashing if boneheaded exception occurs here
{
var expiredBans = GetExpiredBans();
foreach(var ban in expiredBans)
{
try // If removing one ban fails, eg because of a boneheaded problem,
{ // still try to remove other bans
RemoveBan(ban);
}
catch
{
}
}
}
catch
{
}
(Yes, my catch
blocks are empty right now - I am aware that ignoring these exceptions is unacceptable, adding some logging is perpetually on my TODO list)
Having read the articles I linked to above, I can no longer continue doing this without some serious doubt... Am I not shooting myself in the foot? Why / Why not?
If and why should boneheaded exceptions never be caught?
// TODO
comments in either. You never actually get back to properly handling either. (I was just recently burned wasting about 2 days in legacy code that swallowed a NPE in an empty catch block.)GetExpiredBans
andRemoveBan
? The assumption here seems to be "these can fail arbitrarily at any time and we can always ignore that failure". OK, if that's the case then put the try-catch inside them and then the caller doesn't need them. Why did you decide to put the try-catch in the caller, rather than the callee?