Merge pull request #1 from xmarcos/improve_send_method

Improve the send() method
xmarcos · Jan 7, 2015 · bc67d8b · bc67d8b
2 parents 7aa67eb + a543825
commit bc67d8b
Show file tree

Hide file tree

Showing 2 changed files with 130 additions and 16 deletions.
diff --git a/src/Client.php b/src/Client.php
@@ -1,17 +1,20 @@
 <?php
 namespace xmarcos\Carbon;
 
+use Exception;
+use ErrorException;
 use InvalidArgumentException;
 
 class Client
 {
     protected $stream;
     protected $namespace;
+    protected $throw_exceptions;
 
     /**
      * Creates an instance of the Carbon Client
      *
-     * @param resource $stream A php stream that knows how to talk to Carbon
+     * @param resource $stream A php stream that knows how to talk to Carbon.
      */
     public function __construct($stream)
     {
@@ -20,6 +23,23 @@ public function __construct($stream)
         }
 
         $this->stream = $stream;
+        $this->throwExceptions(false);
+    }
+
+    /**
+     * Controls whether failed calls to Carbon will throw an Exception.
+     *
+     * @see send()
+     *
+     * @param boolean $throw
+     *
+     * @return self
+     */
+    public function throwExceptions($throw = true)
+    {
+        $this->throw_exceptions = (bool) $throw;
+
+        return $this;
     }
 
     /**
@@ -55,28 +75,48 @@ public function getNamespace()
      * @param int|float $value     Metric Value
      * @param int|null  $timestamp Metric Timestamp
      *
+     * @throws ErrorException If $this->throw_exceptions is true
      * @return bool
      */
     public function send($path, $value, $timestamp = null)
     {
-        if (!is_resource($this->stream)
-            || !is_string($path)
-            || empty($path)
-            || !is_numeric($value)
-        ) {
-            return false;
-        }
+        $result    = false;
+        $exception = null;
 
-        $value     = (float) $value;
-        $timestamp = is_numeric($timestamp) ? (int) $timestamp : time();
-        $full_path = $this->sanitizePath(
-            sprintf('%s.%s', $this->getNamespace(), $path)
-        );
+        set_error_handler(function ($code, $message, $file = null, $line = 0) {
+            throw new ErrorException($message, $code, null, $file, $line);
+        });
+
+        try {
+            if (!is_string($path) || empty($path)) {
+                throw new InvalidArgumentException('$path must be a non-empty string');
+            }
+
+            if (!is_numeric($value)) {
+                throw new InvalidArgumentException(
+                    sprintf('$value must be of type int|float, %s given.', gettype($value))
+                );
+            }
 
-        $data = sprintf("%s %f %d\n", $full_path, $value, $timestamp);
-        $sent = fwrite($this->stream, $data);
+            $value     = (float) $value;
+            $timestamp = is_numeric($timestamp) ? (int) $timestamp : time();
+            $full_path = $this->sanitizePath(
+                sprintf('%s.%s', $this->getNamespace(), $path)
+            );
+
+            $data   = sprintf("%s %f %d\n", $full_path, $value, $timestamp);
+            $sent   = fwrite($this->stream, $data);
+            $result = is_int($sent) && $sent === strlen($data);
+        } catch (Exception $e) {
+            $exception = $e;
+        }
+        restore_error_handler();
+
+        if (!empty($exception) && $this->throw_exceptions) {
+            throw $exception;
+        }
 
-        return is_int($sent) && $sent === strlen($data);
+        return $result;
     }
 
     /**

diff --git a/tests/unit/ClientTest.php b/tests/unit/ClientTest.php
@@ -1,6 +1,8 @@
 <?php
 namespace xmarcos\Carbon;
 
+use ErrorException;
+use ReflectionClass;
 use InvalidArgumentException;
 use PHPUnit_Framework_TestCase;
 
@@ -47,6 +49,18 @@ public function testSetNamespace($namespace, $expected)
         $this->assertEquals($expected, $carbon->getNamespace());
     }
 
+    public function testThrowExceptionsSetting()
+    {
+        $carbon = new Client($this->stream);
+
+        $throw_exceptions = (new ReflectionClass($carbon))->getProperty('throw_exceptions');
+        $throw_exceptions->setAccessible(true);
+        $this->assertFalse($throw_exceptions->getValue($carbon));
+
+        $carbon->throwExceptions(true);
+        $this->assertTrue($throw_exceptions->getValue($carbon));
+    }
+
     public function providerNamespaces()
     {
         return [
@@ -89,6 +103,66 @@ public function testSend(
         }
     }
 
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testSendErrorOnInvalidPath()
+    {
+        $carbon = new Client($this->stream);
+
+        $sent = $carbon->send(['x'], 1);
+        $this->assertFalse($sent);
+
+        $carbon->throwExceptions(true);
+        $carbon->send(['x'], 1);
+    }
+
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testSendErrorOnInvalidValue()
+    {
+        $carbon = new Client($this->stream);
+
+        $sent = $carbon->send('x', 'string');
+        $this->assertFalse($sent);
+
+        $carbon->throwExceptions(true);
+        $carbon->send('x', []);
+    }
+
+    /**
+     * @expectedException ErrorException
+     */
+    public function testSendErrorOnClosedStream()
+    {
+        $carbon = new Client($this->stream);
+
+        // simulate network failure
+        fclose($this->stream);
+
+        $sent = $carbon->send('metric', 1);
+        $this->assertFalse($sent);
+
+        $carbon->throwExceptions(true);
+        $sent = $carbon->send('metric', 1);
+    }
+
+    public function testSendErrorOnReadOnlyStream()
+    {
+        if (defined('HHVM_VERSION')) {
+            $this->markTestSkipped(
+                "Skipping because HHVM won't open a php://memory stream as read-only"
+            );
+        }
+
+        $read_only_stream = fopen('php://memory', 'r');
+        $carbon = new Client($read_only_stream);
+
+        $sent = $carbon->send('metric', 1);
+        $this->assertFalse($sent);
+    }
+
     public function providerMetrics()
     {
         return [