Skip to content

Commit

Permalink
Avoid logging SystemExceptions twice
Browse files Browse the repository at this point in the history
SystemExceptions are already logged when the System ServiceProvider listens to the Message Logged event, this code used to be for ApplicationExceptions to bypass the fact that they were explicitly ignored by October's core exception handler.

ApplicationExceptions were added 27 Jan 2015 in octobercms/library@6a68036
ApplicationExceptions were explicitly ignored in the core exception handler on 16 Feb 2015 in octobercms/library@237d97d#diff-b6bf0348130fdd1311473a97536310cdR20 and were explicitly logged in the System exception handler on the same day in 7b52e07#diff-547f5794a63e9f138c33b20de8649b7eR20-R31

Not sure why that was originally the case, but we've made the decision that ApplicationExceptions shouldn't be logged by default as they should occur semi-regularly in a healthy application (mostly as an expression of complex logical validation that triggers them based on bad user input): #4569 (comment)

Fixes #5253.
  • Loading branch information
Luke Towers committed Aug 28, 2020
1 parent 6b214ed commit 812c055
Showing 1 changed file with 2 additions and 14 deletions.
16 changes: 2 additions & 14 deletions modules/system/classes/ErrorHandler.php
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
<?php namespace System\Classes;

use Log;
use View;
use Config;
use Cms\Classes\Theme;
use Cms\Classes\Router;
use Cms\Classes\Controller as CmsController;
use October\Rain\Exception\ErrorHandler as ErrorHandlerBase;
use October\Rain\Exception\SystemException;
use Symfony\Component\HttpFoundation\Response;

/**
* System Error Handler, this class handles application exception events.
Expand All @@ -34,18 +34,6 @@ class ErrorHandler extends ErrorHandlerBase
// return parent::handleException($proposedException);
// }

/**
* 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);
}
}

/**
* Looks up an error page using the CMS route "/error". If the route does not
* exist, this function will use the error view found in the Cms module.
Expand Down Expand Up @@ -74,7 +62,7 @@ public function handleCustomError()
}

// Extract content from response object
if ($result instanceof \Symfony\Component\HttpFoundation\Response) {
if ($result instanceof Response) {
$result = $result->getContent();
}

Expand Down

0 comments on commit 812c055

Please sign in to comment.