#6622 Logger refactoring: Separated error and exception handling from logging

- the error handler logs warnings and notices, and turns errors into exceptions (`ErrorException`)
- the exception handler catches all uncatched exceptions and display them to the user (HTML or CLI)
- the "screen" logging backend has been removed
- I've normalized exceptions/errors shown to the user in HTML (wether they are catched by the FrontController or not)

9 files changed, 143 insertions, 243 deletions

diff --git a/core/Error.php b/core/Error.php
index 7f5e740f38..0dd346244f 100644
--- a/core/Error.php
+++ b/core/Error.php
@@ -4,11 +4,11 @@
  *
  * @link http://piwik.org
  * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
- *
  */
+
 namespace Piwik;
 
-require_once PIWIK_INCLUDE_PATH . '/core/Log.php';
+use Piwik\Exception\ErrorException;
 
 /**
  * Piwik's error handler function.
@@ -71,9 +71,6 @@ class Error
             return;
         }
 
-        $error = new \ErrorException($errstr, 0, $errno, $errfile, $errline);
-        Log::error($error);
-
         switch ($errno) {
             case E_ERROR:
             case E_PARSE:
@@ -82,7 +79,10 @@ class Error
             case E_COMPILE_ERROR:
             case E_COMPILE_WARNING:
             case E_USER_ERROR:
-                exit;
+                // Convert the error to an exception with an HTML message
+                $e = new \Exception();
+                $message = self::getHtmlMessage($errno, $errstr, $errfile, $errline, $e->getTraceAsString());
+                throw new ErrorException($message, 0, $errno, $errfile, $errline);
                 break;
 
             case E_WARNING:
@@ -94,8 +94,37 @@ class Error
             case E_DEPRECATED:
             case E_USER_DEPRECATED:
             default:
-                // do not exit
+                Log::info(self::createLogMessage($errno, $errstr, $errfile, $errline));
                 break;
         }
     }
+
+    private static function createLogMessage($errno, $errstr, $errfile, $errline)
+    {
+        return sprintf(
+            "%s(%d): %s - %s",
+            $errfile,
+            $errline,
+            Error::getErrNoString($errno),
+            $errstr
+        );
+    }
+
+    private static function getHtmlMessage($errno, $errstr, $errfile, $errline, $trace)
+    {
+        $trace = Log::$debugBacktraceForTests ?: $trace;
+
+        $message = Error::getErrNoString($errno) . ' - ' . $errstr;
+
+        $html = "<strong>There is an error. Please report the message (Piwik " . (class_exists('Piwik\Version') ? Version::VERSION : '') . ")
+        and full backtrace in the <a href='?module=Proxy&action=redirect&url=http://forum.piwik.org' target='_blank'>Piwik forums</a> (please do a Search first as it might have been reported already!).</strong><br /><br/>
+        ";
+        $html .= "<strong>{$message}</strong> in <em>{$errfile}</em>";
+        $html .= " on line {$errline}<br />";
+        $html .= "<br />Backtrace:<div style=\"font-family:Courier;font-size:10pt\"><br />\n";
+        $html .= str_replace("\n", "<br />\n", $trace);
+        $html .= "</div>";
+
+        return $html;
+    }
 }
diff --git a/core/Exception/ErrorException.php b/core/Exception/ErrorException.php
new file mode 100644
index 0000000000..1673d8def8
--- /dev/null
+++ b/core/Exception/ErrorException.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace Piwik\Exception;
+
+/**
+ * ErrorException
+ *
+ * @author Matthieu Napoli <matthieu@mnapoli.fr>
+ */
+class ErrorException extends \ErrorException
+{
+    public function isHtmlMessage()
+    {
+        return true;
+    }
+}
diff --git a/core/ExceptionHandler.php b/core/ExceptionHandler.php
index 8a10bc1e6b..e68a058e4a 100644
--- a/core/ExceptionHandler.php
+++ b/core/ExceptionHandler.php
@@ -8,6 +8,9 @@
  */
 namespace Piwik;
 
+use Exception;
+use Piwik\Plugins\CoreAdminHome\CustomLogo;
+
 /**
  * Contains Piwik's uncaught exception handler.
  */
@@ -15,11 +18,82 @@ class ExceptionHandler
 {
     public static function setUp()
     {
-        set_exception_handler(array('Piwik\ExceptionHandler', 'logException'));
+        set_exception_handler(array('Piwik\ExceptionHandler', 'handleException'));
+    }
+
+    public static function handleException(Exception $exception)
+    {
+        if (Common::isPhpCliMode()) {
+            self::dieWithCliError($exception);
+        }
+
+        self::dieWithHtmlErrorPage($exception);
+    }
+
+    public static function dieWithCliError(Exception $exception)
+    {
+        $message = $exception->getMessage();
+
+        if (!method_exists($exception, 'isHtmlMessage') || !$exception->isHtmlMessage()) {
+            $message = strip_tags(str_replace('<br />', PHP_EOL, $message));
+        }
+
+        $message = sprintf(
+            "Uncaught exception: %s\nin %s line %d\n%s\n",
+            $message,
+            $exception->getFile(),
+            $exception->getLine(),
+            $exception->getTraceAsString()
+        );
+
+        echo $message;
+
+        exit(1);
+    }
+
+    public static function dieWithHtmlErrorPage(Exception $exception)
+    {
+        Common::sendHeader('Content-Type: text/html; char1set=utf-8');
+
+        echo self::getErrorResponse($exception);
+
+        exit(1);
     }
 
-    public static function logException(\Exception $exception)
+    private static function getErrorResponse(Exception $ex)
     {
-        Log::error($exception);
+        $debugTrace = $ex->getTraceAsString();
+
+        $message = $ex->getMessage();
+
+        if (!method_exists($ex, 'isHtmlMessage') || !$ex->isHtmlMessage()) {
+            $message = Common::sanitizeInputValue($message);
+        }
+
+        $logo = new CustomLogo();
+
+        $logoHeaderUrl = false;
+        $logoFaviconUrl = false;
+        try {
+            $logoHeaderUrl = $logo->getHeaderLogoUrl();
+            $logoFaviconUrl = $logo->getPathUserFavicon();
+        } catch (Exception $ex) {
+            Log::debug($ex);
+        }
+
+        $result = Piwik_GetErrorMessagePage($message, $debugTrace, true, true, $logoHeaderUrl, $logoFaviconUrl);
+
+        /**
+         * Triggered before a Piwik error page is displayed to the user.
+         *
+         * This event can be used to modify the content of the error page that is displayed when
+         * an exception is caught.
+         *
+         * @param string &$result The HTML of the error page.
+         * @param Exception $ex The Exception displayed in the error page.
+         */
+        Piwik::postEvent('FrontController.modifyErrorPage', array(&$result, $ex));
+
+        return $result;
     }
 }
diff --git a/core/FrontController.php b/core/FrontController.php
index 7d1702315f..d536ab22a7 100644
--- a/core/FrontController.php
+++ b/core/FrontController.php
@@ -18,7 +18,6 @@ use Piwik\Http\Router;
 use Piwik\Plugin\Controller;
 use Piwik\Plugin\Report;
 use Piwik\Plugin\Widgets;
-use Piwik\Plugins\CoreAdminHome\CustomLogo;
 use Piwik\Session;
 
 /**
@@ -610,48 +609,4 @@ class FrontController extends Singleton
         Piwik::postEvent('Request.dispatch.end', array(&$result, $module, $action, $parameters));
         return $result;
     }
-
-    /**
-     * Returns HTML that displays an exception's error message (and possibly stack trace).
-     * The result of this method is echo'd by dispatch.php.
-     *
-     * @param Exception $ex The exception to use when generating the error page's HTML.
-     * @return string The HTML to echo.
-     */
-    public function getErrorResponse(Exception $ex)
-    {
-        $debugTrace = $ex->getTraceAsString();
-
-        $message = $ex->getMessage();
-
-        if (!method_exists($ex, 'isHtmlMessage') || !$ex->isHtmlMessage()) {
-            $message = Common::sanitizeInputValue($message);
-        }
-
-        $logo = new CustomLogo();
-
-        $logoHeaderUrl = false;
-        $logoFaviconUrl = false;
-        try {
-            $logoHeaderUrl = $logo->getHeaderLogoUrl();
-            $logoFaviconUrl = $logo->getPathUserFavicon();
-        } catch (Exception $ex) {
-            Log::debug($ex);
-        }
-
-        $result = Piwik_GetErrorMessagePage($message, $debugTrace, true, true, $logoHeaderUrl, $logoFaviconUrl);
-
-        /**
-         * Triggered before a Piwik error page is displayed to the user.
-         *
-         * This event can be used to modify the content of the error page that is displayed when
-         * an exception is caught.
-         *
-         * @param string &$result The HTML of the error page.
-         * @param Exception $ex The Exception displayed in the error page.
-         */
-        Piwik::postEvent('FrontController.modifyErrorPage', array(&$result, $ex));
-
-        return $result;
-    }
 }
diff --git a/core/Log/Formatter/ExceptionHtmlFormatter.php b/core/Log/Formatter/ExceptionHtmlFormatter.php
deleted file mode 100644
index d94190b089..0000000000
--- a/core/Log/Formatter/ExceptionHtmlFormatter.php
+++ /dev/null
@@ -1,65 +0,0 @@
-<?php
-/**
- * Piwik - free/libre analytics platform
- *
- * @link http://piwik.org
- * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
- */
-
-namespace Piwik\Log\Formatter;
-
-use Piwik\Common;
-use Piwik\Error;
-use Piwik\Log;
-use Piwik\Version;
-
-/**
- * Formats a log message containing an exception object into an HTML response.
- */
-class ExceptionHtmlFormatter extends Formatter
-{
-    public function format(array $record)
-    {
-        if (! $this->contextContainsException($record)) {
-            return $this->next($record);
-        }
-
-        /** @var \Exception $exception */
-        $exception = $record['context']['exception'];
-
-        Common::sendHeader('Content-Type: text/html; charset=utf-8');
-
-        $trace = Log::$debugBacktraceForTests ?: $exception->getTraceAsString();
-
-        $message = $this->getMessage($exception);
-
-        $html = '';
-        $html .= "<div style='word-wrap: break-word; border: 3px solid red; padding:4px; width:70%; background-color:#FFFF96;'>
-        <strong>There is an error. Please report the message (Piwik " . (class_exists('Piwik\Version') ? Version::VERSION : '') . ")
-        and full backtrace in the <a href='?module=Proxy&action=redirect&url=http://forum.piwik.org' target='_blank'>Piwik forums</a> (please do a Search first as it might have been reported already!).</strong><br /><br/>
-        ";
-        $html .= "<em>{$message}</em> in <strong>{$exception->getFile()}</strong>";
-        $html .= " on line <strong>{$exception->getLine()}</strong>\n";
-        $html .= "<br /><br />Backtrace --&gt;<div style=\"font-family:Courier;font-size:10pt\"><br />\n";
-        $html .= str_replace("\n", "<br />\n", $trace);
-        $html .= "</div>";
-        $html .= "</div>\n";
-
-        return $html;
-    }
-
-    private function contextContainsException($record)
-    {
-        return isset($record['context']['exception'])
-            && $record['context']['exception'] instanceof \Exception;
-    }
-
-    private function getMessage(\Exception $exception)
-    {
-        if ($exception instanceof \ErrorException) {
-            return Error::getErrNoString($exception->getSeverity()) . ' - ' . $exception->getMessage();
-        }
-
-        return $exception->getMessage();
-    }
-}
diff --git a/core/Log/Formatter/Formatter.php b/core/Log/Formatter/Formatter.php
deleted file mode 100644
index 6f5ac67e64..0000000000
--- a/core/Log/Formatter/Formatter.php
+++ /dev/null
@@ -1,60 +0,0 @@
-<?php
-/**
- * Piwik - free/libre analytics platform
- *
- * @link http://piwik.org
- * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
- */
-
-namespace Piwik\Log\Formatter;
-
-use Monolog\Formatter\FormatterInterface;
-
-/**
- * Formats a log message.
- *
- * Follows the Chain of responsibility design pattern.
- */
-abstract class Formatter implements FormatterInterface
-{
-    /**
-     * @var Formatter|null
-     */
-    protected $next;
-
-    public function __construct(Formatter $next = null)
-    {
-        $this->next = $next;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function formatBatch(array $records)
-    {
-        foreach ($records as $key => $record) {
-            $records[$key] = $this->format($record);
-        }
-
-        return $records;
-    }
-
-    /**
-     * Chain of responsibility pattern.
-     *
-     * @param Formatter $formatter
-     */
-    public function setNext(Formatter $formatter)
-    {
-        $this->next = $formatter;
-    }
-
-    protected function next(array $record)
-    {
-        if (! $this->next) {
-            throw new \RuntimeException('No next formatter to call');
-        }
-
-        return $this->next->format($record);
-    }
-}
diff --git a/core/Log/Formatter/LineMessageFormatter.php b/core/Log/Formatter/LineMessageFormatter.php
index 08f368dfc5..7c38fc32df 100644
--- a/core/Log/Formatter/LineMessageFormatter.php
+++ b/core/Log/Formatter/LineMessageFormatter.php
@@ -8,10 +8,12 @@
 
 namespace Piwik\Log\Formatter;
 
+use Monolog\Formatter\FormatterInterface;
+
 /**
- * Formats a log message into a line of text.
+ * Formats a log message into a line of text using our custom Piwik log format.
  */
-class LineMessageFormatter extends Formatter
+class LineMessageFormatter implements FormatterInterface
 {
     /**
      * The log message format string that turns a tag name, date-time and message into
@@ -49,6 +51,15 @@ class LineMessageFormatter extends Formatter
         return $message;
     }
 
+    public function formatBatch(array $records)
+    {
+        foreach ($records as $key => $record) {
+            $records[$key] = $this->format($record);
+        }
+
+        return $records;
+    }
+
     private function prefixMessageWithRequestId(array $record)
     {
         $requestId = isset($record['extra']['request_id']) ? $record['extra']['request_id'] : '';
diff --git a/core/Log/Handler/StdErrHandler.php b/core/Log/Handler/StdErrHandler.php
deleted file mode 100644
index 6ab856b66f..0000000000
--- a/core/Log/Handler/StdErrHandler.php
+++ /dev/null
@@ -1,56 +0,0 @@
-<?php
-/**
- * Piwik - free/libre analytics platform
- *
- * @link http://piwik.org
- * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
- */
-
-namespace Piwik\Log\Handler;
-
-use Monolog\Formatter\FormatterInterface;
-use Monolog\Handler\AbstractProcessingHandler;
-use Monolog\Logger;
-
-/**
- * Writes log to stderr.
- */
-class StdErrHandler extends AbstractProcessingHandler
-{
-    /**
-     * @var bool
-     */
-    private $isLoggingToStdOut;
-
-    public function __construct(FormatterInterface $formatter, $isLoggingToStdOut)
-    {
-        $this->isLoggingToStdOut = $isLoggingToStdOut;
-
-        // Log level is hardcoded for this one
-        $level = Logger::ERROR;
-
-        parent::__construct($level);
-
-        $this->setFormatter($formatter);
-    }
-
-    protected function write(array $record)
-    {
-        // Do not log on stderr during tests (prevent display of errors in CI output)
-        if (! defined('PIWIK_TEST_MODE')) {
-            $this->writeToStdErr($record['formatted']);
-        }
-
-        // This is the result of an old hack, I guess to force the error message to be displayed in case of errors
-        // TODO we should get rid of it somehow
-        if (! $this->isLoggingToStdOut) {
-            echo $record['formatted'];
-        }
-    }
-
-    private function writeToStdErr($message)
-    {
-        $fe = fopen('php://stderr', 'w');
-        fwrite($fe, $message);
-    }
-}
diff --git a/core/dispatch.php b/core/dispatch.php
index 86b810500a..d842b6a727 100644
--- a/core/dispatch.php
+++ b/core/dispatch.php
@@ -39,10 +39,6 @@ if (PIWIK_ENABLE_DISPATCH) {
             echo $response;
         }
     } catch (Exception $ex) {
-        $response = $controller->getErrorResponse($ex);
-
-        echo $response;
-
-        exit(1);
+        ExceptionHandler::dieWithHtmlErrorPage($ex);
     }
 }
\ No newline at end of file