Adding timeout options both CLI and .cigar.json (#32)

* Adding timeout options both CLI and .cigar.json 🎆🎆🎆 Happy New Year 🎆🎆🎆 This PR adds the timeout options discussed in #31 Closes / implements #31 Any values in the CLI are overridden by what ever value is in the `.cigar.json` * Removed short hand (ct) for --connect-timeout via #32 (comment) by @Brunty * Test response timeout .json config and CLI flags through stubs via #32 (comment) by @Brunty * s is now short the connect-timeout
Brunty · Mar 12, 2019 · 6389b9e · 6389b9e
1 parent 06db910
commit 6389b9e
Show file tree

Hide file tree

Showing 11 changed files with 154 additions and 20 deletions.
diff --git a/.cigar.json.example b/.cigar.json.example
@@ -21,5 +21,17 @@
     "url": "http://httpbin.org/robots.txt",
     "status": 200,
     "content-type": "text/plain"
+  },
+  {
+    "url": "http://httpbin.org/delay/3",
+    "status": 200,
+    "request-timeout": 1,
+    "response-timeout": 5
+  },
+  {
+    "url": "http://httpbin.org/drip?duration=3",
+    "status": 200,
+    "request-timeout": 1,
+    "response-timeout": 5
   }
 ]
diff --git a/bin/cigar b/bin/cigar
@@ -18,7 +18,7 @@ use Brunty\Cigar\Outputter;
 use Brunty\Cigar\Parser;
 use Brunty\Cigar\Result;
 
-$options = getopt('c:ia:u:jh:', ['version', 'help', 'quiet', 'config:', 'insecure', 'auth:', 'url:', 'json', 'header:']);
+$options = getopt('c:ia:u:jh:t:s:', ['version', 'help', 'quiet', 'config:', 'insecure', 'auth:', 'url:', 'json', 'header:', 'timeout:', 'connect-timeout:']);
 
 if (isset($options['help'])) {
     $content = <<<HELP
@@ -27,14 +27,16 @@ if (isset($options['help'])) {
   cigar [options]
 
 \033[33mOptions:\033[0m
-      \033[32m-c file.json, --config=file.json\033[0m   Use the specified config file instead of the default .cigar.json file 
-      \033[32m-u URL,       --url=URL\033[0m            Base URL for checks, e.g. https://example.org/
-      \033[32m-i,           --insecure\033[0m           Allow invalid SSL certificates
-      \033[32m-a,           --auth\033[0m               Authorization header "\074type\076 \074credentials\076"
-      \033[32m-h,           --header\033[0m             Custom header "\074name\076: \074value\076"
-      \033[32m-j,           --json\033[0m               Output JSON
-      \033[32m              --quiet\033[0m              Do not output any message
-      \033[32m              --version\033[0m            Print the version of Cigar 
+      \033[32m-c file.json, --config=file.json\033[0m         Use the specified config file instead of the default .cigar.json file 
+      \033[32m-u URL,       --url=URL\033[0m                  Base URL for checks, e.g. https://example.org/
+      \033[32m-i,           --insecure\033[0m                 Allow invalid SSL certificates
+      \033[32m-a,           --auth\033[0m                     Authorization header "\074type\076 \074credentials\076"
+      \033[32m-h,           --header\033[0m                   Custom header "\074name\076: \074value\076"
+      \033[32m-s            --connect-timeout=TIMEOUT\033[0m  Connect Timeout
+      \033[32m-t,           --timeout=TIMEOUT\033[0m          Timeout
+      \033[32m-j,           --json\033[0m                     Output JSON
+      \033[32m              --quiet\033[0m                    Do not output any message
+      \033[32m              --version\033[0m                  Print the version of Cigar 
 
 Created by Matt Brunt
 E: matt@mfyu.co.uk
@@ -78,17 +80,19 @@ if ( ! file_exists($file)) {
     exit(1);
 }
 
+$secure = ! (isset($options['i']) || isset($options['insecure']));
+$authorization = $options['a'] ?? $options['auth'] ?? null;
+$headers = (array) ($options['h'] ?? $options['header'] ?? []);
+$connectTimeout = $options['s'] ?? $options['connect-timeout'] ?? null;
+$timeout = $options['t'] ?? $options['timeout'] ?? null;
+
 try {
-    $domains = (new Parser($baseUrl))->parse($file);
+    $domains = (new Parser($baseUrl, $connectTimeout, $timeout))->parse($file);
 } catch (\Throwable $e) {
     $outputter->writeErrorLine('Unable to parse .cigar.json file');
     exit(1);
 }
 
-$secure = ! (isset($options['i']) || isset($options['insecure']));
-$authorization = $options['a'] ?? $options['auth'] ?? null;
-$headers = (array) ($options['h'] ?? $options['header'] ?? []);
-
 $results = (new AsyncChecker($secure, $authorization, $headers))->check($domains);
 $passedResults = array_filter($results, function (Result $result) {
     return $result->hasPassed();

diff --git a/spec/AsyncCheckerSpec.php b/spec/AsyncCheckerSpec.php
@@ -62,4 +62,20 @@
             expect($results[0]->getStatusCode())->toEqual($expected[0]->getStatusCode());
         });
     });
+
+    context('when timeouts are set', function () {
+        it('checks a URL that will timeout', function () {
+            // Need to change for a better setup URL that doesn't default to a potentially unknown site
+            $domain = new Url('https://httpbin.org/delay/3', 200, null, null, 1, 1);
+            $domains = [$domain];
+
+            $results = (new AsyncChecker())->check($domains);
+
+            $expected = [
+                new Result($domain, 0),
+            ];
+
+            expect($results[0]->getStatusCode())->toEqual($expected[0]->getStatusCode());
+        });
+    });
 });
diff --git a/spec/CigarCliSpec.php b/spec/CigarCliSpec.php
@@ -87,4 +87,11 @@
 
         expect($process->getExitCode())->toBe(0);
     });
+
+    it('can be passed timeout arguments and it overwrites the configured value in .cigar.json', function () {
+        $process = new Process('cd spec && cp stubs/.cigar.timeouts.json .cigar.json && ../bin/cigar -t 1');
+        $process->run();
+
+        expect($process->getExitCode())->toBe(1);
+    });
 });
diff --git a/spec/ParserSpec.php b/spec/ParserSpec.php
@@ -19,7 +19,10 @@
   {
     "url": "http://httpbin.org/status/418",
     "status": 418,
-    "content": "teapot"
+    "content": "teapot",
+    "content-type": "kitchen/teapot",
+    "connect-timeout": 1,
+    "timeout": 2
   }
 ]
 ',
@@ -31,7 +34,7 @@
         $expected = [
             new Url('http://httpbin.org/status/418', 418),
             new Url('http://httpbin.org/status/200', 200),
-            new Url('http://httpbin.org/status/418', 418, 'teapot'),
+            new Url('http://httpbin.org/status/418', 418, 'teapot', 'kitchen/teapot', 1, 2),
         ];
 
         expect($results)->toEqual($expected);

diff --git a/spec/stubs/.cigar.fail.json b/spec/stubs/.cigar.fail.json
@@ -16,5 +16,17 @@
   {
     "url": "http://httpbin.org/status/500",
     "status": 501
+  },
+  {
+    "url": "http://httpbin.org/delay/3",
+    "status": 200,
+    "request-timeout": 1,
+    "response-timeout": 1
+  },
+  {
+    "url": "http://httpbin.org/drip?duration=3",
+    "status": 200,
+    "request-timeout": 1,
+    "response-timeout": 1
   }
 ]
diff --git a/spec/stubs/.cigar.pass.json b/spec/stubs/.cigar.pass.json
@@ -16,5 +16,17 @@
   {
     "url": "http://httpbin.org/status/500",
     "status": 500
+  },
+  {
+    "url": "http://httpbin.org/delay/3",
+    "status": 200,
+    "request-timeout": 1,
+    "response-timeout": 5
+  },
+  {
+    "url": "http://httpbin.org/drip?duration=3",
+    "status": 200,
+    "request-timeout": 1,
+    "response-timeout": 5
   }
 ]
diff --git a/spec/stubs/.cigar.timeouts.json b/spec/stubs/.cigar.timeouts.json
@@ -0,0 +1,14 @@
+[
+  {
+    "url": "http://httpbin.org/delay/3",
+    "status": 200,
+    "request-timeout": 1,
+    "response-timeout": 5
+  },
+  {
+    "url": "http://httpbin.org/drip?duration=3",
+    "status": 200,
+    "request-timeout": 1,
+    "response-timeout": 5
+  }
+]
diff --git a/src/AsyncChecker.php b/src/AsyncChecker.php
@@ -57,6 +57,13 @@ public function check(array $urlsToCheck): array
                 curl_setopt($channel, CURLOPT_HTTPHEADER, $this->headers);
             }
 
+            if ($urlToCheck->getConnectTimeout() !== null && $urlToCheck->getConnectTimeout() > 0) {
+                curl_setopt($channel, CURLOPT_CONNECTTIMEOUT, $urlToCheck->getConnectTimeout());
+            }
+            if ($urlToCheck->getTimeout() !== null && $urlToCheck->getTimeout() > 0) {
+                curl_setopt($channel, CURLOPT_TIMEOUT, $urlToCheck->getTimeout());
+            }
+
             curl_multi_add_handle($mh, $channel);
 
             $channels[$url] = $channel;

diff --git a/src/Parser.php b/src/Parser.php
@@ -9,9 +9,21 @@ class Parser
      */
     private $baseUrl;
 
-    public function __construct(string $baseUrl = null)
+    /**
+     * @var null|int
+     */
+    private $connectTimeout;
+
+    /**
+     * @var null|int
+     */
+    private $timeout;
+
+    public function __construct(string $baseUrl = null, int $connectTimeout = null, int $timeout = null)
     {
         $this->baseUrl = rtrim($baseUrl, '/');
+        $this->connectTimeout = $connectTimeout;
+        $this->timeout = $timeout;
     }
 
     /**
@@ -31,7 +43,14 @@ public function parse(string $filename): array
         return array_map(function($value) {
             $url = $this->getUrl($value['url']);
 
-            return new Url($url, $value['status'], $value['content'] ?? null, $value['content-type'] ?? null);
+            return new Url(
+                $url,
+                $value['status'],
+                $value['content'] ?? null,
+                $value['content-type'] ?? null,
+                $value['connect-timeout'] ?? $this->connectTimeout,
+                $value['timeout'] ?? $this->timeout
+            );
         }, $urls);
     }
 

diff --git a/src/Url.php b/src/Url.php
@@ -26,12 +26,30 @@ class Url
      */
     private $contentType;
 
-    public function __construct(string $url, int $status, string $content = null, string $contentType = null)
-    {
+    /**
+     * @var null|int
+     */
+    private $connectTimeout;
+
+    /**
+     * @var null|int
+     */
+    private $timeout;
+
+    public function __construct(
+        string $url,
+        int $status,
+        string $content = null,
+        string $contentType = null,
+        int $connectTimeout = null,
+        int $timeout = null
+    ) {
         $this->url = $url;
         $this->status = $status;
         $this->content = $content;
         $this->contentType = $contentType;
+        $this->connectTimeout = $connectTimeout;
+        $this->timeout = $timeout;
     }
 
     public function getUrl(): string
@@ -53,4 +71,14 @@ public function getContentType()
     {
         return $this->contentType;
     }
+
+    public function getConnectTimeout()
+    {
+        return $this->connectTimeout;
+    }
+
+    public function getTimeout()
+    {
+        return $this->timeout;
+    }
 }