Force exceptions on error responses for transaction control commands.

Commands such as MULTI, EXEC, DISCARD, WATCH and UNWATCH disregard the
the "exceptions" client option and always result in an exception being
thrown on error responses even when the client is configured to return
those errors ("exceptions" set to FALSE).

lib/Predis/Transaction/MultiExec.php

@@ -136,7 +136,7 @@ class MultiExec implements BasicClientInterface, ExecutableContextInterface
         $discarded = $this->state->isDiscarded();
 
         if (!$cas || ($cas && $discarded)) {
-            $this->client->multi();
+            $this->call('multi');
 
             if ($discarded) {
                 $this->state->unflag(MultiExecState::CAS);
@@ -162,6 +162,25 @@ class MultiExec implements BasicClientInterface, ExecutableContextInterface
         return $response;
     }
 
+    /**
+     * Sends a Redis command bypassing the transaction logic.
+     *
+     * @param string $method Command ID.
+     * @param array $arguments Arguments for the command.
+     * @return mixed
+     */
+    protected function call($commandID, $arguments = array())
+    {
+        $command  = $this->client->createCommand($commandID, $arguments);
+        $response = $this->client->executeCommand($command);
+
+        if ($response instanceof Response\Error) {
+            throw new Response\ServerException($response->getMessage());
+        }
+
+        return $response;
+    }
+
     /**
      * Executes the specified Redis command.
      *
@@ -202,7 +221,8 @@ class MultiExec implements BasicClientInterface, ExecutableContextInterface
             throw new ClientException('WATCH after MULTI is not allowed');
         }
 
-        $response = $this->client->watch($keys);
+        $response = $this->call('watch', array($keys));
+
         $this->state->flag(MultiExecState::WATCH);
 
         return $response;
@@ -217,7 +237,7 @@ class MultiExec implements BasicClientInterface, ExecutableContextInterface
     {
         if ($this->state->check(MultiExecState::INITIALIZED | MultiExecState::CAS)) {
             $this->state->unflag(MultiExecState::CAS);
-            $this->client->multi();
+            $this->call('multi');
         } else {
             $this->initialize();
         }
@@ -251,7 +271,8 @@ class MultiExec implements BasicClientInterface, ExecutableContextInterface
     public function discard()
     {
         if ($this->state->isInitialized()) {
-            $this->client->{$this->state->isCAS() ? 'unwatch' : 'discard'}();
+            $this->call($this->state->isCAS() ? 'unwatch' : 'discard');
+
             $this->reset();
             $this->state->flag(MultiExecState::DISCARDED);
         }
@@ -329,7 +350,7 @@ class MultiExec implements BasicClientInterface, ExecutableContextInterface
                 return;
             }
 
-            $execResponse = $this->client->exec();
+            $execResponse = $this->call('exec');
 
             if ($execResponse === null) {
                 if ($attempts === 0) {

tests/Predis/Transaction/MultiExecTest.php

@@ -483,6 +483,92 @@ class MultiExecTest extends StandardTestCase
         $this->assertSame(array('MULTI', 'SET', 'ECHO', 'DISCARD'), self::commandsToIDs($commands));
     }
 
+    /**
+     * @group disconnected
+     */
+    public function testExceptionsOptionTakesPrecedenceOverClientOptionsWhenFalse()
+    {
+        $expected = array('before', new Response\Error('ERR simulated error'), 'after');
+
+        $connection = $this->getMockedConnection(function (CommandInterface $command) use ($expected) {
+            switch ($command->getId()) {
+                case 'MULTI':
+                    return true;
+
+                case 'EXEC':
+                    return $expected;
+
+                default:
+                    return new Response\StatusQueued();
+            }
+        });
+
+        $client = new Client($connection, array('exceptions' => true));
+        $tx = new MultiExec($client, array('exceptions' => false));
+
+        $result = $tx->multi()
+                     ->echo('before')
+                     ->echo('ERROR PLEASE!')
+                     ->echo('after')
+                     ->exec();
+
+        $this->assertSame($expected, $result);
+    }
+
+    /**
+     * @group disconnected
+     * @expectedException Predis\Response\ServerException
+     * @expectedExceptionMessage ERR simulated error
+     */
+    public function testExceptionsOptionTakesPrecedenceOverClientOptionsWhenTrue()
+    {
+        $expected = array('before', new Response\Error('ERR simulated error'), 'after');
+
+        $connection = $this->getMockedConnection(function (CommandInterface $command) use ($expected) {
+            switch ($command->getId()) {
+                case 'MULTI':
+                    return true;
+
+                case 'EXEC':
+                    return $expected;
+
+                default:
+                    return new Response\StatusQueued();
+            }
+        });
+
+        $client = new Client($connection, array('exceptions' => false));
+        $tx = new MultiExec($client, array('exceptions' => true));
+
+        $tx->multi()->echo('before')->echo('ERROR PLEASE!')->echo('after')->exec();
+    }
+
+    /**
+     * @group disconnected
+     * @expectedException Predis\Response\ServerException
+     * @expectedExceptionMessage ERR simulated failure on EXEC
+     */
+    public function testExceptionsOptionDoesNotAffectTransactionControlCommands()
+    {
+        $connection = $this->getMockedConnection(function (CommandInterface $command) {
+            switch ($command->getId()) {
+                case 'MULTI':
+                    return true;
+
+                case 'EXEC':
+                    return new Response\Error('ERR simulated failure on EXEC');
+
+                default:
+                    return new Response\StatusQueued();
+            }
+        });
+
+        $client = new Client($connection, array('exceptions' => false));
+        $tx = new MultiExec($client);
+
+        $tx->multi()->echo('test')->exec();
+    }
+
     // ******************************************************************** //
     // ---- INTEGRATION TESTS --------------------------------------------- //
     // ******************************************************************** //
@@ -666,11 +752,11 @@ class MultiExecTest extends StandardTestCase
      * @param \Closure $executeCallback
      * @return MultiExec
      */
-    protected function getMockedTransaction($executeCallback, $options = array())
+    protected function getMockedTransaction($executeCallback, $txOpts = null, $clientOpts = null)
     {
         $connection = $this->getMockedConnection($executeCallback);
-        $client = new Client($connection);
-        $transaction = new MultiExec($client, $options);
+        $client = new Client($connection, $clientOpts ?: array());
+        $transaction = new MultiExec($client, $txOpts ?: array());
 
         return $transaction;
     }
@@ -698,21 +784,21 @@ class MultiExecTest extends StandardTestCase
             switch ($cmd) {
                 case 'WATCH':
                     if ($multi) {
-                        throw new Response\ServerException("ERR $cmd inside MULTI is not allowed");
+                        return new Response\Error("ERR $cmd inside MULTI is not allowed");
                     }
 
                     return $watch = true;
 
                 case 'MULTI':
                     if ($multi) {
-                        throw new Response\ServerException("ERR MULTI calls can not be nested");
+                        return new Response\Error("ERR MULTI calls can not be nested");
                     }
 
                     return $multi = true;
 
                 case 'EXEC':
                     if (!$multi) {
-                        throw new Response\ServerException("ERR $cmd without MULTI");
+                        return new Response\Error("ERR $cmd without MULTI");
                     }
 
                     $watch = $multi = false;
@@ -727,7 +813,7 @@ class MultiExecTest extends StandardTestCase
 
                 case 'DISCARD':
                     if (!$multi) {
-                        throw new Response\ServerException("ERR $cmd without MULTI");
+                        return new Response\Error("ERR $cmd without MULTI");
                     }
 
                     $watch = $multi = false;