Fix sending events async in PHP 5; add tests & fixes for #391 #575 (#576

) * Improve regression test for #575 * Apply final fix for #575 and #391 * Refactor PHPT test to avoid triggering autoloader during shutdown * Trigger early autoload for the gzcompress function * Disable zip compression while handling a fatal under PHP 5.x * Trigger other autoloads * Avoid false failure in regression test * Ignore failures under PHP 5.4/5.5 * Move inspection of client state inside the shutdown function * Apply review fixes
getsentry · Apr 20, 2018 · 61c862b · 61c862b
1 parent b2b6a4e
commit 61c862b
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 11 deletions.
diff --git a/lib/Raven/Client.php b/lib/Raven/Client.php
@@ -129,6 +129,11 @@ class Raven_Client
      */
     protected $_shutdown_function_has_been_set;
 
+    /**
+     * @var bool
+     */
+    public $useCompression;
+
     public function __construct($options_or_dsn = null, $options = array())
     {
         if (is_array($options_or_dsn)) {
@@ -197,6 +202,7 @@ public function __construct($options_or_dsn = null, $options = array())
         $this->context = new Raven_Context();
         $this->breadcrumbs = new Raven_Breadcrumbs();
         $this->_shutdown_function_has_been_set = false;
+        $this->useCompression = function_exists('gzcompress');
 
         $this->sdk = Raven_Util::get($options, 'sdk', array(
             'name' => 'sentry-php',
@@ -224,12 +230,7 @@ public function __construct($options_or_dsn = null, $options = array())
             $this->registerShutdownFunction();
         }
 
-        // manually trigger autoloading, as it cannot be done during error handling in some edge cases due to PHP (see #60149)
-        if (!class_exists('Raven_Stacktrace')) {
-            // @codeCoverageIgnoreStart
-            spl_autoload_call('Raven_Stacktrace');
-            // @codeCoverageIgnoreEnd
-        }
+        $this->triggerAutoload();
     }
 
     public function __destruct()
@@ -260,6 +261,11 @@ public function install()
         $this->error_handler->registerExceptionHandler();
         $this->error_handler->registerErrorHandler();
         $this->error_handler->registerShutdownFunction();
+
+        if ($this->_curl_handler) {
+            $this->_curl_handler->registerShutdownFunction();
+        }
+
         return $this;
     }
 
@@ -998,7 +1004,7 @@ public function encode(&$data)
             return false;
         }
 
-        if (function_exists("gzcompress")) {
+        if ($this->useCompression) {
             $message = gzcompress($message);
         }
 
@@ -1514,4 +1520,21 @@ public function setReprSerializer(Raven_ReprSerializer $reprSerializer)
     {
         $this->reprSerializer = $reprSerializer;
     }
+
+    private function triggerAutoload()
+    {
+        // manually trigger autoloading, as it cannot be done during error handling in some edge cases due to PHP (see #60149)
+
+        if (! class_exists('Raven_Stacktrace')) {
+            spl_autoload_call('Raven_Stacktrace');
+        }
+
+        if (function_exists('mb_detect_encoding')) {
+            mb_detect_encoding('string');
+        }
+
+        if (function_exists('mb_convert_encoding')) {
+            mb_convert_encoding('string', 'UTF8');
+        }
+    }
 }
diff --git a/lib/Raven/CurlHandler.php b/lib/Raven/CurlHandler.php
@@ -29,7 +29,7 @@ public function __construct($options, $join_timeout = 5)
         $this->requests = array();
         $this->join_timeout = $join_timeout;
 
-        register_shutdown_function(array($this, 'join'));
+        $this->registerShutdownFunction();
     }
 
     public function __destruct()
@@ -69,6 +69,11 @@ public function enqueue($url, $data = null, $headers = array())
         return $fd;
     }
 
+    public function registerShutdownFunction()
+    {
+        register_shutdown_function(array($this, 'join'));
+    }
+
     public function join($timeout = null)
     {
         if (!isset($timeout)) {

diff --git a/lib/Raven/ErrorHandler.php b/lib/Raven/ErrorHandler.php
@@ -138,6 +138,8 @@ public function handleFatalError()
                 @$error['message'], 0, @$error['type'],
                 @$error['file'], @$error['line']
             );
+
+            $this->client->useCompression = $this->client->useCompression && PHP_VERSION_ID > 70000;
             $this->handleException($e, true);
         }
     }

diff --git a/test/Raven/Tests/ClientTest.php b/test/Raven/Tests/ClientTest.php
@@ -2174,7 +2174,7 @@ public function testEncode()
         $this->assertRegExp('_^[a-zA-Z0-9/=]+$_', $value, 'Raven_Client::encode returned malformed data');
         $decoded = base64_decode($value);
         $this->assertInternalType('string', $decoded, 'Can not use base64 decode on the encoded blob');
-        if (function_exists("gzcompress")) {
+        if (function_exists('gzcompress')) {
             $decoded = gzuncompress($decoded);
             $this->assertEquals($json_stringify, $decoded, 'Can not decompress compressed blob');
         } else {

diff --git a/test/Raven/phpt/fatal_reported_with_async.phpt b/test/Raven/phpt/fatal_reported_with_async.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Test that, when handling a fatal with async send enabled, we force the async to avoid losing the event
+--SKIPIF--
+<?php if (PHP_VERSION_ID >= 50400 && PHP_VERSION_ID < 50600) die('Skipped: this fails under PHP 5.4/5.5, we cannot fix it'); ?>
 --FILE--
 <?php
 
@@ -10,7 +12,9 @@ while (!file_exists($vendor.'/vendor')) {
 require $vendor.'/test/bootstrap.php';
 require $vendor.'/vendor/autoload.php';
 
-$client = new \Raven_Client(array('curl_method' => 'async'));
+$dsn = 'https://user:password@sentry.test/123456';
+$client = new \Raven_Client($dsn, array('curl_method' => 'async', 'server' => 'sentry.test'));
+// doing this to avoid autoload-driver failures during the error handling
 $pendingEvents = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_pending_events');
 $curlHandler = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_curl_handler');
 $pendingRequests = \PHPUnit\Framework\Assert::getObjectAttribute($curlHandler, 'requests');
@@ -21,7 +25,11 @@ $client->setSendCallback(function () {
 
 $client->install();
 
-register_shutdown_function(function () use ($pendingEvents, $pendingRequests) {
+register_shutdown_function(function () use (&$client) {
+    $pendingEvents = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_pending_events');
+    $curlHandler = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_curl_handler');
+    $pendingRequests = \PHPUnit\Framework\Assert::getObjectAttribute($curlHandler, 'requests');
+
     if (! empty($pendingEvents)) {
         echo 'There are pending events inside the client';
     }