diff options
author | Matthieu Napoli <matthieu@mnapoli.fr> | 2014-12-04 07:34:51 +0300 |
---|---|---|
committer | Matthieu Napoli <matthieu@mnapoli.fr> | 2014-12-04 08:02:55 +0300 |
commit | 14cf2581242c75907fb554e08a30041ad107f558 (patch) | |
tree | 9ce5ab8e5547ef90fb238d8cc15250277fa410f5 /core | |
parent | ee6554e1d64bf4a4ecf05bfa513a35de8df3ef9e (diff) |
#6622 Logger refactoring: move to PSR-3 compatibility
- keep the log message as string
- exceptions are logged in context under the "exception" key
- Piwik\Error objects are replaced by \ErrorException (logged the PSR-3 way)
Diffstat (limited to 'core')
-rw-r--r-- | core/Error.php | 83 | ||||
-rw-r--r-- | core/ExceptionHandler.php | 10 | ||||
-rw-r--r-- | core/Log.php | 13 | ||||
-rw-r--r-- | core/Log/Formatter/ErrorHtmlFormatter.php | 35 | ||||
-rw-r--r-- | core/Log/Formatter/ErrorTextFormatter.php | 24 | ||||
-rw-r--r-- | core/Log/Formatter/ExceptionHtmlFormatter.php | 19 | ||||
-rw-r--r-- | core/Log/Formatter/ExceptionTextFormatter.php | 22 |
7 files changed, 87 insertions, 119 deletions
diff --git a/core/Error.php b/core/Error.php index 33b53132d9..7f5e740f38 100644 --- a/core/Error.php +++ b/core/Error.php @@ -11,72 +11,11 @@ namespace Piwik; require_once PIWIK_INCLUDE_PATH . '/core/Log.php'; /** - * Holds PHP error information (non-exception errors). Also contains log formatting logic - * for PHP errors and Piwik's error handler function. + * Piwik's error handler function. */ class Error { /** - * The backtrace string to use when testing. - * - * @var string - */ - public static $debugBacktraceForTests = null; - - /** - * The error number. See http://php.net/manual/en/errorfunc.constants.php#errorfunc.constants.errorlevels - * - * @var int - */ - public $errno; - - /** - * The error message. - * - * @var string - */ - public $errstr; - - /** - * The file in which the error occurred. - * - * @var string - */ - public $errfile; - - /** - * The line number on which the error occurred. - * - * @var int - */ - public $errline; - - /** - * The error backtrace. - * - * @var string - */ - public $backtrace; - - /** - * Constructor. - * - * @param int $errno - * @param string $errstr - * @param string $errfile - * @param int $errline - * @param string $backtrace - */ - public function __construct($errno, $errstr, $errfile, $errline, $backtrace) - { - $this->errno = $errno; - $this->errstr = $errstr; - $this->errfile = $errfile; - $this->errline = $errline; - $this->backtrace = $backtrace; - } - - /** * Returns a string description of a PHP error number. * * @param int $errno `E_ERROR`, `E_WARNING`, `E_PARSE`, etc. @@ -132,25 +71,7 @@ class Error return; } - $backtrace = ''; - if (empty(self::$debugBacktraceForTests)) { - $bt = @debug_backtrace(); - if ($bt !== null && isset($bt[0])) { - foreach ($bt as $i => $debug) { - $backtrace .= "#$i " - . (isset($debug['class']) ? $debug['class'] : '') - . (isset($debug['type']) ? $debug['type'] : '') - . (isset($debug['function']) ? $debug['function'] : '') - . '(...) called at [' - . (isset($debug['file']) ? $debug['file'] : '') . ':' - . (isset($debug['line']) ? $debug['line'] : '') . ']' . "\n"; - } - } - } else { - $backtrace = self::$debugBacktraceForTests; - } - - $error = new Error($errno, $errstr, $errfile, $errline, $backtrace); + $error = new \ErrorException($errstr, 0, $errno, $errfile, $errline); Log::error($error); switch ($errno) { diff --git a/core/ExceptionHandler.php b/core/ExceptionHandler.php index 9bbb5a23ac..8a10bc1e6b 100644 --- a/core/ExceptionHandler.php +++ b/core/ExceptionHandler.php @@ -9,18 +9,10 @@ namespace Piwik; /** - * Contains Piwik's uncaught exception handler and log file formatting for exception - * instances. + * Contains Piwik's uncaught exception handler. */ class ExceptionHandler { - /** - * The backtrace string to use when testing. - * - * @var string - */ - public static $debugBacktraceForTests = null; - public static function setUp() { set_exception_handler(array('Piwik\ExceptionHandler', 'logException')); diff --git a/core/Log.php b/core/Log.php index 3f73f5dc24..8bcf91df9c 100644 --- a/core/Log.php +++ b/core/Log.php @@ -117,6 +117,13 @@ class Log extends Singleton const GET_AVAILABLE_WRITERS_EVENT = 'Log.getAvailableWriters'; /** + * The backtrace string to use when testing. + * + * @var string + */ + public static $debugBacktraceForTests; + + /** * Singleton instance. * * @var Log @@ -261,6 +268,12 @@ class Log extends Singleton private function doLog($level, $message, $parameters = array()) { + // To ensure the compatibility with PSR-3, the message must be a string + if ($message instanceof \Exception) { + $parameters['exception'] = $message; + $message = $message->getMessage(); + } + // Create a record similar to Monolog to ease future transition $record = array( 'message' => $message, diff --git a/core/Log/Formatter/ErrorHtmlFormatter.php b/core/Log/Formatter/ErrorHtmlFormatter.php index 619e1e0630..da60fcd468 100644 --- a/core/Log/Formatter/ErrorHtmlFormatter.php +++ b/core/Log/Formatter/ErrorHtmlFormatter.php @@ -20,40 +20,41 @@ class ErrorHtmlFormatter extends Formatter { public function format(array $record) { - $message = $record['message']; - - if (! $message instanceof Error) { + if (! $this->contextContainsError($record)) { return $this->next($record); } - $errno = $message->errno & error_reporting(); - - // problem when using error_reporting with the @ silent fail operator - // it gives an errno 0, and in this case the objective is to NOT display anything on the screen! - // is there any other case where the errno is zero at this point? - // TODO: (@mnapoli) If `@` is used then the error handler will return, so I guess this step is redundant and useless... - if ($errno == 0) { - $message = false; - return $message; - } + /** @var \ErrorException $exception */ + $exception = $record['context']['exception']; Common::sendHeader('Content-Type: text/html; charset=utf-8'); + $trace = Log::$debugBacktraceForTests ?: $exception->getTraceAsString(); + $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!).<br /><br/> "; - $html .= Error::getErrNoString($message->errno); - $html .= ":</strong> <em>{$message->errstr}</em> in <strong>{$message->errfile}</strong>"; - $html .= " on line <strong>{$message->errline}</strong>\n"; + $html .= Error::getErrNoString($exception->getSeverity()); + $html .= ":</strong> <em>{$exception->getMessage()}</em> in <strong>{$exception->getFile()}</strong>"; + $html .= " on line <strong>{$exception->getLine()}</strong>\n"; $html .= "<br /><br />Backtrace --><div style=\"font-family:Courier;font-size:10pt\"><br />\n"; - $html .= str_replace("\n", "<br />\n", $message->backtrace); + $html .= str_replace("\n", "<br />\n", $trace); $html .= "</div><br />"; $html .= "\n </pre></div><br />"; $record['message'] = $html; + // Remove the exception so that it's not formatted again by another formatter + unset($record['context']['exception']); + return $record; } + + private function contextContainsError($record) + { + return isset($record['context']['exception']) + && $record['context']['exception'] instanceof \ErrorException; + } } diff --git a/core/Log/Formatter/ErrorTextFormatter.php b/core/Log/Formatter/ErrorTextFormatter.php index a4df5990e8..ac9a7021ba 100644 --- a/core/Log/Formatter/ErrorTextFormatter.php +++ b/core/Log/Formatter/ErrorTextFormatter.php @@ -18,13 +18,27 @@ class ErrorTextFormatter extends Formatter { public function format(array $record) { - $message = $record['message']; - - if ($message instanceof Error) { - $record['message'] = $message->errfile . '(' . $message->errline . '): ' . Error::getErrNoString($message->errno) - . ' - ' . $message->errstr . "\n" . $message->backtrace; + if (! $this->contextContainsError($record)) { + return $this->next($record); } + /** @var \ErrorException $exception */ + $exception = $record['context']['exception']; + + $trace = Log::$debugBacktraceForTests ?: $exception->getTraceAsString(); + + $record['message'] = $exception->getFile() . '(' . $exception->getLine() . '): ' . Error::getErrNoString($exception->getSeverity()) + . ' - ' . $exception->getMessage() . "\n" . $trace; + + // Remove the exception so that it's not formatted again by another formatter + unset($record['context']['exception']); + return $this->next($record); } + + private function contextContainsError($record) + { + return isset($record['context']['exception']) + && $record['context']['exception'] instanceof \ErrorException; + } } diff --git a/core/Log/Formatter/ExceptionHtmlFormatter.php b/core/Log/Formatter/ExceptionHtmlFormatter.php index 70549dbd7e..bb7994bd36 100644 --- a/core/Log/Formatter/ExceptionHtmlFormatter.php +++ b/core/Log/Formatter/ExceptionHtmlFormatter.php @@ -19,7 +19,7 @@ class ExceptionHtmlFormatter extends Formatter { public function format(array $record) { - if (! $record['message'] instanceof \Exception) { + if (! $this->contextContainsException($record)) { return $this->next($record); } @@ -28,8 +28,23 @@ class ExceptionHtmlFormatter extends Formatter $outputFormat = strtolower(Common::getRequestVar('format', 'html', 'string')); $response = new ResponseBuilder($outputFormat); - $record['message'] = $response->getResponseException(new \Exception($record['message']->getMessage())); + /** @var \Exception $exception */ + $exception = $record['context']['exception']; + + // Why do we create a new exception and not use the real one?? + $exception = new \Exception($exception->getMessage()); + + $record['message'] = $response->getResponseException($exception); + + // Remove the exception so that it's not formatted again by another formatter + unset($record['context']['exception']); return $record; } + + private function contextContainsException($record) + { + return isset($record['context']['exception']) + && $record['context']['exception'] instanceof \Exception; + } } diff --git a/core/Log/Formatter/ExceptionTextFormatter.php b/core/Log/Formatter/ExceptionTextFormatter.php index 62b850f443..0b7df2fd7c 100644 --- a/core/Log/Formatter/ExceptionTextFormatter.php +++ b/core/Log/Formatter/ExceptionTextFormatter.php @@ -18,13 +18,25 @@ class ExceptionTextFormatter extends Formatter { public function format(array $record) { - $message = $record['message']; - - if ($message instanceof \Exception) { - $record['message'] = sprintf("%s(%d): %s\n%s", $message->getFile(), $message->getLine(), $message->getMessage(), - ExceptionHandler::$debugBacktraceForTests ?: $message->getTraceAsString()); + if (! $this->contextContainsException($record)) { + return $this->next($record); } + /** @var \Exception $exception */ + $exception = $record['context']['exception']; + + $record['message'] = sprintf("%s(%d): %s\n%s", $exception->getFile(), $exception->getLine(), $exception->getMessage(), + Log::$debugBacktraceForTests ?: $exception->getTraceAsString()); + + // Remove the exception so that it's not formatted again by another formatter + unset($record['context']['exception']); + return $this->next($record); } + + private function contextContainsException($record) + { + return isset($record['context']['exception']) + && $record['context']['exception'] instanceof \Exception; + } } |