Is logging every exception with FirstChanceException event handler a bad practice?

Question

I joined a company and the project I'm working on have an event handler that extends the FirstChanceException functionality to log the exceptions. The logs are stored in a file and not registered to the console. All the controllers actions have try catch blocks and they ignore the exception in the catch (because it's being logged in the FirstChanceException event handler) and return a value so the user doesn't get any errors.

I have a strong feeling that this is an antipattern or a bad practice but I don't have any good arguments.

Do you find this a bad practice and why, what are the downsides of this approach?

To add a bit more context, I found annoying that they don't show logs in the console (I can't debug an app without logs on the console) so I configured it to show them in my dev environment. Latter on, I started to find random exceptions like sockets closing and ES operations failing. These exceptions don't show up if you remove the FirstChanceException event handler because they are being catch and handled by .Net code itself. So that's a downside but the don't even used logs in the console in the first place.

Answer 1

This is a bad practice for two major reasons:

1. Extra code has been written - and has to continue to be written - for what the framework already does quite well. Register a logging exception handler within the pipeline framework being used itself and remove the try..catch (empty) paradigm from the controller actions. The actions should look pretty slim - validate incoming parameters, hand them off to a service layer, and then hand a successful result back. Let unhandlable errors be logged by the pipeline.

2. Actions in controllers are expected by and large to adhere to the REST standard. And that standard allows for error codes to be returned to the client. Analyzing an HTTP status is quick and easy and client frameworks can perform all sorts of useful boilerplate logic (such as retries, etc.) based on that.

Also, (opinion-based) I feel that hiding errors from consumers is akin to "security by obscurity" - maybe they don't see what's happening, but errors ARE preventing the actions from succeeding. The hunt has to begin in the file logs for what happened when - and first chance exceptions occur all the time in the framework itself - you're going to have a lot of noise to (hopefully) very little signal.

Answer 2

I don't see anything wrong with having a logging mechanism hook into the FirstChanceException handler. It might be the best way to guarantee that all exceptions get logged.

I do have a problem with controllers that consume exceptions without so much as telling the user that their request has failed. You don't want to bother the user with all the technical details (that is for the developers when they analyze what went wrong), but the user should at the very least be informed that their request was not successfully handled. How would you like it if the bank app reported to you that your rent for next month was successfully transferred, but in reality it wasn't due to some internal hiccup?

As for your issue with not having logs in the console, my first course of action would have been to (locally) modify that logging component to write the logs o the console as well as the file it currently uses.