Cleaning up ring responses, adding guard to ensure toString does not throw

src/Client.php

@@ -249,8 +249,7 @@ class Client implements ClientInterface
 
         // Future responses do not need to process right away.
         if ($response instanceof RingFutureInterface) {
-            $trans->response = $this->createFutureResponse($trans, $response);
-            return $trans->response;
+            return $this->createFutureResponse($trans, $response);
         }
 
         // Throw a wrapped exception if the transactions has an error.
@@ -266,33 +265,6 @@ class Client implements ClientInterface
         throw RingBridge::getNoRingResponseException($trans->request);
     }
 
-    private function createFutureResponse(
-        Transaction $trans,
-        RingFutureInterface $response
-    ) {
-        // Create a future response that's hooked up to the ring future.
-        return new FutureResponse(
-            // Dereference function
-            function () use ($response, $trans) {
-                // Dereference the underlying future and block until complete.
-                $response->deref();
-                // Throw an exception if present. Remove them to prevent this.
-                if ($trans->exception) {
-                    throw RequestEvents::wrapException($trans->request, $trans->exception);
-                }
-                // No exception, so the transaction should have a response.
-                if ($trans->response) {
-                    return $trans->response;
-                }
-                throw RingBridge::getNoRingResponseException($trans->request);
-            },
-            // Cancel function. Just proxy to the underlying future.
-            function () use ($response) {
-                return $response->cancel();
-            }
-        );
-    }
-
     /**
      * Get an array of default options to apply to the client
      *
@@ -319,6 +291,33 @@ class Client implements ClientInterface
         return $settings;
     }
 
+    private function createFutureResponse(
+        Transaction $trans,
+        RingFutureInterface $response
+    ) {
+        // Create a future response that's hooked up to the ring future.
+        return new FutureResponse(
+            // Dereference function
+            function () use ($response, $trans) {
+                $response->deref();
+                // Throw an exception if present. You need to remove transaction
+                // exceptions to prevent them from being thrown.
+                if ($trans->exception) {
+                    throw RequestEvents::wrapException($trans->request, $trans->exception);
+                } elseif ($trans->response) {
+                    // Return the response if one was set on the transaction.
+                    return $trans->response;
+                }
+                // If we know that the events were fired, then there's a problem.
+                throw RingBridge::getNoRingResponseException($trans->request);
+            },
+            // Cancel function. Just proxy to the underlying future.
+            function () use ($response) {
+                return $response->cancel();
+            }
+        );
+    }
+
     /**
      * Expand a URI template and inherit from the base URL if it's relative
      *
@@ -328,15 +327,19 @@ class Client implements ClientInterface
      */
     private function buildUrl($url)
     {
+        // URI template (absolute or relative)
         if (!is_array($url)) {
-            if (strpos($url, '://')) {
-                return (string) $url;
-            }
-            return (string) $this->baseUrl->combine($url);
-        } elseif (strpos($url[0], '://')) {
+            return strpos($url, '://')
+                ? (string) $url
+                : (string) $this->baseUrl->combine($url);
+        }
+
+        // Absolute URL
+        if (strpos($url[0], '://')) {
             return Utils::uriTemplate($url[0], $url[1]);
         }
 
+        // Combine the relative URL with the base URL
         return (string) $this->baseUrl->combine(
             Utils::uriTemplate($url[0], $url[1])
         );

src/Message/FutureResponse.php

@@ -56,7 +56,12 @@ class FutureResponse implements ResponseInterface, FutureInterface
 
     public function __toString()
     {
-        return $this->result->__toString();
+        try {
+            return $this->result->__toString();
+        } catch (\Exception $e) {
+            trigger_error($e->getMessage(), E_USER_WARNING);
+            return '';
+        }
     }
 
     public function getProtocolVersion()

src/RingBridge.php

@@ -131,8 +131,6 @@ class RingBridge
             ? $response['transfer_info'] : [];
 
         if (!empty($response['status'])) {
-            // Transition to "error" if an error is present. Otherwise, complete.
-            $trans->state = isset($response['error']) ? 'error' : 'complete';
             $options = [];
             if (isset($response['version'])) {
                 $options['protocol_version'] = $response['version'];
@@ -142,7 +140,7 @@ class RingBridge
             }
             $trans->response = $messageFactory->createResponse(
                 $response['status'],
-                $response['headers'],
+                isset($response['headers']) ? $response['headers'] : [],
                 isset($response['body']) ? $response['body'] : null,
                 $options
             );
@@ -201,11 +199,14 @@ class RingBridge
      */
     public static function getNoRingResponseException(RequestInterface $request)
     {
-        return new RequestException(
-            'Sending the request did not return a response, exception, or '
-            . 'populate the transaction with a response. This is most likely '
-            . 'due to an incorrectly implemented Guzzle Ring adapter.',
-            $request
-        );
+        $message = <<<EOT
+Sending the request did not return a response, exception, or populate the
+transaction with a response. This is most likely due to an incorrectly
+implemented Guzzle-Ring adapter that is not calling the "then" function of a
+request array when the response is ready. If you are simply trying to mock
+responses, then it is recommneded to use the
+GuzzleHttp\Ring\Client\MockAdapter.
+EOT;
+        return new RequestException($message, $request);
     }
 }

tests/ClientTest.php

@@ -329,7 +329,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase
 
     /**
      * @expectedException \GuzzleHttp\Exception\RequestException
-     * @expectedExceptionMessage incorrectly implemented Guzzle Ring adapter
+     * @expectedExceptionMessage not calling the "then"
      */
     public function testEnsuresResponseIsPresentAfterSending()
     {
@@ -340,7 +340,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase
 
     /**
      * @expectedException \GuzzleHttp\Exception\RequestException
-     * @expectedExceptionMessage incorrectly implemented Guzzle Ring adapter
+     * @expectedExceptionMessage not calling the "then"
      */
     public function testEnsuresResponseIsPresentAfterDereferencing()
     {

tests/Message/FutureResponseTest.php

@@ -131,4 +131,16 @@ class FutureResponseTest extends \PHPUnit_Framework_TestCase
         $this->assertFalse($future->cancel());
         $future->getStatusCode();
     }
+
+    public function testExceptionInToStringTriggersError()
+    {
+        $future = new FutureResponse(function () {});
+        $err = '';
+        set_error_handler(function () use (&$err) {
+            $err = func_get_args()[1];
+        });
+        echo $future;
+        restore_error_handler();
+        $this->assertContains('Future did not return a valid response', $err);
+    }
 }