From 080c82fb04ada925974b26cad7db14f8b0e25eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sat, 13 Jun 2015 20:26:09 +0200 Subject: [PATCH 1/3] Reject error response codes with a ResponseException which allows access to its underlying Response object --- src/Io/Transaction.php | 3 ++- src/Message/ResponseException.php | 32 +++++++++++++++++++++++++++++++ tests/FunctionalBrowserTest.php | 11 +++++++++-- tests/Io/TransactionTest.php | 27 ++++++++++++++++++++++++++ tests/bootstrap.php | 10 ++++++++++ 5 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 src/Message/ResponseException.php create mode 100644 tests/Io/TransactionTest.php diff --git a/src/Io/Transaction.php b/src/Io/Transaction.php index 4072e48..0c12a42 100644 --- a/src/Io/Transaction.php +++ b/src/Io/Transaction.php @@ -8,6 +8,7 @@ use Clue\React\Buzz\Browser; use React\HttpClient\Client as HttpClient; use Clue\React\Buzz\Io\Sender; +use Clue\React\Buzz\Message\ResponseException; class Transaction { @@ -79,7 +80,7 @@ public function onResponse(Response $response, Request $request) // only status codes 200-399 are considered to be valid, reject otherwise if ($this->obeySuccessCode && ($response->getCode() < 200 || $response->getCode() >= 400)) { - throw new \RuntimeException('HTTP status code ' . $response->getCode() . ' (' . $response->getReasonPhrase() . ')', $response->getCode()); + throw new ResponseException($response, 'HTTP status code ' . $response->getCode() . ' (' . $response->getReasonPhrase() . ')', $response->getCode()); } // resolve our initial promise diff --git a/src/Message/ResponseException.php b/src/Message/ResponseException.php new file mode 100644 index 0000000..566b23a --- /dev/null +++ b/src/Message/ResponseException.php @@ -0,0 +1,32 @@ +response = $response; + } + + /** + * get Response message object + * + * @return Response + */ + public function getResponse() + { + return $this->response; + } +} diff --git a/tests/FunctionalBrowserTest.php b/tests/FunctionalBrowserTest.php index 871cd1e..858f798 100644 --- a/tests/FunctionalBrowserTest.php +++ b/tests/FunctionalBrowserTest.php @@ -56,9 +56,16 @@ public function testInvalidPort() $this->loop->run(); } - public function testInvalidPath() + public function testErrorStatusCodeRejectsWithResponseException() { - $this->expectPromiseReject($this->browser->get($this->base . 'status/404')); + $that = $this; + $this->expectPromiseReject($this->browser->get($this->base . 'status/404'))->then(null, function ($e) use ($that) { + $that->assertInstanceOf('Clue\Buzz\React\Message\ResponseException', $e); + $that->assertEquals(404, $e->getCode()); + + $that->assertInstanceOf('Clue\Buzz\React\Message\Response', $e->getResponse()); + $that->assertEquals(404, $e->getResponse()->getCode()); + }); $this->loop->run(); } diff --git a/tests/Io/TransactionTest.php b/tests/Io/TransactionTest.php new file mode 100644 index 0000000..f0e999c --- /dev/null +++ b/tests/Io/TransactionTest.php @@ -0,0 +1,27 @@ +getMockBuilder('Clue\React\Buzz\Message\Request')->disableOriginalConstructor()->getMock(); + $response = new Response('HTTP/1.0', 404, 'File not found'); + + // mock sender to resolve promise with the given $response in response to the given $request + $sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + $sender->expects($this->once())->method('send')->with($this->equalTo($request))->will($this->returnValue($this->createPromiseResolved($response))); + + $transaction = new Transaction($request, $sender); + $promise = $transaction->send(); + + $that = $this; + $this->expectPromiseReject($promise)->then(null, function ($exception) use ($that, $response) { + $that->assertInstanceOf('Clue\React\Buzz\Message\ResponseException', $exception); + $that->assertEquals(404, $exception->getCode()); + $that->assertSame($response, $exception->getResponse()); + }); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 39fcc2c..f45ddf2 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -1,5 +1,7 @@ resolve($value); + + return $deferred->promise(); + } } class CallableStub From bc05b16901bd77337fb0d16c8c16bf3cc2978166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sat, 13 Jun 2015 22:07:32 +0200 Subject: [PATCH 2/3] Simplify ResponseException by moving parameter logic to its ctor --- src/Io/Transaction.php | 2 +- src/Message/ResponseException.php | 6 ++++++ tests/Message/ResponseExceptionTest.php | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 tests/Message/ResponseExceptionTest.php diff --git a/src/Io/Transaction.php b/src/Io/Transaction.php index 0c12a42..62e36bf 100644 --- a/src/Io/Transaction.php +++ b/src/Io/Transaction.php @@ -80,7 +80,7 @@ public function onResponse(Response $response, Request $request) // only status codes 200-399 are considered to be valid, reject otherwise if ($this->obeySuccessCode && ($response->getCode() < 200 || $response->getCode() >= 400)) { - throw new ResponseException($response, 'HTTP status code ' . $response->getCode() . ' (' . $response->getReasonPhrase() . ')', $response->getCode()); + throw new ResponseException($response); } // resolve our initial promise diff --git a/src/Message/ResponseException.php b/src/Message/ResponseException.php index 566b23a..e11c137 100644 --- a/src/Message/ResponseException.php +++ b/src/Message/ResponseException.php @@ -15,6 +15,12 @@ class ResponseException extends RuntimeException public function __construct(Response $response, $message = null, $code = null, $previous = null) { + if ($message === null) { + $message = 'HTTP status code ' . $response->getCode() . ' (' . $response->getReasonPhrase() . ')'; + } + if ($code === null) { + $code = $response->getCode(); + } parent::__construct($message, $code, $previous); $this->response = $response; diff --git a/tests/Message/ResponseExceptionTest.php b/tests/Message/ResponseExceptionTest.php new file mode 100644 index 0000000..2cb6855 --- /dev/null +++ b/tests/Message/ResponseExceptionTest.php @@ -0,0 +1,18 @@ +assertEquals(404, $e->getCode()); + $this->assertEquals('HTTP status code 404 (File not found)', $e->getMessage()); + + $this->assertSame($response, $e->getResponse()); + } +} From 7f38142d306e7b4ea0a0fc1fedd417c7f8369167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sat, 13 Jun 2015 22:12:38 +0200 Subject: [PATCH 3/3] Documentation for ResponseException --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index e25c1ff..9f17fc1 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,17 @@ It shares all properties of the [`Message`](#message) parent class. The `Request` value object represents the outgoing request to be sent via the [`Browser`](#browser). It shares all properties of the [`Message`](#message) parent class. +### ResponseException + +The `ResponseException` is an `Exception` sub-class that will be used to reject +a request promise if the remote server returns a non-success status code +(anything but 2xx or 3xx). +You can control this behavior via the ["obeySuccessCode" option](#options). + +The `getCode()` method can be used to return the HTTP response status code. + +The `getResponse()` method can be used to access its underlying [`Response`](#response) object. + ## Advanced ### Sender