Easier exception thrown when sending commands in parallel

Adding CommandTransferException
Cleaning up Exceptions thrown from multi transfer
Closes #197

src/Guzzle/Common/Exception/ExceptionCollection.php

@@ -55,4 +55,14 @@ class ExceptionCollection extends \Exception implements GuzzleException, \Iterat
     {
         return new \ArrayIterator($this->exceptions);
     }
+
+    /**
+     * Get the first exception in the collection
+     *
+     * @return \Exception
+     */
+    public function getFirst()
+    {
+        return $this->exceptions ? $this->exceptions[0] : null;
+    }
 }

src/Guzzle/Http/Client.php

@@ -362,18 +362,13 @@ class Client extends AbstractHasDispatcher implements ClientInterface
         try {
             $curlMulti->send();
         } catch (ExceptionCollection $e) {
-            throw $multipleRequests ? $e : $e->getIterator()->offsetGet(0);
+            throw $multipleRequests ? $e : $e->getFirst();
         }
 
         if (!$multipleRequests) {
             return end($requests)->getResponse();
         } else {
-            return array_map(
-                function ($request) {
-                    return $request->getResponse();
-                },
-                $requests
-            );
+            return array_map(function ($request) { return $request->getResponse(); }, $requests);
         }
     }
 

src/Guzzle/Http/Curl/CurlMulti.php

@@ -323,7 +323,7 @@ class CurlMulti extends AbstractHasDispatcher implements CurlMultiInterface
         // Keep a list of all requests, and remove errored requests from the list
         $store = new \SplObjectStorage();
         foreach ($requestsInScope as $request) {
-            $store->attach($request, $request);
+            $store->attach($request);
         }
 
         $multiException = new MultiTransferException('Errors during multi transfer');

src/Guzzle/Http/Exception/MultiTransferException.php

@@ -24,7 +24,7 @@ class MultiTransferException extends ExceptionCollection
     }
 
     /**
-     * Set an array of successful requests
+     * Add to the array of successful requests
      *
      * @param RequestInterface $request Successful request
      *
@@ -38,7 +38,7 @@ class MultiTransferException extends ExceptionCollection
     }
 
     /**
-     * Set an array of failed requests
+     * Add to the array of failed requests
      *
      * @param RequestInterface $request Failed request
      *

src/Guzzle/Service/Client.php

@@ -8,6 +8,8 @@ use Guzzle\Common\Exception\BadMethodCallException;
 use Guzzle\Inflection\InflectorInterface;
 use Guzzle\Inflection\Inflector;
 use Guzzle\Http\Client as HttpClient;
+use Guzzle\Http\Exception\MultiTransferException;
+use Guzzle\Service\Exception\CommandTransferException;
 use Guzzle\Http\Message\RequestInterface;
 use Guzzle\Service\Command\CommandInterface;
 use Guzzle\Service\Command\Factory\CompositeFactory;
@@ -198,6 +200,7 @@ class Client extends HttpClient implements ClientInterface
      *
      * @return mixed Returns the result of the executed command or an array of commands if executing multiple commands
      * @throws InvalidArgumentException if an invalid command is passed
+     * @throws CommandTransferException if an exception is encountered when transferring multiple commands
      */
     public function execute($command)
     {
@@ -210,26 +213,53 @@ class Client extends HttpClient implements ClientInterface
             throw new InvalidArgumentException('Command must be a command or array of commands');
         }
 
+        $failureException = null;
         $requests = array();
+        $successful = new \SplObjectStorage();
 
         foreach ($command as $c) {
             $c->setClient($this);
             // Set the state to new if the command was previously executed
-            $requests[] = $c->prepare()->setState(RequestInterface::STATE_NEW);
+            $request = $c->prepare()->setState(RequestInterface::STATE_NEW);
+            $successful[$request] = $c;
+            $requests[] = $request;
             $this->dispatch('command.before_send', array('command' => $c));
         }
 
-        if ($singleCommand) {
-            $this->send($requests[0]);
-        } else {
+        try {
             $this->send($requests);
+        } catch (MultiTransferException $failureException) {
+            $failures = new \SplObjectStorage();
+            // Remove failed requests from the successful requests array and add to the failures array
+            foreach ($failureException->getFailedRequests() as $request) {
+                if (isset($successful[$request])) {
+                    $failures[$request] = $successful[$request];
+                    unset($successful[$request]);
+                }
+            }
         }
 
-        foreach ($command as $c) {
+        foreach ($successful as $c) {
             $this->dispatch('command.after_send', array('command' => $c));
         }
 
-        return $singleCommand ? end($command)->getResult() : $command;
+        // Return the response or throw an exception
+        if (!$failureException) {
+            return $singleCommand ? end($command)->getResult() : $command;
+        } elseif ($singleCommand) {
+            // If only sending a single request, then don't use a CommandTransferException
+            throw $failureException->getFirst();
+        } else {
+            // Throw a CommandTransferException using the successful and failed commands
+            $e = CommandTransferException::fromMultiTransferException($failureException);
+            foreach ($failures as $failure) {
+                $e->addFailedCommand($failures[$failure]);
+            }
+            foreach ($successful as $success) {
+                $e->addSuccessfulCommand($successful[$success]);
+            }
+            throw $e;
+        }
     }
 
     /**

src/Guzzle/Service/Exception/CommandTransferException.php

@@ -0,0 +1,95 @@
+<?php
+
+namespace Guzzle\Service\Exception;
+
+use Guzzle\Http\Exception\MultiTransferException;
+use Guzzle\Service\Command\CommandInterface;
+
+/**
+ * Exception thrown when transferring commands in parallel
+ */
+class CommandTransferException extends MultiTransferException
+{
+    protected $successfulCommands = array();
+    protected $failedCommands = array();
+
+    /**
+     * Creates a new CommandTransferException from a MultiTransferException
+     *
+     * @param MultiTransferException $e Exception to base a new exception on
+     *
+     * @return self
+     */
+    public static function fromMultiTransferException(MultiTransferException $e)
+    {
+        $ce = new self($e->getMessage(), $e->getCode(), $e->getPrevious());
+
+        foreach ($e->getSuccessfulRequests() as $request) {
+            $ce->addSuccessfulRequest($request);
+        }
+
+        foreach ($e->getFailedRequests() as $request) {
+            $ce->addFailedRequest($request);
+        }
+
+        return $ce;
+    }
+
+    /**
+     * Get all of the commands in the transfer
+     *
+     * @return array
+     */
+    public function getAllCommands()
+    {
+        return array_merge($this->successfulCommands, $this->failedCommands);
+    }
+
+    /**
+     * Add to the array of successful commands
+     *
+     * @param CommandInterface $command Successful command
+     *
+     * @return self
+     */
+    public function addSuccessfulCommand(CommandInterface $command)
+    {
+        $this->successfulCommands[] = $command;
+
+        return $this;
+    }
+
+    /**
+     * Add to the array of failed commands
+     *
+     * @param CommandInterface $command Failed command
+     *
+     * @return self
+     */
+    public function addFailedCommand(CommandInterface $command)
+    {
+        $this->failedCommands[] = $command;
+
+        return $this;
+    }
+
+    /**
+     * Get an array of successful commands
+     *
+     * @return array
+     */
+    public function getSuccessfulCommands()
+    {
+        return $this->successfulCommands;
+    }
+
+    /**
+     * Get an array of failed commands
+     *
+     * @return array
+     */
+    public function getFailedCommands()
+    {
+        return $this->failedCommands;
+    }
+}

tests/Guzzle/Tests/Common/Exception/ExceptionCollectionTest.php

@@ -21,6 +21,7 @@ class ExceptionCollectionTest extends \Guzzle\Tests\GuzzleTestCase
         $e->add($exceptions[0]);
         $e->add($exceptions[1]);
         $this->assertEquals("Test\nTesting", $e->getMessage());
+        $this->assertSame($exceptions[0], $e->getFirst());
     }
 
     public function testActsAsArray()

tests/Guzzle/Tests/Service/ClientTest.php

@@ -7,6 +7,7 @@ use Guzzle\Http\Message\Response;
 use Guzzle\Plugin\Mock\MockPlugin;
 use Guzzle\Service\Description\Operation;
 use Guzzle\Service\Client;
+use Guzzle\Service\Exception\CommandTransferException;
 use Guzzle\Service\Description\ServiceDescription;
 use Guzzle\Tests\Service\Mock\Command\MockCommand;
 use Guzzle\Service\Resource\ResourceIteratorClassFactory;
@@ -394,4 +395,40 @@ class ClientTest extends \Guzzle\Tests\GuzzleTestCase
         $this->assertEquals('bar', $command->get('mesa'));
         $this->assertEquals('test', $command->get('jar'));
     }
+
+    /**
+     * @expectedException \Guzzle\Http\Exception\BadResponseException
+     */
+    public function testWrapsSingleCommandExceptions()
+    {
+        $client = new Mock\MockClient('http://foobaz.com');
+        $mock = new MockPlugin(array(new Response(401)));
+        $client->addSubscriber($mock);
+        $client->execute(new MockCommand());
+    }
+
+    public function testWrapsMultipleCommandExceptions()
+    {
+        $client = new Mock\MockClient('http://foobaz.com');
+        $mock = new MockPlugin(array(new Response(200), new Response(200), new Response(404), new Response(500)));
+        $client->addSubscriber($mock);
+
+        $cmds = array(new MockCommand(), new MockCommand(), new MockCommand(), new MockCommand());
+        try {
+            $client->execute($cmds);
+        } catch (CommandTransferException $e) {
+            $this->assertEquals(2, count($e->getFailedRequests()));
+            $this->assertEquals(2, count($e->getFailedCommands()));
+            $this->assertEquals(2, count($e->getSuccessfulRequests()));
+            $this->assertEquals(2, count($e->getSuccessfulCommands()));
+
+            foreach ($e->getSuccessfulCommands() as $c) {
+                $this->assertTrue($c->getResponse()->isSuccessful());
+            }
+
+            foreach ($e->getFailedCommands() as $c) {
+                $this->assertFalse($c->getRequest()->getResponse()->isSuccessful());
+            }
+        }
+    }
 }

tests/Guzzle/Tests/Service/Exception/CommandTransferExceptionTest.php

@@ -0,0 +1,40 @@
+<?php
+
+namespace Guzzle\Tests\Service\Exception;
+
+use Guzzle\Http\Exception\MultiTransferException;
+use Guzzle\Http\Message\Request;
+use Guzzle\Service\Exception\CommandTransferException;
+use Guzzle\Tests\Service\Mock\Command\MockCommand;
+
+/**
+ * @covers Guzzle\Service\Exception\CommandTransferException
+ */
+class CommandTransferExceptionTest extends \Guzzle\Tests\GuzzleTestCase
+{
+    public function testStoresCommands()
+    {
+        $c1 = new MockCommand();
+        $c2 = new MockCommand();
+        $e = new CommandTransferException('Test');
+        $e->addSuccessfulCommand($c1)->addFailedCommand($c2);
+        $this->assertSame(array($c1), $e->getSuccessfulCommands());
+        $this->assertSame(array($c2), $e->getFailedCommands());
+        $this->assertSame(array($c1, $c2), $e->getAllCommands());
+    }
+
+    public function testConvertsMultiExceptionIntoCommandTransfer()
+    {
+        $r1 = new Request('GET', 'http://foo.com');
+        $r2 = new Request('GET', 'http://foobaz.com');
+        $e = new MultiTransferException('Test', 123);
+        $e->addSuccessfulRequest($r1)->addFailedRequest($r2);
+        $ce = CommandTransferException::fromMultiTransferException($e);
+
+        $this->assertInstanceOf('Guzzle\Service\Exception\CommandTransferException', $ce);
+        $this->assertEquals('Test', $ce->getMessage());
+        $this->assertEquals(123, $ce->getCode());
+        $this->assertSame(array($r1), $ce->getSuccessfulRequests());
+        $this->assertSame(array($r2), $ce->getFailedRequests());
+    }
+}