Removing unneeded functions from Events to intercept with exceptions. Moving exception handling from events for before send to the client.

src/Guzzle/Http/Client.php

@@ -4,8 +4,12 @@ namespace Guzzle\Http;
 
 use Guzzle\Common\Collection;
 use Guzzle\Common\HasDispatcherTrait;
+use Guzzle\Http\Adapter\TransactionInterface;
 use Guzzle\Http\Event\ClientCreateRequestEvent;
 use Guzzle\Http\Event\ClientEvents;
+use Guzzle\Http\Event\RequestEvents;
+use Guzzle\Http\Event\RequestErrorEvent;
+use Guzzle\Http\Exception\RequestException;
 use Guzzle\Http\Message\FutureResponseInterface;
 use Guzzle\Version;
 use Guzzle\Http\Adapter\AdapterInterface;
@@ -19,7 +23,6 @@ use Guzzle\Http\Message\MessageFactory;
 use Guzzle\Http\Message\MessageFactoryInterface;
 use Guzzle\Http\Message\RequestInterface;
 use Guzzle\Url\Url;
-use Guzzle\Url\UriTemplate;
 
 /**
  * HTTP client
@@ -187,23 +190,49 @@ class Client implements ClientInterface
     public function send(RequestInterface $request)
     {
         $transaction = new Transaction($this, $request, $this->messageFactory);
+        $send = false;
 
-        if (!$request->getEventDispatcher()->dispatch(
-            'request.before_send',
+        try {
+            $send = !$request->getEventDispatcher()->dispatch(
+                RequestEvents::BEFORE_SEND,
                 new RequestBeforeSendEvent($transaction)
-        )->isPropagationStopped()) {
+            )->isPropagationStopped();
+        } catch (\Exception $e) {
+            $this->handleSendError($e, $request, $transaction);
+        }
+
+        if ($send) {
             $this->adapter->send($transaction);
         }
 
-        $response = $transaction->getResponse();
-
-        if (!($response instanceof FutureResponseInterface) && !$response->getEffectiveUrl()) {
+        if (!($response = $transaction->getResponse())) {
+            throw new \RuntimeException('No response was associated with the transaction');
+        } elseif (!($response instanceof FutureResponseInterface) && !$response->getEffectiveUrl()) {
             $response->setEffectiveUrl($request->getUrl());
         }
 
         return $response;
     }
 
+    /**
+     * Handle a client error
+     */
+    private function handleSendError(\Exception $e, RequestInterface $request, TransactionInterface $transaction)
+    {
+        // Convert non-request exception to a wrapped exception
+        if (!($e instanceof RequestException)) {
+            $e = new RequestException($e->getMessage(), $request, null, $e);
+        }
+
+        // Dispatch an event and allow interception
+        if (!$transaction->getRequest()->getEventDispatcher()->dispatch(
+            RequestEvents::ERROR,
+            new RequestErrorEvent($transaction, $e)
+        )->isPropagationStopped()) {
+            throw $e;
+        }
+    }
+
     /**
      * Get an array of default options to apply to the client
      *

src/Guzzle/Http/Event/AbstractRequestEvent.php

@@ -5,7 +5,6 @@ namespace Guzzle\Http\Event;
 use Guzzle\Common\Event;
 use Guzzle\Http\Adapter\TransactionInterface;
 use Guzzle\Http\ClientInterface;
-use Guzzle\Http\Exception\RequestException;
 use Guzzle\Http\Message\RequestInterface;
 
 abstract class AbstractRequestEvent extends Event
@@ -49,24 +48,13 @@ abstract class AbstractRequestEvent extends Event
         return $this->transaction;
     }
 
-    /**
-     * Emit an error event
-     */
-    protected function emitError(RequestException $exception)
-    {
-        $this->transaction->getRequest()->getEventDispatcher()->dispatch(
-            'request.error',
-            new RequestErrorEvent($this->transaction, $exception)
-        );
-    }
-
     /**
      * Emit an after_send event
      */
     protected function emitAfterSend()
     {
         $this->transaction->getRequest()->getEventDispatcher()->dispatch(
-            'request.after_send',
+            RequestEvents::AFTER_SEND,
             new RequestAfterSendEvent($this->transaction)
         );
     }

src/Guzzle/Http/Event/RequestAfterSendEvent.php

@@ -2,30 +2,26 @@
 
 namespace Guzzle\Http\Event;
 
-use Guzzle\Http\Exception\RequestException;
 use Guzzle\Http\Message\ResponseInterface;
 
 /**
  * Event object emitted after a request has been sent.
  *
- * You may change the result value associated with a request using the setResult() method of the event.
+ * You may change the Response associated with the request using the
+ * intercept() method of the event.
  */
 class RequestAfterSendEvent extends AbstractRequestEvent
 {
     /**
-     * Intercept the request and associate aa response or exception
+     * Intercept the request and associate a response
      *
-     * @param ResponseInterface|RequestException $result Result to set for the request
+     * @param ResponseInterface $response Response to set
      */
-    public function intercept($result)
+    public function intercept(ResponseInterface $response)
     {
-        if ($result instanceof RequestException) {
-            $this->emitError($result);
-        } else {
-            $this->getTransaction()->setResponse($result);
+        $this->getTransaction()->setResponse($response);
         $this->stopPropagation();
     }
-    }
 
     /**
      * Get the response of the request

src/Guzzle/Http/Event/RequestBeforeSendEvent.php

@@ -3,31 +3,24 @@
 namespace Guzzle\Http\Event;
 
 use Guzzle\Http\Message\ResponseInterface;
-use Guzzle\Http\Exception\RequestException;
 
 /**
  * Event object emitted before a request is sent.
  *
- * You can intercept a request and inject a response or exception onto the event object. Intercepting a request from an
- * event listener will prevent the client from sending the request over the wire. The injected response will then be
- * used as the result of the request.
+ * You may change the Response associated with the request using the
+ * intercept() method of the event.
  */
 class RequestBeforeSendEvent extends AbstractRequestEvent
 {
     /**
-     * Intercept the request and inject a response or exception
+     * Intercept the request and associate a response
      *
-     * @param ResponseInterface|RequestException $result Response or Exception to set
+     * @param ResponseInterface $response Response to set
      */
-    public function intercept($result)
+    public function intercept(ResponseInterface $response)
     {
+        $this->getTransaction()->setResponse($response);
         $this->stopPropagation();
-
-        if ($result instanceof ResponseInterface) {
-            $this->getTransaction()->setResponse($result);
         $this->emitAfterSend();
-        } else {
-            $this->emitError($result);
-        }
     }
 }

tests/Guzzle/Tests/Http/ClientTest.php

@@ -8,6 +8,7 @@ use Guzzle\Http\Event\ClientEvents;
 use Guzzle\Http\Event\RequestBeforeSendEvent;
 use Guzzle\Http\Event\RequestEvents;
 use Guzzle\Http\Message\Response;
+use Guzzle\Http\Exception\RequestException;
 
 /**
  * @covers Guzzle\Http\Client
@@ -259,4 +260,55 @@ class ClientTest extends \PHPUnit_Framework_TestCase
         $this->assertSame($response2, $client->get('http://test.com'));
         $this->assertEquals('http://test.com', $response2->getEffectiveUrl());
     }
+
+    /**
+     * @expectedException \RuntimeException
+     * @expectedExceptionMessage No response
+     */
+    public function testEnsuresResponseIsPresentAfterSending()
+    {
+        $client = new Client();
+        $client->getEventDispatcher()->addListener(RequestEvents::BEFORE_SEND, function ($e) {
+            $e->stopPropagation();
+        });
+        $client->get('/');
+    }
+
+    public function testClientHandlesErrorsDuringBeforeSend()
+    {
+        $client = new Client();
+        $client->getEventDispatcher()->addListener(RequestEvents::BEFORE_SEND, function ($e) {
+            throw new RequestException('foo', $e->getRequest());
+        });
+        $client->getEventDispatcher()->addListener(RequestEvents::ERROR, function ($e) {
+            $e->intercept(new Response(200));
+        });
+        $this->assertEquals(200, $client->get('/')->getStatusCode());
+    }
+
+    /**
+     * @expectedException \Guzzle\Http\Exception\RequestException
+     * @expectedExceptionMessage foo
+     */
+    public function testClientHandlesErrorsDuringBeforeSendAndThrowsIfUnhandled()
+    {
+        $client = new Client();
+        $client->getEventDispatcher()->addListener(RequestEvents::BEFORE_SEND, function ($e) {
+            throw new RequestException('foo', $e->getRequest());
+        });
+        $client->get('/');
+    }
+
+    /**
+     * @expectedException \Guzzle\Http\Exception\RequestException
+     * @expectedExceptionMessage foo
+     */
+    public function testClientHandlesErrorsDuringBeforeSendAndThrowsIfUnhandledAndWrapsThem()
+    {
+        $client = new Client();
+        $client->getEventDispatcher()->addListener(RequestEvents::BEFORE_SEND, function ($e) {
+            throw new \Exception('foo');
+        });
+        $client->get('/');
+    }
 }