From 1da70011f2729bfa20a93c52fcfd9409cb9a8359 Mon Sep 17 00:00:00 2001 From: ssgoncalves Date: Mon, 10 Oct 2022 12:07:24 -0300 Subject: [PATCH] fix: add catch to any throwable in consumer manager --- src/Connectors/Consumer/Manager.php | 7 +-- src/TopicHandler/Consumer/AbstractHandler.php | 5 +- src/TopicHandler/Consumer/Handler.php | 4 +- tests/Integration/Dummies/MessageConsumer.php | 6 +- .../Unit/Connectors/Consumer/ManagerTest.php | 63 +++++++++---------- tests/Unit/Dummies/ConsumerHandlerDummy.php | 4 +- 6 files changed, 42 insertions(+), 47 deletions(-) diff --git a/src/Connectors/Consumer/Manager.php b/src/Connectors/Consumer/Manager.php index 64ff15b4..7e831c27 100644 --- a/src/Connectors/Consumer/Manager.php +++ b/src/Connectors/Consumer/Manager.php @@ -2,7 +2,6 @@ namespace Metamorphosis\Connectors\Consumer; -use Exception; use Metamorphosis\Consumers\ConsumerInterface; use Metamorphosis\Exceptions\ResponseTimeoutException; use Metamorphosis\Exceptions\ResponseWarningException; @@ -10,6 +9,7 @@ use Metamorphosis\Record\ConsumerRecord; use Metamorphosis\TopicHandler\Consumer\Handler as ConsumerHandler; use RdKafka\Message; +use Throwable; class Manager { @@ -65,9 +65,8 @@ public function handleMessage(): void $this->consumerHandler->warning($exception); return; - } catch (Exception $exception) { - $this->consumerHandler->failed($exception); - + } catch (Throwable $throwable) { + $this->consumerHandler->failed($throwable); return; } diff --git a/src/TopicHandler/Consumer/AbstractHandler.php b/src/TopicHandler/Consumer/AbstractHandler.php index 2d5d51da..62d0d07b 100644 --- a/src/TopicHandler/Consumer/AbstractHandler.php +++ b/src/TopicHandler/Consumer/AbstractHandler.php @@ -2,9 +2,9 @@ namespace Metamorphosis\TopicHandler\Consumer; -use Exception; use Metamorphosis\Exceptions\ResponseWarningException; use Metamorphosis\TopicHandler\ConfigOptions\Consumer as ConsumerConfigOptions; +use Throwable; abstract class AbstractHandler implements Handler { @@ -25,11 +25,10 @@ public function __construct(?ConsumerConfigOptions $configOptions = null) public function warning(ResponseWarningException $exception): void { } - /** * @phpcsSuppress SlevomatCodingStandard.Functions.UnusedParameter.UnusedParameter */ - public function failed(Exception $exception): void + public function failed(Throwable $exception): void { } diff --git a/src/TopicHandler/Consumer/Handler.php b/src/TopicHandler/Consumer/Handler.php index 0b7f1368..b7fccb0f 100644 --- a/src/TopicHandler/Consumer/Handler.php +++ b/src/TopicHandler/Consumer/Handler.php @@ -2,9 +2,9 @@ namespace Metamorphosis\TopicHandler\Consumer; -use Exception; use Metamorphosis\Exceptions\ResponseWarningException; use Metamorphosis\Record\RecordInterface; +use Throwable; interface Handler { @@ -26,5 +26,5 @@ public function finished(): void; /** * Handle failure process. */ - public function failed(Exception $exception): void; + public function failed(Throwable $throwable): void; } diff --git a/tests/Integration/Dummies/MessageConsumer.php b/tests/Integration/Dummies/MessageConsumer.php index 9b2c5cc7..5d86f423 100644 --- a/tests/Integration/Dummies/MessageConsumer.php +++ b/tests/Integration/Dummies/MessageConsumer.php @@ -2,11 +2,11 @@ namespace Tests\Integration\Dummies; -use Exception; use Illuminate\Support\Facades\Log; use Metamorphosis\Exceptions\ResponseWarningException; use Metamorphosis\Record\RecordInterface; use Metamorphosis\TopicHandler\Consumer\AbstractHandler; +use Throwable; class MessageConsumer extends AbstractHandler { @@ -24,10 +24,10 @@ public function warning(ResponseWarningException $exception): void ]); } - public function failed(Exception $exception): void + public function failed(Throwable $throwable): void { Log::error('Failed to handle kafka record for sku.', [ - 'exception' => $exception, + 'exception' => $throwable, ]); } } diff --git a/tests/Unit/Connectors/Consumer/ManagerTest.php b/tests/Unit/Connectors/Consumer/ManagerTest.php index 52e3bcc1..baa8ced2 100644 --- a/tests/Unit/Connectors/Consumer/ManagerTest.php +++ b/tests/Unit/Connectors/Consumer/ManagerTest.php @@ -2,7 +2,9 @@ namespace Tests\Unit\Connectors\Consumer; +use Error; use Exception; +use InvalidArgumentException; use Metamorphosis\Connectors\Consumer\Manager; use Metamorphosis\Consumers\ConsumerInterface; use Metamorphosis\Exceptions\ResponseTimeoutException; @@ -13,6 +15,9 @@ use Mockery as m; use RdKafka\Message as KafkaMessage; use Tests\LaravelTestCase; +use Tests\Unit\Dummies\ConsumerHandlerDummy; +use Throwable; +use TypeError; class ManagerTest extends LaravelTestCase { @@ -48,27 +53,11 @@ public function testShouldHandleMultiplesMessages(): void $kafkaMessage3->payload = 'original message 3'; $kafkaMessage3->err = RD_KAFKA_RESP_ERR_NO_ERROR; - $messages = [$kafkaMessage1, $kafkaMessage2, $kafkaMessage3]; - $count = 0; - $exception = new Exception('Exception occurs when consuming.'); - // Expectations - $consumer->shouldReceive('consume') - ->times(4) - ->andReturnUsing( - function () use ($messages, &$count, $exception) { - $message = $messages[$count] ?? null; - if (!$message) { - throw $exception; - } - $count++; - - return $message; - } - ); - - $consumerHandler->expects() - ->failed($exception); + $consumer->shouldReceive() + ->consume() + ->times(3) + ->andReturn($kafkaMessage1, $kafkaMessage2, $kafkaMessage3); $dispatcher->expects() ->handle($consumerRecord) @@ -78,7 +67,6 @@ function () use ($messages, &$count, $exception) { $runner->handleMessage(); $runner->handleMessage(); $runner->handleMessage(); - $runner->handleMessage(); } public function testShouldCallWarningWhenErrorOccurs(): void @@ -149,19 +137,10 @@ public function testShouldHandleAsyncCommit(): void ); // Expectations - $consumer->shouldReceive('consume') + $consumer->shouldReceive() + ->consume() ->times(3) - ->andReturnUsing( - function () use ($messages, &$count, $exception) { - $message = $messages[$count] ?? null; - if (!$message) { - throw $exception; - } - $count++; - - return $message; - } - ); + ->andReturn($kafkaMessage1, $kafkaMessage2); $consumer->expects() ->commitAsync() @@ -184,4 +163,22 @@ function () use ($messages, &$count, $exception) { $runner->handleMessage(); $runner->handleMessage(); } + + public function getThrowableScenarios(): array + { + return [ + 'Exception' => [ + 'throwable' => new Exception(), + ], + 'Error' => [ + 'throwable' => new Error(), + ], + 'InvalidArgumentException' => [ + 'throwable' => new InvalidArgumentException(), + ], + 'TypeError' => [ + 'throwable' => new TypeError(), + ], + ]; + } } diff --git a/tests/Unit/Dummies/ConsumerHandlerDummy.php b/tests/Unit/Dummies/ConsumerHandlerDummy.php index 3ac49c63..fc9f6fec 100644 --- a/tests/Unit/Dummies/ConsumerHandlerDummy.php +++ b/tests/Unit/Dummies/ConsumerHandlerDummy.php @@ -2,9 +2,9 @@ namespace Tests\Unit\Dummies; -use Exception; use Metamorphosis\Record\RecordInterface; use Metamorphosis\TopicHandler\Consumer\AbstractHandler; +use Throwable; class ConsumerHandlerDummy extends AbstractHandler { @@ -12,7 +12,7 @@ public function handle(RecordInterface $data): void { } - public function failed(Exception $exception): void + public function failed(Throwable $throwable): void { } }