Fix handling of invalid $reason and rewrote PromiseExceptionTests (#63)

* Reason callback will not use a reason that is not a Throwable as root exception
* Use data provider for tests of PromiseExceptionTest

Author: gmponos

src/Promise.php

@@ -61,10 +61,10 @@ public function __construct(PromiseInterface $promise, RequestInterface $request
                 $this->exception = $reason;
             } elseif ($reason instanceof GuzzleExceptions\GuzzleException) {
                 $this->exception = $this->handleException($reason, $request);
-            } elseif ($reason instanceof \Exception) {
+            } elseif ($reason instanceof \Throwable) {
                 $this->exception = new \RuntimeException('Invalid exception returned from Guzzle6', 0, $reason);
             } else {
-                $this->exception = new \UnexpectedValueException('Reason returned from Guzzle6 must be an Exception', 0, $reason);
+                $this->exception = new \UnexpectedValueException('Reason returned from Guzzle6 must be an Exception');
             }
 
             throw $this->exception;

tests/PromiseExceptionTest.php

@@ -1,60 +1,62 @@
 <?php
 
+declare(strict_types=1);
+
 namespace Http\Adapter\Guzzle6\Tests;
 
 use GuzzleHttp\Exception as GuzzleExceptions;
 use Http\Adapter\Guzzle6\Promise;
+use Http\Client\Exception\HttpException;
+use Http\Client\Exception\NetworkException;
+use Http\Client\Exception\RequestException;
+use Http\Client\Exception\TransferException;
 use PHPUnit\Framework\TestCase;
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
 
 /**
  * @author Tobias Nyholm <[email protected]>
+ * @author George Mponos <[email protected]>
  */
-class PromiseExceptionTest extends TestCase
+final class PromiseExceptionTest extends TestCase
 {
-    public function testGetException()
-    {
-        $request = $this->getMockBuilder('Psr\Http\Message\RequestInterface')->getMock();
-        $response = $this->getMockBuilder('Psr\Http\Message\ResponseInterface')->getMock();
-        $promise = $this->getMockBuilder('GuzzleHttp\Promise\PromiseInterface')->getMock();
-
-        $adapter = new Promise($promise, $request);
-        $method = new \ReflectionMethod('Http\Adapter\Guzzle6\Promise', 'handleException');
-        $method->setAccessible(true);
-
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\ConnectException('foo', $request), $request);
-        $this->assertInstanceOf('Http\Client\Exception\NetworkException', $outputException, "Guzzle's ConnectException should be converted to a NetworkException");
-
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\TooManyRedirectsException('foo', $request), $request);
-        $this->assertInstanceOf('Http\Client\Exception\RequestException', $outputException, "Guzzle's TooManyRedirectsException should be converted to a RequestException");
-
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\RequestException('foo', $request, $response), $request);
-        $this->assertInstanceOf('Http\Client\Exception\HttpException', $outputException, "Guzzle's RequestException should be converted to a HttpException");
-
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\BadResponseException('foo', $request, $response), $request);
-        $this->assertInstanceOf('Http\Client\Exception\HttpException', $outputException, "Guzzle's BadResponseException should be converted to a HttpException");
-
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\ClientException('foo', $request, $response), $request);
-        $this->assertInstanceOf('Http\Client\Exception\HttpException', $outputException, "Guzzle's ClientException should be converted to a HttpException");
-
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\ServerException('foo', $request, $response), $request);
-        $this->assertInstanceOf('Http\Client\Exception\HttpException', $outputException, "Guzzle's ServerException should be converted to a HttpException");
-
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\TransferException('foo'), $request);
-        $this->assertInstanceOf('Http\Client\Exception\TransferException', $outputException, "Guzzle's TransferException should be converted to a TransferException");
-
-        /*
-         * Test RequestException without response
-         */
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\RequestException('foo', $request), $request);
-        $this->assertInstanceOf('Http\Client\Exception\RequestException', $outputException, "Guzzle's RequestException with no response should be converted to a RequestException");
-
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\BadResponseException('foo', $request), $request);
-        $this->assertInstanceOf('Http\Client\Exception\RequestException', $outputException, "Guzzle's BadResponseException with no response should be converted to a RequestException");
-
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\ClientException('foo', $request), $request);
-        $this->assertInstanceOf('Http\Client\Exception\RequestException', $outputException, "Guzzle's ClientException with no response should be converted to a RequestException");
+    /**
+     * @dataProvider exceptionThatIsThrownForGuzzleExceptionProvider
+     */
+    public function testExceptionThatIsThrownForGuzzleException(
+        RequestInterface $request,
+        $reason,
+        string $adapterExceptionClass
+    ) {
+        $guzzlePromise = new \GuzzleHttp\Promise\Promise();
+        $guzzlePromise->reject($reason);
+        $promise = new Promise($guzzlePromise, $request);
+        $this->expectException($adapterExceptionClass);
+        $promise->wait();
+    }
 
-        $outputException = $method->invoke($adapter, new GuzzleExceptions\ServerException('foo', $request), $request);
-        $this->assertInstanceOf('Http\Client\Exception\RequestException', $outputException, "Guzzle's ServerException with no response should be converted to a RequestException");
+    public function exceptionThatIsThrownForGuzzleExceptionProvider(): array
+    {
+        $request = $this->getMockBuilder(RequestInterface::class)->getMock();
+        $response = $this->getMockBuilder(ResponseInterface::class)->getMock();
+
+        return [
+            [$request, new GuzzleExceptions\ConnectException('foo', $request), NetworkException::class],
+            [$request, new GuzzleExceptions\TooManyRedirectsException('foo', $request), RequestException::class],
+            [$request, new GuzzleExceptions\RequestException('foo', $request, $response), HttpException::class],
+            [$request, new GuzzleExceptions\BadResponseException('foo', $request, $response), HttpException::class],
+            [$request, new GuzzleExceptions\ClientException('foo', $request, $response), HttpException::class],
+            [$request, new GuzzleExceptions\ServerException('foo', $request, $response), HttpException::class],
+            [$request, new GuzzleExceptions\TransferException('foo'), TransferException::class],
+            // check cases without response
+            [$request, new GuzzleExceptions\RequestException('foo', $request), RequestException::class],
+            [$request, new GuzzleExceptions\BadResponseException('foo', $request), RequestException::class],
+            [$request, new GuzzleExceptions\ClientException('foo', $request), RequestException::class],
+            [$request, new GuzzleExceptions\ServerException('foo', $request), RequestException::class],
+            // Non PSR-18 Exceptions thrown
+            [$request, new \Exception('foo'), \RuntimeException::class],
+            [$request, new \Error('foo'), \RuntimeException::class],
+            [$request, 'whatever', \UnexpectedValueException::class],
+        ];
     }
 }