Merge pull request #6 from marcrubiocmp/master

Exceptions in MessageHandler are masked as time out errors
Mad-Collective · Mar 10, 2017 · 9b3e238 · 9b3e238
2 parents ba6b6bf + acd6446
commit 9b3e238
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 43 deletions.
diff --git a/src/Cmp/Queues/Domain/Queue/Exception/TimeoutReaderException.php b/src/Cmp/Queues/Domain/Queue/Exception/TimeoutReaderException.php
@@ -1,14 +1,7 @@
 <?php
-/**
- * Created by PhpStorm.
- * User: quimmanrique
- * Date: 17/02/17
- * Time: 15:25
- */
-
 namespace Cmp\Queues\Domain\Queue\Exception;
 
 
-class TimeoutReaderException extends \Exception
+class TimeoutReaderException extends ReaderException
 {
 }
diff --git a/src/Cmp/Queues/Domain/Task/JSONTaskFactory.php b/src/Cmp/Queues/Domain/Task/JSONTaskFactory.php
@@ -4,18 +4,29 @@
 
 use Cmp\Queues\Domain\Queue\JSONMessageFactory;
 use Cmp\Queues\Domain\Task\Exception\InvalidJSONTaskException;
+use Cmp\Queues\Domain\Task\Exception\TaskException;
 
 class JSONTaskFactory implements JSONMessageFactory
 {
-
+    /**
+     * @param $json
+     *
+     * @return Task
+     * @throws InvalidJSONTaskException
+     * @throws TaskException
+     */
     public function create($json)
     {
-        try {
-            $taskArray = json_decode($json, true);
-            return new Task($taskArray['name'], $taskArray['body'], $taskArray['delay']);
-        } catch (\Exception $e) {
-            throw new InvalidJSONTaskException();
+        $taskArray = json_decode($json, true);
+
+        if (json_last_error() !== JSON_ERROR_NONE) {
+            throw new InvalidJSONTaskException("String is not valid JSON");
         }
-    }
 
+        if (!isset($taskArray['name'], $taskArray['body'])) {
+            throw new InvalidJSONTaskException("Cannot reconstruct task. Name or body fields are missing");
+        }
+
+       return new Task($taskArray['name'], $taskArray['body'], isset($taskArray['delay']) ? $taskArray['delay'] : 0);
+    }
 }
diff --git a/src/Cmp/Queues/Infrastructure/AmqpLib/v26/RabbitMQ/Queue/MessageHandler.php b/src/Cmp/Queues/Infrastructure/AmqpLib/v26/RabbitMQ/Queue/MessageHandler.php
@@ -1,16 +1,8 @@
 <?php
-/**
- * Created by PhpStorm.
- * User: quimmanrique
- * Date: 13/02/17
- * Time: 19:13
- */
-
 namespace Cmp\Queues\Infrastructure\AmqpLib\v26\RabbitMQ\Queue;
 
 use Cmp\Queues\Domain\Queue\JSONMessageFactory;
 use PhpAmqpLib\Message\AMQPMessage;
-use Psr\Log\LoggerInterface;
 
 class MessageHandler
 {
@@ -39,13 +31,9 @@ public function __construct(JSONMessageFactory $jsonMessageFactory)
      */
     public function handleMessage(AMQPMessage $message)
     {
-        try {
-            $task = $this->jsonMessageFactory->create($message->body);
-            call_user_func_array($this->callback, [$task]);
-            $message->delivery_info['channel']->basic_ack($message->delivery_info['delivery_tag']);
-        } catch (\Exception $e) {
-            throw $e;
-        }
+        $task = $this->jsonMessageFactory->create($message->body);
+        call_user_func_array($this->callback, [$task]);
+        $message->delivery_info['channel']->basic_ack($message->delivery_info['delivery_tag']);
     }
 
     public function setCallback(callable $callback)

diff --git a/src/Cmp/Queues/Infrastructure/AmqpLib/v26/RabbitMQ/Queue/QueueReader.php b/src/Cmp/Queues/Infrastructure/AmqpLib/v26/RabbitMQ/Queue/QueueReader.php
@@ -1,11 +1,4 @@
 <?php
-/**
- * Created by PhpStorm.
- * User: quimmanrique
- * Date: 13/02/17
- * Time: 17:33
- */
-
 namespace Cmp\Queues\Infrastructure\AmqpLib\v26\RabbitMQ\Queue;
 
 use Cmp\Queues\Domain\Queue\Exception\ReaderException;
@@ -17,6 +10,7 @@
 use Cmp\Queues\Infrastructure\AmqpLib\v26\RabbitMQ\Queue\Config\QueueConfig;
 use PhpAmqpLib\Channel\AMQPChannel;
 use PhpAmqpLib\Connection\AMQPLazyConnection;
+use PhpAmqpLib\Exception\AMQPTimeoutException;
 use Psr\Log\LoggerInterface;
 
 class QueueReader implements DomainQueueReader
@@ -102,8 +96,10 @@ public function read(callable $callback, $timeout=0)
         $this->messageHandler->setCallback($callback);
         try {
             $this->channel->wait(null, false, $timeout);
+        } catch(AMQPTimeoutException $e) {
+            throw new TimeoutReaderException("Timed out at $timeout seconds while reading.", 0, $e);
         } catch(\Exception $e) {
-            throw new TimeoutReaderException();
+            throw new ReaderException("Error occurred while reading", 0, $e);
         }
     }
 
@@ -150,7 +146,7 @@ protected function initialize()
             );
         } catch (\ErrorException $exception) {
             $this->logger->error('Error trying to connect to rabbitMQ:' . $exception->getMessage());
-            throw new ReaderException($exception->getMessage(), $exception->getCode());
+            throw new ReaderException("Error initializing queue reader", 0, $exception);
         }
     }
 

diff --git a/tests/spec/Cmp/Queues/Domain/Task/JSONTaskFactorySpec.php b/tests/spec/Cmp/Queues/Domain/Task/JSONTaskFactorySpec.php
@@ -2,6 +2,7 @@
 
 namespace spec\Cmp\Queues\Domain\Task;
 
+use Cmp\Queues\Domain\Task\Exception\InvalidJSONTaskException;
 use Cmp\Queues\Domain\Task\Task;
 use PhpSpec\ObjectBehavior;
 use Prophecy\Argument;
@@ -16,9 +17,22 @@ function it_is_initializable()
     function it_should_convert_from_json_to_Task()
     {
         $taskPreFactory = new Task('name', array(1,2,3), 10);
-        $taskPostFactory = $this->create(json_encode($taskPreFactory));
-        $taskPostFactory->getName()->shouldBe($taskPreFactory->getName());
-        $taskPostFactory->getBody()->shouldBe($taskPreFactory->getBody());
-        $taskPostFactory->getDelay()->shouldBe($taskPreFactory->getDelay());
+        $this->create(json_encode($taskPreFactory))->shouldBeLike($taskPreFactory);
+    }
+
+    function it_throws_exception_for_invalid_json()
+    {
+        $invalidJsonString = 'fsadfgkajghksdghdg';
+        $this->shouldThrow(InvalidJSONTaskException::class)->duringCreate($invalidJsonString);
+    }
+
+    function it_throws_exception_when_missing_required_keys()
+    {
+        $decodedJsonData = [
+            'foo' => 'bar'
+        ];
+
+        $jsonStr = json_encode($decodedJsonData);
+        $this->shouldThrow(InvalidJSONTaskException::class)->duringCreate($jsonStr);
     }
 }