From d09d2951c78bccd3e53c6f8dd6784cbc1e790e1d Mon Sep 17 00:00:00 2001 From: Harry Bragg Date: Sat, 16 Jun 2018 23:05:04 +0100 Subject: [PATCH 1/2] remove code from isHandling, but dont throw exception on non handling --- src/Graze/Monolog/Handler/RaygunHandler.php | 56 +++++------------ .../Handler/RaygunHandlerIntegrationTest.php | 63 +++++++++++++++++++ .../Monolog/Handler/RaygunHandlerTest.php | 60 ++++++------------ 3 files changed, 100 insertions(+), 79 deletions(-) create mode 100644 tests/integration/src/Graze/Monolog/Handler/RaygunHandlerIntegrationTest.php diff --git a/src/Graze/Monolog/Handler/RaygunHandler.php b/src/Graze/Monolog/Handler/RaygunHandler.php index 903c6f3..6d79c65 100644 --- a/src/Graze/Monolog/Handler/RaygunHandler.php +++ b/src/Graze/Monolog/Handler/RaygunHandler.php @@ -10,6 +10,7 @@ * @see http://github.com/graze/MonologExtensions/blob/master/LICENSE * @link http://github.com/graze/MonologExtensions */ + namespace Graze\Monolog\Handler; use Graze\Monolog\Formatter\RaygunFormatter; @@ -26,8 +27,8 @@ class RaygunHandler extends AbstractProcessingHandler /** * @param RaygunClient $client - * @param int $level - * @param bool $bubble + * @param int $level + * @param bool $bubble */ public function __construct(RaygunClient $client, $level = Logger::DEBUG, $bubble = true) { @@ -36,28 +37,6 @@ public function __construct(RaygunClient $client, $level = Logger::DEBUG, $bubbl parent::__construct($level, $bubble); } - /** - * {@inheritdoc} - */ - public function isHandling(array $record) - { - if (parent::isHandling($record) && isset($record['context'])) { - $context = $record['context']; - - //Ensure only valid records will be handled and no InvalidArgumentException will be thrown - if ((isset($context['exception']) && - ( - $context['exception'] instanceof \Exception || - (PHP_VERSION_ID > 70000 && $context['exception'] instanceof \Throwable) - ) - ) || (isset($context['file']) && isset($context['line'])) - ) { - return true; - } - } - return false; - } - /** * @param array $record */ @@ -65,10 +44,10 @@ protected function write(array $record) { $context = $record['context']; - if (isset($context['exception']) && - ( - $context['exception'] instanceof \Exception || - (PHP_VERSION_ID > 70000 && $context['exception'] instanceof \Throwable) + if (isset($context['exception']) + && ( + $context['exception'] instanceof \Exception + || (PHP_VERSION_ID > 70000 && $context['exception'] instanceof \Throwable) ) ) { $this->writeException( @@ -77,25 +56,24 @@ protected function write(array $record) $record['formatted']['custom_data'], $record['formatted']['timestamp'] ); - } elseif (isset($context['file']) && $context['line']) { + } elseif (isset($context['file']) && isset($context['line'])) { $this->writeError( $record['formatted'], $record['formatted']['tags'], $record['formatted']['custom_data'], $record['formatted']['timestamp'] ); - } else { - throw new \InvalidArgumentException('Invalid record given.'); } + // do nothing if its not an exception or an error } /** - * @param array $record - * @param array $tags - * @param array $customData + * @param array $record + * @param array $tags + * @param array $customData * @param int|float $timestamp */ - protected function writeError(array $record, array $tags = array(), array $customData = array(), $timestamp = null) + protected function writeError(array $record, array $tags = [], array $customData = [], $timestamp = null) { $context = $record['context']; $this->client->SendError( @@ -110,12 +88,12 @@ protected function writeError(array $record, array $tags = array(), array $custo } /** - * @param array $record - * @param array $tags - * @param array $customData + * @param array $record + * @param array $tags + * @param array $customData * @param int|float $timestamp */ - protected function writeException(array $record, array $tags = array(), array $customData = array(), $timestamp = null) + protected function writeException(array $record, array $tags = [], array $customData = [], $timestamp = null) { $this->client->SendException($record['context']['exception'], $tags, $customData, $timestamp); } diff --git a/tests/integration/src/Graze/Monolog/Handler/RaygunHandlerIntegrationTest.php b/tests/integration/src/Graze/Monolog/Handler/RaygunHandlerIntegrationTest.php new file mode 100644 index 0000000..a2f5a53 --- /dev/null +++ b/tests/integration/src/Graze/Monolog/Handler/RaygunHandlerIntegrationTest.php @@ -0,0 +1,63 @@ +raygun = Mockery::mock(RaygunClient::class); + $this->handler = new RaygunHandler($this->raygun, Logger::NOTICE); + $this->logger = new Logger('raygunHandlerTest', [$this->handler]); + } + + public function testErrorWithExceptionTriggersLogger() + { + $exception = new RuntimeException('test exception'); + + $this->raygun + ->shouldReceive('SendException') + ->once() + ->with($exception, [], [], null); + + $this->logger->error('test error', ['exception' => $exception]); + } + + public function testErrorWithErrorWillTriggerLogger() + { + $this->raygun + ->shouldReceive('SendError') + ->once() + ->with(0, "test line error", __FILE__, 5, [], [], null); + $this->logger->error('test line error', ['file' => __FILE__, 'line' => 5]); + } + + public function testErrorWithNoLineWillDoNothing() + { + $this->logger->error('test line error', ['file' => __FILE__]); + } + + public function testErrorWithNoFileWillDoNothing() + { + $this->logger->error('test line error', ['line' => __FILE__]); + } + + public function testErrorWithNeitherWillDoNothing() + { + $this->logger->error('test line error'); + } +} diff --git a/tests/unit/src/Graze/Monolog/Handler/RaygunHandlerTest.php b/tests/unit/src/Graze/Monolog/Handler/RaygunHandlerTest.php index 5638167..574994e 100644 --- a/tests/unit/src/Graze/Monolog/Handler/RaygunHandlerTest.php +++ b/tests/unit/src/Graze/Monolog/Handler/RaygunHandlerTest.php @@ -105,50 +105,30 @@ public function testHandleException() $handler->handle($record); } - /** - * @dataProvider isHandleData - * - * @param array $record - * @param bool $expected - */ - public function testIsHandling(array $record, $expected) + public function testHandleEmptyDoesNothing() { - $handler = new RaygunHandler($this->client); + $record = $this->getRecord(300, 'bar'); + $record['extra'] = array('bar' => 'baz', 'tags' => array('foo', 'bar')); + $record['extra']['timestamp'] = 1234567890; + $formatted = array_merge($record, + array( + 'tags' => array('foo', 'bar'), + 'timestamp' => 1234567890, + 'custom_data' => array('bar' => 'baz') + ) + ); - $this->assertEquals($expected, $handler->isHandling($record)); - } + $formatter = m::mock('Monolog\\Formatter\\FormatterInterface'); + $handler = new RaygunHandler($this->client); + $handler->setFormatter($formatter); - /** - * @return array - */ - public function isHandleData() - { - return [ - [$this->getRecord(300, 'foo', ['exception' => new \Exception('foo')]), true], - [$this->getRecord(300, 'bar', ['file' => '/a/path/to/a/file', 'line' => 123]), true], - [$this->getRecord(300, 'baz', ['file' => '/a/path/to/a/file']), false], - [$this->getRecord(300, 'faz', ['line' => 456]), false], - // no context entry - [ - [ - 'message' => 'foobar', - 'level' => 300, - 'level_name' => Logger::getLevelName(300), - 'channel' => 'test', - 'datetime' => \DateTime::createFromFormat('U.u', sprintf('%.6F', microtime(true))), - 'extra' => [], - ], - false, - ], - ]; - } + $formatter + ->shouldReceive('format') + ->once() + ->with($record) + ->andReturn($formatted); - public function testHandleCallsIsHandlingFirst() - { - $handler = new RaygunHandler($this->client); - $nonHandlingRecord = $this->getRecord(300, 'baz', array()); - $this->assertFalse($handler->isHandling($nonHandlingRecord)); - $this->assertFalse($handler->handle($nonHandlingRecord)); + $handler->handle($record); } /** From 0b4b47871ce84d96818acc4fce6cc643a1ce9da5 Mon Sep 17 00:00:00 2001 From: Harry Bragg Date: Sat, 16 Jun 2018 23:22:21 +0100 Subject: [PATCH 2/2] code standards --- .../Handler/RaygunHandlerIntegrationTest.php | 3 +- .../Monolog/Handler/RaygunHandlerTest.php | 101 +++++++++--------- 2 files changed, 52 insertions(+), 52 deletions(-) diff --git a/tests/integration/src/Graze/Monolog/Handler/RaygunHandlerIntegrationTest.php b/tests/integration/src/Graze/Monolog/Handler/RaygunHandlerIntegrationTest.php index a2f5a53..7e8be1a 100644 --- a/tests/integration/src/Graze/Monolog/Handler/RaygunHandlerIntegrationTest.php +++ b/tests/integration/src/Graze/Monolog/Handler/RaygunHandlerIntegrationTest.php @@ -3,7 +3,6 @@ namespace Graze\Monolog\Handler; use Mockery; -use Mockery\MockInterface; use Monolog\Logger; use PHPUnit\Framework\TestCase; use Raygun4php\RaygunClient; @@ -13,7 +12,7 @@ class RaygunHandlerIntegrationTest extends TestCase { /** @var Logger */ private $logger; - /** @var RaygunClient|MockInterface */ + /** @var mixed */ private $raygun; /** @var RaygunHandler */ private $handler; diff --git a/tests/unit/src/Graze/Monolog/Handler/RaygunHandlerTest.php b/tests/unit/src/Graze/Monolog/Handler/RaygunHandlerTest.php index 574994e..98fb82d 100644 --- a/tests/unit/src/Graze/Monolog/Handler/RaygunHandlerTest.php +++ b/tests/unit/src/Graze/Monolog/Handler/RaygunHandlerTest.php @@ -1,8 +1,8 @@ getRecord(300, - 'foo', array( + 'foo', + [ 'file' => 'bar', 'line' => 1, - ) + ] ); - $record['context']['tags'] = array('foo'); + $record['context']['tags'] = ['foo']; $record['context']['timestamp'] = 1234567890; - $record['extra'] = array('bar' => 'baz', 'tags' => array('bar')); + $record['extra'] = ['bar' => 'baz', 'tags' => ['bar']]; $formatted = array_merge($record, - array( - 'tags' => array('foo', 'bar'), - 'timestamp' => 1234567890, - 'custom_data' => array('bar' => 'baz') - ) + [ + 'tags' => ['foo', 'bar'], + 'timestamp' => 1234567890, + 'custom_data' => ['bar' => 'baz'], + ] ); $formatter = m::mock('Monolog\\Formatter\\FormatterInterface'); @@ -56,14 +57,14 @@ public function testHandleError() $handler->setFormatter($formatter); $formatter - ->shouldReceive('format') - ->once() - ->with($record) - ->andReturn($formatted); + ->shouldReceive('format') + ->once() + ->with($record) + ->andReturn($formatted); $this->client - ->shouldReceive('SendError') - ->once() - ->with(0, 'foo', 'bar', 1, array('foo', 'bar'), array('bar' => 'baz'), 1234567890); + ->shouldReceive('SendError') + ->once() + ->with(0, 'foo', 'bar', 1, ['foo', 'bar'], ['bar' => 'baz'], 1234567890); $handler->handle($record); } @@ -71,36 +72,36 @@ public function testHandleError() public function testHandleException() { $exception = new \Exception('foo'); - $record = $this->getRecord(300, 'foo', array('exception' => $exception)); - $record['extra'] = array('bar' => 'baz', 'tags' => array('foo', 'bar')); + $record = $this->getRecord(300, 'foo', ['exception' => $exception]); + $record['extra'] = ['bar' => 'baz', 'tags' => ['foo', 'bar']]; $record['extra']['timestamp'] = 1234567890; $formatted = array_merge($record, - array( - 'tags' => array('foo', 'bar'), - 'timestamp' => 1234567890, - 'custom_data' => array('bar' => 'baz') - ) + [ + 'tags' => ['foo', 'bar'], + 'timestamp' => 1234567890, + 'custom_data' => ['bar' => 'baz'], + ] ); - $formatted['context']['exception'] = array( - 'class' => get_class($exception), + $formatted['context']['exception'] = [ + 'class' => get_class($exception), 'message' => $exception->getMessage(), - 'code' => $exception->getCode(), - 'file' => $exception->getFile().':'.$exception->getLine(), - ); + 'code' => $exception->getCode(), + 'file' => $exception->getFile() . ':' . $exception->getLine(), + ]; $formatter = m::mock('Monolog\\Formatter\\FormatterInterface'); $handler = new RaygunHandler($this->client); $handler->setFormatter($formatter); $formatter - ->shouldReceive('format') - ->once() - ->with($record) - ->andReturn($formatted); + ->shouldReceive('format') + ->once() + ->with($record) + ->andReturn($formatted); $this->client - ->shouldReceive('SendException') - ->once() - ->with($exception, array('foo', 'bar'), array('bar' => 'baz'), 1234567890); + ->shouldReceive('SendException') + ->once() + ->with($exception, ['foo', 'bar'], ['bar' => 'baz'], 1234567890); $handler->handle($record); } @@ -108,14 +109,14 @@ public function testHandleException() public function testHandleEmptyDoesNothing() { $record = $this->getRecord(300, 'bar'); - $record['extra'] = array('bar' => 'baz', 'tags' => array('foo', 'bar')); + $record['extra'] = ['bar' => 'baz', 'tags' => ['foo', 'bar']]; $record['extra']['timestamp'] = 1234567890; $formatted = array_merge($record, - array( - 'tags' => array('foo', 'bar'), - 'timestamp' => 1234567890, - 'custom_data' => array('bar' => 'baz') - ) + [ + 'tags' => ['foo', 'bar'], + 'timestamp' => 1234567890, + 'custom_data' => ['bar' => 'baz'], + ] ); $formatter = m::mock('Monolog\\Formatter\\FormatterInterface'); @@ -137,14 +138,14 @@ public function testHandleEmptyDoesNothing() public function testHandleThrowable() { $exception = new \TypeError('foo'); - $record = $this->getRecord(300, 'foo', array('exception' => $exception)); - $record['context']['tags'] = array('foo'); + $record = $this->getRecord(300, 'foo', ['exception' => $exception]); + $record['context']['tags'] = ['foo']; $formatted = array_merge($record, - array( - 'tags' => array('foo'), - 'custom_data' => array(), - 'timestamp' => null, - ) + [ + 'tags' => ['foo'], + 'custom_data' => [], + 'timestamp' => null, + ] ); $formatter = m::mock('Monolog\\Formatter\\FormatterInterface'); @@ -159,7 +160,7 @@ public function testHandleThrowable() $this->client ->shouldReceive('SendException') ->once() - ->with($exception, array('foo'), array(), null); + ->with($exception, ['foo'], [], null); $handler->handle($record); }