Merge pull request #606 from rollbar/fixed/php8.2-dynamic-property-cr…

…eation Fixed PHP 8.2 deprecated dynamic property creation
rollbar · Feb 23, 2023 · 3626cdc · 3626cdc
2 parents 703c196 + 2083763
commit 3626cdc
Show file tree

Hide file tree

Showing 13 changed files with 83 additions and 81 deletions.
diff --git a/src/ErrorWrapper.php b/src/ErrorWrapper.php
@@ -6,8 +6,6 @@ class ErrorWrapper extends \Exception
 {
     private static $constName;
 
-    public $isUncaught;
-
     private $utilities;
 
     private static function getConstName($const)

diff --git a/src/Handlers/ErrorHandler.php b/src/Handlers/ErrorHandler.php
@@ -42,9 +42,7 @@ public function handle(...$args)
         $exception = $this->logger()->
                             getDataBuilder()->
                             generateErrorWrapper($errno, $errstr, $errfile, $errline);
-        $exception->isUncaught = true;
-        $this->logger()->log(Level::ERROR, $exception, array());
-        unset($exception->isUncaught);
+        $this->logger()->report(Level::ERROR, $exception, isUncaught: true);
 
         return false;
     }

diff --git a/src/Handlers/ExceptionHandler.php b/src/Handlers/ExceptionHandler.php
@@ -24,10 +24,8 @@ public function handle(...$args)
         }
 
         $exception = $args[0];
-        $exception->isUncaught = true;
-        $this->logger()->log(Level::ERROR, $exception, array());
-        unset($exception->isUncaught);
-
+        $this->logger()->report(Level::ERROR, $exception, isUncaught: true);
+
         // if there was no prior handler, then we toss that exception
         if ($this->previousHandler === null) {
             throw $exception;

diff --git a/src/Handlers/FatalHandler.php b/src/Handlers/FatalHandler.php
@@ -38,9 +38,7 @@ public function handle(...$args)
             $exception = $this->logger()->
                                 getDataBuilder()->
                                 generateErrorWrapper($errno, $errstr, $errfile, $errline);
-            $exception->isUncaught = true;
-            $this->logger()->log(Level::CRITICAL, $exception, array());
-            unset($exception->isUncaught);
+            $this->logger()->report(Level::CRITICAL, $exception, isUncaught: true);
         }
     }
 

diff --git a/src/Rollbar.php b/src/Rollbar.php
@@ -249,13 +249,7 @@ public static function logUncaught(string|Level $level, Throwable $toLog, array
         if (is_null(self::$logger)) {
             return self::getNotInitializedResponse();
         }
-        $toLog->isUncaught = true;
-        try {
-            $result = self::$logger->report($level, $toLog, $context);
-        } finally {
-            unset($toLog->isUncaught);
-        }
-        return $result;
+        return self::$logger->report($level, $toLog, $context, isUncaught: true);
     }
 
     /**

diff --git a/src/RollbarLogger.php b/src/RollbarLogger.php
@@ -200,9 +200,11 @@ public function log($level, string|Stringable $message, array $context = array()
      * Creates the {@see Response} object and reports the message to the Rollbar
      * service.
      *
-     * @param string|Level      $level   The severity level to send to Rollbar.
-     * @param string|Stringable $message The log message.
-     * @param array             $context Any additional context data.
+     * @param string|Level      $level      The severity level to send to Rollbar.
+     * @param string|Stringable $message    The log message.
+     * @param array             $context    Any additional context data.
+     * @param bool              $isUncaught True if the error or exception was captured by a Rollbar handler. Thus, it
+     *                                      was not caught by the application.
      *
      * @return Response
      *
@@ -211,8 +213,12 @@ public function log($level, string|Stringable $message, array $context = array()
      *
      * @since  4.0.0
      */
-    public function report($level, string|Stringable $message, array $context = array()): Response
-    {
+    public function report(
+        string|Level $level,
+        string|Stringable $message,
+        array $context = array(),
+        bool $isUncaught = false
+    ): Response {
         if ($this->disabled()) {
             $this->verboseLogger()->notice('Rollbar is disabled');
             return new Response(0, "Disabled");
@@ -241,7 +247,6 @@ public function report($level, string|Stringable $message, array $context = arra
         $accessToken = $this->getAccessToken();
         $payload     = $this->getPayload($accessToken, $level, $message, $context);
 
-        $isUncaught = $this->isUncaughtLogData($message);
         if ($this->config->checkIgnored($payload, $message, $isUncaught)) {
             $this->verboseLogger()->info('Occurrence ignored');
             $response = new Response(0, "Ignored");
@@ -475,22 +480,4 @@ protected function encode(array $payload): EncodedPayload
         $encoded->encode();
         return $encoded;
     }
-
-    /**
-     * Check whether the data to log represents an uncaught error, exception,
-     * or fatal error. This works in concert with src/Handlers/, which sets
-     * the `isUncaught` property on the `Throwable` representation of data.
-     *
-     * @since 3.0.1
-     */
-    public function isUncaughtLogData(mixed $toLog): bool
-    {
-        if (! $toLog instanceof Throwable) {
-            return false;
-        }
-        if (! isset($toLog->isUncaught)) {
-            return false;
-        }
-        return $toLog->isUncaught === true;
-    }
 }
diff --git a/tests/DefaultsTest.php b/tests/DefaultsTest.php
@@ -14,6 +14,11 @@ class DefaultsTest extends BaseRollbarTest
      */
     private \Rollbar\Defaults $defaults;
 
+    /**
+     * @var string[]
+     */
+    private array $defaultPsrLevels;
+
     public function setUp(): void
     {
         $this->defaults = new Defaults;

diff --git a/tests/Handlers/ErrorHandlerTest.php b/tests/Handlers/ErrorHandlerTest.php
@@ -57,11 +57,11 @@ public function testHandle(): void
     {
         $logger = $this->getMockBuilder(RollbarLogger::class)
                         ->setConstructorArgs(array(self::$simpleConfig))
-                        ->setMethods(array('log'))
+                        ->setMethods(array('report'))
                         ->getMock();
 
         $logger->expects($this->once())
-                ->method('log');
+                ->method('report');
 
         /**
          * Disable PHPUnit's error handler as it would get triggered as the

diff --git a/tests/Handlers/ExceptionHandlerTest.php b/tests/Handlers/ExceptionHandlerTest.php
@@ -73,11 +73,11 @@ public function testHandle(): void
 
         $logger = $this->getMockBuilder(RollbarLogger::class)
                         ->setConstructorArgs(array(self::$simpleConfig))
-                        ->setMethods(array('log'))
+                        ->setMethods(array('report'))
                         ->getMock();
 
         $logger->expects($this->once())
-                ->method('log');
+                ->method('report');
 
         $handler = new ExceptionHandler($logger);
         $handler->register();
@@ -87,4 +87,37 @@ public function testHandle(): void
         set_exception_handler(function () {
         });
     }
+
+    /**
+     * This test is specifically for the deprecated dynamic properties in PHP 8.2. We were setting a property named
+     * "isUncaught" on the exception object, which is now deprecated. This test ensures that we are no longer setting
+     * that property.
+     *
+     * @return void
+     */
+    public function testDeprecatedDynamicProperties(): void
+    {
+        // Set error reporting level and error handler to capture deprecation
+        // warnings.
+        $prev = error_reporting(E_ALL);
+        $errors = array();
+        set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$errors) {
+            $errors[] = array(
+                'errno'   => $errno,
+                'errstr'  => $errstr,
+                'errfile' => $errfile,
+                'errline' => $errline,
+            );
+        });
+        $handler = new ExceptionHandler(new RollbarLogger(self::$simpleConfig));
+        $handler->register();
+
+        $handler->handle(new \Exception());
+        restore_error_handler();
+        error_reporting($prev);
+
+        // self::assertSame used instead of self::assertSame so the contents of
+        // $errors are printed in the test output.
+        self::assertSame([], $errors);
+    }
 }
diff --git a/tests/RollbarLoggerTest.php b/tests/RollbarLoggerTest.php
@@ -178,6 +178,21 @@ public function testReport(): void
         $this->assertEquals(200, $response->getStatus());
     }
 
+    public function testReportWithIsUncaught(): void
+    {
+        $test = $this;
+        $logger = new RollbarLogger([
+            "access_token" => $this->getTestAccessToken(),
+            "environment"  => "testing-php",
+            'check_ignore' => function ($isUncaught) use ($test) {
+                $test::assertTrue($isUncaught);
+            },
+        ]);
+
+        $response = $logger->report(Level::WARNING, "Testing PHP Notifier", isUncaught: true);
+        $this->assertEquals(200, $response->getStatus());
+    }
+
     public function testDefaultVerbose(): void
     {
         $this->testNotVerbose();
@@ -765,31 +780,4 @@ public function testRaiseOnError(): void
             $logger->log(Level::ERROR, $ex);
         }
     }
-
-    /**
-     * @dataProvider providesToLogEntityForUncaughtCheck
-     */
-    public function testIsUncaughtLogData(mixed $toLog, bool $expected, string $message): void
-    {
-        $logger = new RollbarLogger(array(
-            "access_token" => $this->getTestAccessToken(),
-            "environment" => 'test',
-            "raise_on_error" => true
-        ));
-
-        $this->assertSame($logger->isUncaughtLogData($toLog), $expected, $message);
-    }
-
-    public static function providesToLogEntityForUncaughtCheck(): array
-    {
-        $uncaught = new Exception;
-        $uncaught->isUncaught = true;
-        return [
-            [ 'some string', false, 'String log data should not be seen as uncaught' ],
-            [ [], false, 'Array log data should not be seen as uncaught' ],
-            [ new StdClass, false, 'An object not deriving from Throwable should not be seen as uncaught' ],
-            [ new Exception, false, 'A raw exception is not seen as uncaught' ],
-            [ $uncaught, true, 'A Throwable-derived object marked as uncaught must be seen as uncaught' ],
-        ];
-    }
 }
diff --git a/tests/RollbarTest.php b/tests/RollbarTest.php
@@ -230,12 +230,14 @@ public function testLogUncaughtUnsetLogger(): void
 
     public function testLogUncaught(): void
     {
-        $sut = new Rollbar;
-        Rollbar::init(self::$simpleConfig);
-        $logger = Rollbar::logger();
-        $toLog = new \Exception;
+        $test = $this;
+        Rollbar::init(array_merge(self::$simpleConfig, [
+            'check_ignore' => function ($isUncaught) use ($test) {
+                $test::assertTrue($isUncaught);
+            },
+        ]));
+        $toLog  = new \Exception;
         $result = Rollbar::logUncaught(Level::ERROR, $toLog);
         $this->assertEquals(200, $result->getStatus());
-        $this->assertObjectNotHasAttribute('uncaught', $toLog);
     }
 }
diff --git a/tests/TestHelpers/MockPhpStream.php b/tests/TestHelpers/MockPhpStream.php
@@ -4,7 +4,7 @@
 
 class MockPhpStream
 {
-
+    public $context;
     protected static int $index = 0;
     protected static $length = null;
 

diff --git a/tests/Truncation/TruncationTest.php b/tests/Truncation/TruncationTest.php
@@ -9,7 +9,8 @@
 
 class TruncationTest extends BaseRollbarTest
 {
-
+    private Truncation $truncate;
+
     public function setUp(): void
     {
         $config = new Config(array('access_token' => $this->getTestAccessToken()));