Remove logging of ApplicationException

[pikanji]

This fix resolves #4568 .

Even though logging is suppressed by this dontReport list, ApplicationException is logged by ErrorHandler.
ErrorHandler::beforeHandleError is the one that logging ApplicationException.
It is called by ErrorHandler::handleException, which is called in the listener of exception.beforeRender event registered by ServiceProvider::registerErrorHandler

[LukeTowers]

@pikanji ApplicationException was added in January 2015: octobercms/library@6a68036. Logging was added in February 2015 (7b52e07#diff-547f5794a63e9f138c33b20de8649b7eR20-R31)

We'll need to get @daftspunk to make the decision on this one.

[bennothommo]

@LukeTowers my two cents - given that it's mentioned twice that it is not meant to be logged, and it's only been added ad-hoc to the ErrorHandler to be logged, I think it's probably a safe bet it is not meant to be logged.

[github-actions]

This pull request will be closed and archived in 3 days, as there has been no activity in the last 30 days. If this is still being worked on, please respond and we will re-open this pull request.

[daftspunk]

This looks like it might have been a typo, it should read SystemException. Overall October's platform makes inconsistent use of these exceptions likely because of the general confusion around their meaning.

To clarify the difference:

• SystemException occurs when something unhealthy happened within the system. It couldn't write to the disk. The database has fallen over. An unhappy path has occurred due to a failure in the running system itself, either by configuration or environment or by a logic error from the programmer. In a perfect world, a System Exception should never happen.

• ApplicationException occurs when the user has caused something to go wrong. They've filled out a form incorrectly. They didn't follow a procedure properly. These are treated as basic validation errors usually deriving from an expected path in the software where the application is not being used correctly. In a normal operating environment, Application Exceptions are likely to occur frequently.

It would appear October in general needs an overall audit to ensure the correct use of these exception types is applied.

It would also make sense that SystemException is logged, but ApplicationException probably shouldn't be logged by default.

[daftspunk]

I'm leaving this open because it contains actionable information but the current code itself should not get merged; ApplicationException should be renamed to SystemException.

[LukeTowers]

@daftspunk do you mean rename the exception class itself? I use it all the time in my project's for its intended purpose, telling the user they've done something wrong.

[daftspunk]

I mean don't log messages that tell the user they've done something wrong, only log system errors.

Change code to this, but maybe look at the impact of doing so first:

    /**
     * We are about to display an error page to the user,
     * if it is an SystemException, this event should be logged.
     * @return void
     */
    public function beforeHandleError($exception)
    {
        if ($exception instanceof SystemException) {
            Log::error($exception);
        }
    }

[LukeTowers]

Did a quick look through the core, all SystemException uses right now are valid, they're mostly used for cases where either the system trips on something that it can't recover from or as a "developer validation" tool (i.e. perhaps it could recover from it, but the developer should really just follow the error message to fix whatever it is they're doing wrong).