Added enhanced HttpConnector error reporting and accompanying tests.

Added connection tries getter and setter to HttpConnector and accompanying tests.

Author: Bilge

.travis.yml

@@ -14,10 +14,10 @@ install:
   - alias composer=composer\ -n && composer selfupdate
   - composer validate
   - composer --prefer-source install
-  - composer --prefer-source require satooshi/php-coveralls
 
 script:
   - bin/test --coverage-clover=build/logs/clover.xml
 
 after_success:
+  - composer --prefer-source require satooshi/php-coveralls
   - vendor/bin/coveralls -v

src/Porter/Net/Http/HttpConnector.php

@@ -6,8 +6,16 @@
 use ScriptFUSION\Porter\Options\EncapsulatedOptions;
 use ScriptFUSION\Retry\ErrorHandler\ExponentialBackoffErrorHandler;
 
+/**
+ * Fetches data from an HTTP server via the PHP wrapper.
+ *
+ * Enhanced error reporting is achieved by ignoring HTTP error codes in the wrapper, instead throwing
+ * HttpServerException which includes the body of the response in the error message.
+ */
 class HttpConnector extends CachingConnector
 {
+    const DEFAULT_TRIES = 5;
+
     /** @var HttpOptions */
     private $options;
 
@@ -17,20 +25,35 @@ class HttpConnector extends CachingConnector
     /** @var string */
     private $baseUrl;
 
+    /** @var int */
+    private $tries = self::DEFAULT_TRIES;
+
     public function __construct(HttpOptions $options = null)
     {
         parent::__construct();
 
         $this->options = $options ?: new HttpOptions;
     }
 
+    /**
+     * {@inheritdoc}
+     *
+     * @param string $source Source.
+     * @param EncapsulatedOptions|null $options Optional. Options.
+     *
+     * @return string Response.
+     *
+     * @throws \InvalidArgumentException Options is not an instance of HttpOptions.
+     * @throws HttpConnectionException Failed to connect to source.
+     * @throws HttpServerException Server sent an error code.
+     */
     public function fetchFreshData($source, EncapsulatedOptions $options = null)
     {
         if ($options && !$options instanceof HttpOptions) {
             throw new \InvalidArgumentException('Options must be an instance of HttpOptions.');
         }
 
-        return \ScriptFUSION\Retry\retry(5, function () use ($source, $options) {
+        return \ScriptFUSION\Retry\retry($this->getTries(), function () use ($source, $options) {
             if (false === $response = @file_get_contents(
                 $this->getOrCreateUrlBuilder()->buildUrl(
                     $source,
@@ -39,13 +62,21 @@ public function fetchFreshData($source, EncapsulatedOptions $options = null)
                 ),
                 false,
                 stream_context_create([
-                    'http' => array_merge(
+                    'http' => ['ignore_errors' => true] + array_merge(
                         $this->options->extractHttpContextOptions(),
                         $options ? $options->extractHttpContextOptions() : []
                     ),
                 ])
             )) {
-                throw new HttpConnectionException(error_get_last()['message']);
+                $error = error_get_last();
+                throw new HttpConnectionException($error['message'], $error['type']);
+            }
+
+            $code = explode(' ', $http_response_header[0], 3)[1];
+            if ($code < 200 || $code >= 400) {
+                throw new HttpServerException(
+                    "HTTP server responded with error: \"$http_response_header[0]\".\n\n$response"
+                );
             }
 
             return $response;
@@ -76,4 +107,28 @@ public function setBaseUrl($baseUrl)
 
         return $this;
     }
+
+    /**
+     * Gets the maximum number of fetch attempts
+     *
+     * @return int
+     */
+    public function getTries()
+    {
+        return $this->tries;
+    }
+
+    /**
+     * Sets the maximum number of fetch attempts.
+     *
+     * @param int $tries Maximum fetch attempts.
+     *
+     * @return $this
+     */
+    public function setTries($tries)
+    {
+        $this->tries = max(1, $tries|0);
+
+        return $this;
+    }
 }

src/Porter/Net/Http/HttpServerException.php

@@ -0,0 +1,10 @@
+<?php
+namespace ScriptFUSION\Porter\Net\Http;
+
+/**
+ * The exception that is thrown when the server responds with an error code.
+ */
+class HttpServerException extends \RuntimeException
+{
+    // Intentionally empty.
+}

test/Functional/Porter/Net/Http/HttpConnectorTest.php

@@ -1,25 +1,120 @@
 <?php
 namespace ScriptFUSIONTest\Functional\Porter\Net\Http;
 
+use ScriptFUSION\Porter\Connector\Connector;
+use ScriptFUSION\Porter\Net\Http\HttpConnectionException;
 use ScriptFUSION\Porter\Net\Http\HttpConnector;
 use ScriptFUSION\Porter\Net\Http\HttpOptions;
+use ScriptFUSION\Porter\Net\Http\HttpServerException;
+use ScriptFUSION\Retry\FailingTooHardException;
 use Symfony\Component\Process\Process;
 
 final class HttpConnectorTest extends \PHPUnit_Framework_TestCase
 {
     const HOST = 'localhost:12345';
     const URI = '/test?baz=qux';
 
-    public function testConnectionToLocalWebserver()
+    private static $dir;
+
+    /** @var HttpConnector */
+    private $connector;
+
+    public static function setUpBeforeClass()
+    {
+        self::$dir = __DIR__ . '/servers';
+    }
+
+    protected function setUp()
     {
-        $connector = new HttpConnector((new HttpOptions)->addHeader($header = 'Foo: Bar'));
+        $this->connector = new HttpConnector;
+    }
 
-        $process = (new Process(sprintf('php -S %s feedback.php', self::HOST)))->setWorkingDirectory(__DIR__);
-        $process->start();
-        $response = $connector->fetch('http://' . self::HOST . self::URI);
-        $process->stop();
+    public function testConnectionToLocalWebserver()
+    {
+        $server = $this->startServer('feedback');
+        $response = $this->fetch(new HttpConnector((new HttpOptions)->addHeader($header = 'Foo: Bar')));
+        $this->stopServer($server);
 
         self::assertRegExp('[\AGET \Q' . self::HOST . self::URI . '\E HTTP/\d+\.\d+$]m', $response);
         self::assertRegExp("[^$header$]m", $response);
     }
+
+    public function testConnectionTimeout()
+    {
+        try {
+            $this->fetch($this->connector->setTries(1));
+        } catch (FailingTooHardException $exception) {
+            self::assertInstanceOf(HttpConnectionException::class, $exception->getPrevious());
+
+            return;
+        }
+
+        self::fail('Expected exception not thrown.');
+    }
+
+    public function testErrorResponse()
+    {
+        $server = $this->startServer('404');
+        try {
+            $this->fetch();
+        } catch (FailingTooHardException $exception) {
+            self::assertInstanceOf(HttpServerException::class, $exception->getPrevious(), $exception->getMessage());
+            self::assertStringEndsWith('foo', $exception->getPrevious()->getMessage());
+
+            return;
+        } finally {
+            $this->stopServer($server);
+        }
+
+        self::fail('Expected exception not thrown.');
+    }
+
+    public function testOneTry()
+    {
+        $this->setExpectedException(FailingTooHardException::class, '1');
+
+        $this->fetch($this->connector->setTries(1));
+    }
+
+    public function testDefaultTries()
+    {
+        $this->setExpectedException(FailingTooHardException::class, (string)HttpConnector::DEFAULT_TRIES);
+
+        $this->fetch();
+    }
+
+    /**
+     * @param string $script
+     *
+     * @return Process
+     */
+    private function startServer($script)
+    {
+        $server = (new Process(sprintf('php -S %s %s.php', self::HOST, $script)))->setWorkingDirectory(self::$dir);
+        $server->start();
+
+        return $server;
+    }
+
+    private function stopServer(Process $server)
+    {
+        /*
+         * Bizarrely, process IDs are one less than they should be on Travis.
+         * See https://github.com/symfony/symfony/issues/19611
+         */
+        if (array_key_exists('TRAVIS', $_SERVER)) {
+            posix_kill($server->getPid() + 1, SIGINT);
+
+            return;
+        }
+
+        $server->stop();
+    }
+
+    private function fetch(Connector $connector = null)
+    {
+        $connector = $connector ?: $this->connector;
+
+        return $connector->fetch('http://' . self::HOST . self::URI);
+    }
 }

test/Functional/Porter/Net/Http/servers/404.php

@@ -0,0 +1,4 @@
+<?php
+http_response_code(404);
+
+echo 'foo';