replaced thrown exceptions logic with logger->warning calls for Thrif…

…tUdpTransport
jonahgeorge · Sep 15, 2021 · 1f1462a · 1f1462a
1 parent 98b3af3
commit 1f1462a
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 19 deletions.
diff --git a/src/Jaeger/ThriftUdpTransport.php b/src/Jaeger/ThriftUdpTransport.php
@@ -4,7 +4,6 @@
 
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
-use Thrift\Exception\TTransportException;
 use Thrift\Transport\TTransport;
 
 class ThriftUdpTransport extends TTransport
@@ -38,7 +37,7 @@ public function __construct(string $host, int $port, LoggerInterface $logger = n
         $this->port = $port;
         $this->socket = @socket_create(AF_INET, SOCK_DGRAM, SOL_UDP);
         if ($this->socket === false) {
-            $this->handleError("socket_create failed");
+            $this->handleSocketError("socket_create failed");
         }
         $this->logger = $logger ?? new NullLogger();
     }
@@ -55,14 +54,12 @@ public function isOpen()
 
     /**
      * Open the transport for reading/writing
-     *
-     * @throws TTransportException if cannot open
      */
     public function open()
     {
         $ok = @socket_connect($this->socket, $this->host, $this->port);
         if ($ok === false) {
-            $this->handleError('socket_connect failed');
+            $this->handleSocketError('socket_connect failed');
         }
     }
 
@@ -71,6 +68,11 @@ public function open()
      */
     public function close()
     {
+        if (is_null($this->socket)) {
+            $this->logger->warning("can't close empty socket");
+            return ;
+        }
+
         @socket_close($this->socket);
         $this->socket = null;
     }
@@ -91,25 +93,25 @@ public function read($len)
      * Writes the given data out.
      *
      * @param string $buf The data to write
-     * @throws TTransportException if writing fails
      */
     public function write($buf)
     {
         if (!$this->isOpen()) {
-            throw new TTransportException('transport is closed');
+            $this->logger->warning('transport is closed');
+            return ;
         }
 
         $ok = @socket_write($this->socket, $buf);
         if ($ok === false) {
-            $this->handleError("socket_write failed");
+            $this->handleSocketError("socket_write failed");
         }
     }
 
-    public function handleError($msg)
+    public function handleSocketError($msg)
     {
         $errorCode = socket_last_error($this->socket);
         $errorMsg = socket_strerror($errorCode);
 
-        throw new TTransportException(sprintf('%s: [code - %d] %s', $msg, $errorCode, $errorMsg));
+        $this->logger->warning(sprintf('%s: [code - %d] %s', $msg, $errorCode, $errorMsg));
     }
 }
diff --git a/tests/Jaeger/Logger/StackLogger.php b/tests/Jaeger/Logger/StackLogger.php
@@ -0,0 +1,30 @@
+<?php
+
+namespace Jaeger\Tests\Logger;
+
+use Psr\Log\LoggerTrait;
+
+class StackLogger implements \Psr\Log\LoggerInterface
+{
+    /** @var array */
+    protected $messagesStack = [];
+
+    use LoggerTrait;
+
+    public function log($level, $message, array $context = array())
+    {
+        $this->messagesStack[] = $message;
+    }
+
+    public function getLastMessage() {
+        return array_pop($this->messagesStack);
+    }
+
+    public function getMessagesCount() {
+        return count($this->messagesStack);
+    }
+
+    public function clear() {
+        $this->messagesStack = [];
+    }
+}
diff --git a/tests/Jaeger/ThriftUdpTransportTest.php b/tests/Jaeger/ThriftUdpTransportTest.php
@@ -2,6 +2,7 @@
 
 namespace Jaeger\Tests;
 
+use Jaeger\Tests\Logger\StackLogger;
 use Jaeger\ThriftUdpTransport;
 use PHPUnit\Framework\TestCase;
 use Thrift\Exception\TTransportException;
@@ -13,43 +14,72 @@ class ThriftUdpTransportTest extends TestCase
      */
     private $transport;
 
+    /**
+     * @var StackLogger
+     */
+    private $logger;
+
     public function setUp(): void
     {
-        $this->transport = new ThriftUdpTransport('127.0.0.1', 12345);
+        $this->logger = new StackLogger();
+        $this->transport = new ThriftUdpTransport('127.0.0.1', 12345, $this->logger);
     }
 
     public function testisOpenWhenOpen()
     {
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
         $this->assertTrue($this->transport->isOpen());
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
     }
 
     public function testisOpenWhenClosed()
     {
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
         $this->transport->close();
         $this->assertFalse($this->transport->isOpen());
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
     }
 
     public function testClose()
     {
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
         $this->transport->close();
 
-        $this->expectException(TTransportException::class);
-        $this->expectExceptionMessage('transport is closed');
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
         $this->transport->write('hello');
+        $this->assertEquals($this->logger->getMessagesCount(), 1);
+        $this->assertEquals($this->logger->getLastMessage(), 'transport is closed');
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
+    }
+
+    public function testDoubleClose() {
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
+        $this->transport->close();
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
+        $this->transport->close();
+        $this->assertEquals($this->logger->getMessagesCount(), 1);
+        $this->assertEquals(
+            $this->logger->getLastMessage(),
+            "can't close empty socket"
+        );
     }
 
     public function testException() {
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
         $this->transport->open();
+        $this->assertEquals($this->logger->getMessagesCount(), 0);
 
-        $this->expectException(TTransportException::class);
+        $this->transport->write(str_repeat("some string", 10000));
 
-        $msgRegEx = "/socket_write failed: \[code - \d+\] Message too long/";
-        if (method_exists($this, "expectExceptionMessageRegExp")) {
-            $this->expectExceptionMessageRegExp($msgRegEx);
+        $this->assertEquals($this->logger->getMessagesCount(), 1);
+        $msg = $this->logger->getLastMessage();
+        $pattern = "/socket_write failed: \[code - \d+\] Message too long/";
+
+        if (method_exists($this, "assertMatchesRegularExpression")) {
+            $this->assertMatchesRegularExpression($pattern, $msg);
         } else {
-            $this->expectExceptionMessageMatches($msgRegEx);
+            $this->assertRegExp($pattern, $msg);
         }
 
-        $this->transport->write(str_repeat("some string", 10000));
     }
 }