diff --git a/composer.json b/composer.json index df4f57b2..6efd2819 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "require-dev": { "phpunit/phpunit": "^5.0", "graylog2/gelf-php": "~1.0", - "raven/raven": "^0.13", + "sentry/sentry": "^0.13", "ruflin/elastica": ">=0.90 <3.0", "doctrine/couchdb": "~1.0@dev", "aws/aws-sdk-php": "^2.4.9", @@ -30,7 +30,7 @@ }, "suggest": { "graylog2/gelf-php": "Allow sending log messages to a GrayLog2 server", - "raven/raven": "Allow sending log messages to a Sentry server", + "sentry/sentry": "Allow sending log messages to a Sentry server", "doctrine/couchdb": "Allow sending log messages to a CouchDB server", "ruflin/elastica": "Allow sending log messages to an Elastic Search server", "php-amqplib/php-amqplib": "Allow sending log messages to an AMQP server using php-amqplib", diff --git a/src/Monolog/Formatter/GelfMessageFormatter.php b/src/Monolog/Formatter/GelfMessageFormatter.php index b4de509d..64e76652 100644 --- a/src/Monolog/Formatter/GelfMessageFormatter.php +++ b/src/Monolog/Formatter/GelfMessageFormatter.php @@ -106,7 +106,7 @@ class GelfMessageFormatter extends NormalizerFormatter } foreach ($record['extra'] as $key => $val) { - $val = is_scalar($val) ? $val : $this->toJson($val); + $val = is_scalar($val) || null === $val ? $val : $this->toJson($val); $len += strlen($this->extraPrefix . $key . $val); if ($len > self::MAX_LENGTH) { $message->setAdditional($this->extraPrefix . $key, substr($val, 0, self::MAX_LENGTH - $len)); @@ -116,7 +116,7 @@ class GelfMessageFormatter extends NormalizerFormatter } foreach ($record['context'] as $key => $val) { - $val = is_scalar($val) ? $val : $this->toJson($val); + $val = is_scalar($val) || null === $val ? $val : $this->toJson($val); $len += strlen($this->contextPrefix . $key . $val); if ($len > self::MAX_LENGTH) { $message->setAdditional($this->contextPrefix . $key, substr($val, 0, self::MAX_LENGTH - $len)); diff --git a/src/Monolog/Formatter/JsonFormatter.php b/src/Monolog/Formatter/JsonFormatter.php index 06e9d13d..a985e2ab 100644 --- a/src/Monolog/Formatter/JsonFormatter.php +++ b/src/Monolog/Formatter/JsonFormatter.php @@ -186,6 +186,9 @@ class JsonFormatter extends NormalizerFormatter foreach ($trace as $frame) { if (isset($frame['file'])) { $data['trace'][] = $frame['file'].':'.$frame['line']; + } elseif (isset($frame['function']) && $frame['function'] === '{closure}') { + // We should again normalize the frames, because it might contain invalid items + $data['trace'][] = $frame['function']; } else { // We should again normalize the frames, because it might contain invalid items $data['trace'][] = $this->normalize($frame); diff --git a/src/Monolog/Formatter/NormalizerFormatter.php b/src/Monolog/Formatter/NormalizerFormatter.php index 0f2844be..f4cdf460 100644 --- a/src/Monolog/Formatter/NormalizerFormatter.php +++ b/src/Monolog/Formatter/NormalizerFormatter.php @@ -129,6 +129,9 @@ class NormalizerFormatter implements FormatterInterface foreach ($trace as $frame) { if (isset($frame['file'])) { $data['trace'][] = $frame['file'].':'.$frame['line']; + } elseif (isset($frame['function']) && $frame['function'] === '{closure}') { + // We should again normalize the frames, because it might contain invalid items + $data['trace'][] = $frame['function']; } else { // We should again normalize the frames, because it might contain invalid items $data['trace'][] = $this->toJson($this->normalize($frame), true); diff --git a/src/Monolog/Handler/FingersCrossedHandler.php b/src/Monolog/Handler/FingersCrossedHandler.php index d1c246be..dd7ddfb4 100644 --- a/src/Monolog/Handler/FingersCrossedHandler.php +++ b/src/Monolog/Handler/FingersCrossedHandler.php @@ -81,6 +81,26 @@ class FingersCrossedHandler extends Handler implements ProcessableHandlerInterfa return true; } + /** + * Manually activate this logger regardless of the activation strategy + */ + public function activate() + { + if ($this->stopBuffering) { + $this->buffering = false; + } + if (!$this->handler instanceof HandlerInterface) { + $record = end($this->buffer) ?: null; + + $this->handler = call_user_func($this->handler, $record, $this); + if (!$this->handler instanceof HandlerInterface) { + throw new \RuntimeException("The factory callable should return a HandlerInterface"); + } + } + $this->handler->handleBatch($this->buffer); + $this->buffer = array(); + } + /** * {@inheritdoc} */ @@ -96,17 +116,7 @@ class FingersCrossedHandler extends Handler implements ProcessableHandlerInterfa array_shift($this->buffer); } if ($this->activationStrategy->isHandlerActivated($record)) { - if ($this->stopBuffering) { - $this->buffering = false; - } - if (!$this->handler instanceof HandlerInterface) { - $this->handler = call_user_func($this->handler, $record, $this); - if (!$this->handler instanceof HandlerInterface) { - throw new \RuntimeException("The factory callable should return a HandlerInterface"); - } - } - $this->handler->handleBatch($this->buffer); - $this->buffer = array(); + $this->activate(); } } else { $this->handler->handle($record); diff --git a/src/Monolog/Handler/NewRelicHandler.php b/src/Monolog/Handler/NewRelicHandler.php index e23078bf..2b27a16f 100644 --- a/src/Monolog/Handler/NewRelicHandler.php +++ b/src/Monolog/Handler/NewRelicHandler.php @@ -92,23 +92,27 @@ class NewRelicHandler extends AbstractProcessingHandler newrelic_notice_error($record['message']); } - foreach ($record['formatted']['context'] as $key => $parameter) { - if (is_array($parameter) && $this->explodeArrays) { - foreach ($parameter as $paramKey => $paramValue) { - $this->setNewRelicParameter('context_' . $key . '_' . $paramKey, $paramValue); + if (isset($record['formatted']['context'])) { + foreach ($record['formatted']['context'] as $key => $parameter) { + if (is_array($parameter) && $this->explodeArrays) { + foreach ($parameter as $paramKey => $paramValue) { + $this->setNewRelicParameter('context_' . $key . '_' . $paramKey, $paramValue); + } + } else { + $this->setNewRelicParameter('context_' . $key, $parameter); } - } else { - $this->setNewRelicParameter('context_' . $key, $parameter); } } - foreach ($record['formatted']['extra'] as $key => $parameter) { - if (is_array($parameter) && $this->explodeArrays) { - foreach ($parameter as $paramKey => $paramValue) { - $this->setNewRelicParameter('extra_' . $key . '_' . $paramKey, $paramValue); + if (isset($record['formatted']['extra'])) { + foreach ($record['formatted']['extra'] as $key => $parameter) { + if (is_array($parameter) && $this->explodeArrays) { + foreach ($parameter as $paramKey => $paramValue) { + $this->setNewRelicParameter('extra_' . $key . '_' . $paramKey, $paramValue); + } + } else { + $this->setNewRelicParameter('extra_' . $key, $parameter); } - } else { - $this->setNewRelicParameter('extra_' . $key, $parameter); } } } diff --git a/src/Monolog/Handler/RotatingFileHandler.php b/src/Monolog/Handler/RotatingFileHandler.php index 22e6c3dc..cd938057 100644 --- a/src/Monolog/Handler/RotatingFileHandler.php +++ b/src/Monolog/Handler/RotatingFileHandler.php @@ -24,6 +24,10 @@ use Monolog\Logger; */ class RotatingFileHandler extends StreamHandler { + const FILE_PER_DAY = 'Y-m-d'; + const FILE_PER_MONTH = 'Y-m'; + const FILE_PER_YEAR = 'Y'; + protected $filename; protected $maxFiles; protected $mustRotate; @@ -64,6 +68,20 @@ class RotatingFileHandler extends StreamHandler public function setFilenameFormat($filenameFormat, $dateFormat) { + if (!in_array($dateFormat, array(self::FILE_PER_DAY, self::FILE_PER_MONTH, self::FILE_PER_YEAR))) { + trigger_error( + 'Invalid date format - format should be one of '. + 'RotatingFileHandler::FILE_PER_DAY, RotatingFileHandler::FILE_PER_MONTH '. + 'or RotatingFileHandler::FILE_PER_YEAR.', + E_USER_DEPRECATED + ); + } + if (substr_count($filenameFormat, '{date}') === 0) { + trigger_error( + 'Invalid filename format - format should contain at least `{date}`, because otherwise rotating is impossible.', + E_USER_DEPRECATED + ); + } $this->filenameFormat = $filenameFormat; $this->dateFormat = $dateFormat; $this->url = $this->getTimedFilename(); diff --git a/src/Monolog/Handler/WhatFailureGroupHandler.php b/src/Monolog/Handler/WhatFailureGroupHandler.php index e5533a6e..db793869 100644 --- a/src/Monolog/Handler/WhatFailureGroupHandler.php +++ b/src/Monolog/Handler/WhatFailureGroupHandler.php @@ -35,6 +35,8 @@ class WhatFailureGroupHandler extends GroupHandler $handler->handle($record); } catch (\Exception $e) { // What failure? + } catch (\Throwable $e) { + // What failure? } } @@ -51,6 +53,8 @@ class WhatFailureGroupHandler extends GroupHandler $handler->handleBatch($records); } catch (\Exception $e) { // What failure? + } catch (\Throwable $e) { + // What failure? } } } diff --git a/tests/Monolog/Handler/FingersCrossedHandlerTest.php b/tests/Monolog/Handler/FingersCrossedHandlerTest.php index 8e31e9b8..b92bf437 100644 --- a/tests/Monolog/Handler/FingersCrossedHandlerTest.php +++ b/tests/Monolog/Handler/FingersCrossedHandlerTest.php @@ -22,6 +22,7 @@ class FingersCrossedHandlerTest extends TestCase /** * @covers Monolog\Handler\FingersCrossedHandler::__construct * @covers Monolog\Handler\FingersCrossedHandler::handle + * @covers Monolog\Handler\FingersCrossedHandler::activate */ public function testHandleBuffers() { @@ -39,6 +40,7 @@ class FingersCrossedHandlerTest extends TestCase /** * @covers Monolog\Handler\FingersCrossedHandler::handle + * @covers Monolog\Handler\FingersCrossedHandler::activate */ public function testHandleStopsBufferingAfterTrigger() { @@ -53,6 +55,7 @@ class FingersCrossedHandlerTest extends TestCase /** * @covers Monolog\Handler\FingersCrossedHandler::handle + * @covers Monolog\Handler\FingersCrossedHandler::activate * @covers Monolog\Handler\FingersCrossedHandler::reset */ public function testHandleRestartBufferingAfterReset() @@ -71,6 +74,7 @@ class FingersCrossedHandlerTest extends TestCase /** * @covers Monolog\Handler\FingersCrossedHandler::handle + * @covers Monolog\Handler\FingersCrossedHandler::activate */ public function testHandleRestartBufferingAfterBeingTriggeredWhenStopBufferingIsDisabled() { @@ -87,6 +91,7 @@ class FingersCrossedHandlerTest extends TestCase /** * @covers Monolog\Handler\FingersCrossedHandler::handle + * @covers Monolog\Handler\FingersCrossedHandler::activate */ public function testHandleBufferLimit() { @@ -103,6 +108,7 @@ class FingersCrossedHandlerTest extends TestCase /** * @covers Monolog\Handler\FingersCrossedHandler::handle + * @covers Monolog\Handler\FingersCrossedHandler::activate */ public function testHandleWithCallback() { @@ -121,6 +127,7 @@ class FingersCrossedHandlerTest extends TestCase /** * @covers Monolog\Handler\FingersCrossedHandler::handle + * @covers Monolog\Handler\FingersCrossedHandler::activate * @expectedException RuntimeException */ public function testHandleWithBadCallbackThrowsException() @@ -173,6 +180,22 @@ class FingersCrossedHandlerTest extends TestCase $this->assertTrue($test->hasWarningRecords()); } + /** + * @covers Monolog\Handler\FingersCrossedHandler::__construct + * @covers Monolog\Handler\FingersCrossedHandler::activate + */ + public function testOverrideActivationStrategy() + { + $test = new TestHandler(); + $handler = new FingersCrossedHandler($test, new ErrorLevelActivationStrategy('warning')); + $handler->handle($this->getRecord(Logger::DEBUG)); + $this->assertFalse($test->hasDebugRecords()); + $handler->activate(); + $this->assertTrue($test->hasDebugRecords()); + $handler->handle($this->getRecord(Logger::INFO)); + $this->assertTrue($test->hasInfoRecords()); + } + /** * @covers Monolog\Handler\FingersCrossed\ChannelLevelActivationStrategy::__construct * @covers Monolog\Handler\FingersCrossed\ChannelLevelActivationStrategy::isHandlerActivated @@ -209,6 +232,7 @@ class FingersCrossedHandlerTest extends TestCase /** * @covers Monolog\Handler\FingersCrossedHandler::handle + * @covers Monolog\Handler\FingersCrossedHandler::activate */ public function testHandleUsesProcessors() { diff --git a/tests/Monolog/Handler/NewRelicHandlerTest.php b/tests/Monolog/Handler/NewRelicHandlerTest.php index 4eda6155..4d3a615f 100644 --- a/tests/Monolog/Handler/NewRelicHandlerTest.php +++ b/tests/Monolog/Handler/NewRelicHandlerTest.php @@ -11,6 +11,7 @@ namespace Monolog\Handler; +use Monolog\Formatter\LineFormatter; use Monolog\TestCase; use Monolog\Logger; @@ -104,6 +105,13 @@ class NewRelicHandlerTest extends TestCase $this->assertEquals($expected, self::$customParameters); } + public function testThehandlerCanHandleTheRecordsFormattedUsingTheLineFormatter() + { + $handler = new StubNewRelicHandler(); + $handler->setFormatter(new LineFormatter()); + $handler->handle($this->getRecord(Logger::ERROR)); + } + public function testTheAppNameIsNullByDefault() { $handler = new StubNewRelicHandler(); diff --git a/tests/Monolog/Handler/RotatingFileHandlerTest.php b/tests/Monolog/Handler/RotatingFileHandlerTest.php index f4cefda1..96e6dff4 100644 --- a/tests/Monolog/Handler/RotatingFileHandlerTest.php +++ b/tests/Monolog/Handler/RotatingFileHandlerTest.php @@ -12,19 +12,52 @@ namespace Monolog\Handler; use Monolog\TestCase; +use PHPUnit_Framework_Error_Deprecated; /** * @covers Monolog\Handler\RotatingFileHandler */ class RotatingFileHandlerTest extends TestCase { + /** + * This var should be private but then the anonymous function + * in the `setUp` method won't be able to set it. `$this` cant't + * be used in the anonymous function in `setUp` because PHP 5.3 + * does not support it. + */ + public $lastError; + public function setUp() { $dir = __DIR__.'/Fixtures'; chmod($dir, 0777); if (!is_writable($dir)) { - $this->markTestSkipped($dir.' must be writeable to test the RotatingFileHandler.'); + $this->markTestSkipped($dir.' must be writable to test the RotatingFileHandler.'); } + $this->lastError = null; + $self = $this; + // workaround with &$self used for PHP 5.3 + set_error_handler(function($code, $message) use (&$self) { + $self->lastError = array( + 'code' => $code, + 'message' => $message, + ); + }); + } + + private function assertErrorWasTriggered($code, $message) + { + if (empty($this->lastError)) { + $this->fail( + sprintf( + 'Failed asserting that error with code `%d` and message `%s` was triggered', + $code, + $message + ) + ); + } + $this->assertEquals($code, $this->lastError['code'], sprintf('Expected an error with code %d to be triggered, got `%s` instead', $code, $this->lastError['code'])); + $this->assertEquals($message, $this->lastError['message'], sprintf('Expected an error with message `%d` to be triggered, got `%s` instead', $message, $this->lastError['message'])); } public function testRotationCreatesNewFile() @@ -43,14 +76,14 @@ class RotatingFileHandlerTest extends TestCase /** * @dataProvider rotationTests */ - public function testRotation($createFile) + public function testRotation($createFile, $dateFormat, $timeCallback) { - touch($old1 = __DIR__.'/Fixtures/foo-'.date('Y-m-d', time() - 86400).'.rot'); - touch($old2 = __DIR__.'/Fixtures/foo-'.date('Y-m-d', time() - 86400 * 2).'.rot'); - touch($old3 = __DIR__.'/Fixtures/foo-'.date('Y-m-d', time() - 86400 * 3).'.rot'); - touch($old4 = __DIR__.'/Fixtures/foo-'.date('Y-m-d', time() - 86400 * 4).'.rot'); + touch($old1 = __DIR__.'/Fixtures/foo-'.date($dateFormat, $timeCallback(-1)).'.rot'); + touch($old2 = __DIR__.'/Fixtures/foo-'.date($dateFormat, $timeCallback(-2)).'.rot'); + touch($old3 = __DIR__.'/Fixtures/foo-'.date($dateFormat, $timeCallback(-3)).'.rot'); + touch($old4 = __DIR__.'/Fixtures/foo-'.date($dateFormat, $timeCallback(-4)).'.rot'); - $log = __DIR__.'/Fixtures/foo-'.date('Y-m-d').'.rot'; + $log = __DIR__.'/Fixtures/foo-'.date($dateFormat).'.rot'; if ($createFile) { touch($log); @@ -58,6 +91,7 @@ class RotatingFileHandlerTest extends TestCase $handler = new RotatingFileHandler(__DIR__.'/Fixtures/foo.rot', 2); $handler->setFormatter($this->getIdentityFormatter()); + $handler->setFilenameFormat('{filename}-{date}', $dateFormat); $handler->handle($this->getRecord()); $handler->close(); @@ -72,11 +106,88 @@ class RotatingFileHandlerTest extends TestCase public function rotationTests() { + $now = time(); + $dayCallback = function($ago) use ($now) { + return $now + 86400 * $ago; + }; + $monthCallback = function($ago) { + return gmmktime(0, 0, 0, date('n') + $ago, date('d'), date('Y')); + }; + $yearCallback = function($ago) { + return gmmktime(0, 0, 0, date('n'), date('d'), date('Y') + $ago); + }; + return array( 'Rotation is triggered when the file of the current day is not present' - => array(true), - 'Rotation is not triggered when the file is already present' - => array(false), + => array(true, RotatingFileHandler::FILE_PER_DAY, $dayCallback), + 'Rotation is not triggered when the file of the current day is already present' + => array(false, RotatingFileHandler::FILE_PER_DAY, $dayCallback), + + 'Rotation is triggered when the file of the current month is not present' + => array(true, RotatingFileHandler::FILE_PER_MONTH, $monthCallback), + 'Rotation is not triggered when the file of the current month is already present' + => array(false, RotatingFileHandler::FILE_PER_MONTH, $monthCallback), + + 'Rotation is triggered when the file of the current year is not present' + => array(true, RotatingFileHandler::FILE_PER_YEAR, $yearCallback), + 'Rotation is not triggered when the file of the current year is already present' + => array(false, RotatingFileHandler::FILE_PER_YEAR, $yearCallback), + ); + } + + /** + * @dataProvider dateFormatProvider + */ + public function testAllowOnlyFixedDefinedDateFormats($dateFormat, $valid) + { + $handler = new RotatingFileHandler(__DIR__.'/Fixtures/foo.rot', 2); + $handler->setFilenameFormat('{filename}-{date}', $dateFormat); + if (!$valid) { + $this->assertErrorWasTriggered( + E_USER_DEPRECATED, + 'Invalid date format - format should be one of '. + 'RotatingFileHandler::FILE_PER_DAY, RotatingFileHandler::FILE_PER_MONTH '. + 'or RotatingFileHandler::FILE_PER_YEAR.' + ); + } + } + + public function dateFormatProvider() + { + return array( + array(RotatingFileHandler::FILE_PER_DAY, true), + array(RotatingFileHandler::FILE_PER_MONTH, true), + array(RotatingFileHandler::FILE_PER_YEAR, true), + array('m-d-Y', false), + array('Y-m-d-h-i', false) + ); + } + + /** + * @dataProvider filenameFormatProvider + */ + public function testDisallowFilenameFormatsWithoutDate($filenameFormat, $valid) + { + $handler = new RotatingFileHandler(__DIR__.'/Fixtures/foo.rot', 2); + $handler->setFilenameFormat($filenameFormat, RotatingFileHandler::FILE_PER_DAY); + if (!$valid) { + $this->assertErrorWasTriggered( + E_USER_DEPRECATED, + 'Invalid filename format - format should contain at least `{date}`, because otherwise rotating is impossible.' + ); + } + } + + public function filenameFormatProvider() + { + return array( + array('{filename}', false), + array('{filename}-{date}', true), + array('{date}', true), + array('foobar-{date}', true), + array('foo-{date}-bar', true), + array('{date}-foobar', true), + array('foobar', false), ); } @@ -95,5 +206,6 @@ class RotatingFileHandlerTest extends TestCase foreach (glob(__DIR__.'/Fixtures/*.rot') as $file) { unlink($file); } + restore_error_handler(); } }