From b4ef866812c74b7e6183de8e4515262bf11d6f9a Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Mar 2018 15:09:56 +0100 Subject: [PATCH 01/11] Improve regression test for #575 --- test/Raven/phpt/fatal_reported_with_async.phpt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/Raven/phpt/fatal_reported_with_async.phpt b/test/Raven/phpt/fatal_reported_with_async.phpt index 9447e7cd5..6918ee207 100644 --- a/test/Raven/phpt/fatal_reported_with_async.phpt +++ b/test/Raven/phpt/fatal_reported_with_async.phpt @@ -10,10 +10,8 @@ while (!file_exists($vendor.'/vendor')) { require $vendor.'/test/bootstrap.php'; require $vendor.'/vendor/autoload.php'; -$client = new \Raven_Client(array('curl_method' => 'async')); -$pendingEvents = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_pending_events'); -$curlHandler = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_curl_handler'); -$pendingRequests = \PHPUnit\Framework\Assert::getObjectAttribute($curlHandler, 'requests'); +$dsn = 'https://user:password@sentry.test/123456'; +$client = new \Raven_Client($dsn, array('curl_method' => 'async')); $client->setSendCallback(function () { echo 'Sending handled fatal error...' . PHP_EOL; @@ -21,7 +19,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'; } From 806091e77bc703e08c6e86e6778ddea1c07c55ed Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Mar 2018 15:10:46 +0100 Subject: [PATCH 02/11] Apply final fix for #575 and #391 --- lib/Raven/Client.php | 5 +++++ lib/Raven/CurlHandler.php | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/Raven/Client.php b/lib/Raven/Client.php index 751c5fa59..ec586dcc1 100644 --- a/lib/Raven/Client.php +++ b/lib/Raven/Client.php @@ -258,6 +258,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; } diff --git a/lib/Raven/CurlHandler.php b/lib/Raven/CurlHandler.php index bcdd632e2..ed93edad4 100644 --- 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)) { From 325999f8651f60691a994d8e8114c86e81e50349 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Mar 2018 15:42:18 +0100 Subject: [PATCH 03/11] Refactor PHPT test to avoid triggering autoloader during shutdown --- test/Raven/phpt/fatal_reported_with_async.phpt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/Raven/phpt/fatal_reported_with_async.phpt b/test/Raven/phpt/fatal_reported_with_async.phpt index 6918ee207..a5e3fe27d 100644 --- a/test/Raven/phpt/fatal_reported_with_async.phpt +++ b/test/Raven/phpt/fatal_reported_with_async.phpt @@ -12,6 +12,9 @@ require $vendor.'/vendor/autoload.php'; $dsn = 'https://user:password@sentry.test/123456'; $client = new \Raven_Client($dsn, array('curl_method' => 'async')); +$pendingEvents = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_pending_events'); +$curlHandler = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_curl_handler'); +$pendingRequests = \PHPUnit\Framework\Assert::getObjectAttribute($curlHandler, 'requests'); $client->setSendCallback(function () { echo 'Sending handled fatal error...' . PHP_EOL; @@ -19,11 +22,7 @@ $client->setSendCallback(function () { $client->install(); -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'); - +register_shutdown_function(function () use (&$pendingEvents, &$pendingRequests) { if (! empty($pendingEvents)) { echo 'There are pending events inside the client'; } From 68eb97c74295d4294f5858d8998b3e0881e2338f Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Mar 2018 15:42:51 +0100 Subject: [PATCH 04/11] Trigger early autoload for the gzcompress function --- lib/Raven/Client.php | 8 +++++++- test/Raven/Tests/ClientTest.php | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/Raven/Client.php b/lib/Raven/Client.php index ec586dcc1..22fa1f2b7 100644 --- a/lib/Raven/Client.php +++ b/lib/Raven/Client.php @@ -128,6 +128,11 @@ class Raven_Client */ protected $_shutdown_function_has_been_set; + /** + * @var bool + */ + private $useCompression; + public function __construct($options_or_dsn = null, $options = array()) { if (is_array($options_or_dsn)) { @@ -195,6 +200,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') && extension_loaded('zip'); $this->sdk = Raven_Util::get($options, 'sdk', array( 'name' => 'sentry-php', @@ -995,7 +1001,7 @@ public function encode(&$data) return false; } - if (function_exists("gzcompress")) { + if ($this->useCompression) { $message = gzcompress($message); } diff --git a/test/Raven/Tests/ClientTest.php b/test/Raven/Tests/ClientTest.php index 528eddabe..8438393ee 100644 --- a/test/Raven/Tests/ClientTest.php +++ b/test/Raven/Tests/ClientTest.php @@ -2072,7 +2072,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') && extension_loaded('zip')) { $decoded = gzuncompress($decoded); $this->assertEquals($json_stringify, $decoded, 'Can not decompress compressed blob'); } else { From ce2b5b5f515538b729ee0b2f1391dcfa90d9456c Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Mar 2018 16:27:12 +0100 Subject: [PATCH 05/11] Disable zip compression while handling a fatal under PHP 5.x --- lib/Raven/Client.php | 2 +- lib/Raven/ErrorHandler.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Raven/Client.php b/lib/Raven/Client.php index 22fa1f2b7..a880c0449 100644 --- a/lib/Raven/Client.php +++ b/lib/Raven/Client.php @@ -131,7 +131,7 @@ class Raven_Client /** * @var bool */ - private $useCompression; + public $useCompression; public function __construct($options_or_dsn = null, $options = array()) { diff --git a/lib/Raven/ErrorHandler.php b/lib/Raven/ErrorHandler.php index dcc5876fa..3214ec180 100644 --- 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 &= PHP_VERSION_ID > 70000; $this->handleException($e, true); } } From 63f834de0befa957e1d5ad0f5c98d5c1588e5716 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 21 Mar 2018 16:53:08 +0100 Subject: [PATCH 06/11] Trigger other autoloads --- lib/Raven/Client.php | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/Raven/Client.php b/lib/Raven/Client.php index a880c0449..575b274bd 100644 --- a/lib/Raven/Client.php +++ b/lib/Raven/Client.php @@ -228,12 +228,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() @@ -1517,4 +1512,23 @@ 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'); + } + if (function_exists('mb_convert_encoding')) { + mb_convert_encoding('string', 'UTF8'); + } + } } From 6eb54f21da0be21db6a36ac80c081feee34d5f85 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 18 Apr 2018 15:44:25 +0200 Subject: [PATCH 07/11] Avoid false failure in regression test --- test/Raven/phpt/fatal_reported_with_async.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Raven/phpt/fatal_reported_with_async.phpt b/test/Raven/phpt/fatal_reported_with_async.phpt index a5e3fe27d..184d62220 100644 --- a/test/Raven/phpt/fatal_reported_with_async.phpt +++ b/test/Raven/phpt/fatal_reported_with_async.phpt @@ -11,7 +11,7 @@ require $vendor.'/test/bootstrap.php'; require $vendor.'/vendor/autoload.php'; $dsn = 'https://user:password@sentry.test/123456'; -$client = new \Raven_Client($dsn, array('curl_method' => 'async')); +$client = new \Raven_Client($dsn, array('curl_method' => 'async', 'server' => 'sentry.test')); $pendingEvents = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_pending_events'); $curlHandler = \PHPUnit\Framework\Assert::getObjectAttribute($client, '_curl_handler'); $pendingRequests = \PHPUnit\Framework\Assert::getObjectAttribute($curlHandler, 'requests'); From 3f60d5372a63161d00d7e21e05cca899d2c157eb Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 18 Apr 2018 15:49:31 +0200 Subject: [PATCH 08/11] Fix CS --- lib/Raven/Client.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Raven/Client.php b/lib/Raven/Client.php index 575b274bd..163b7e76c 100644 --- a/lib/Raven/Client.php +++ b/lib/Raven/Client.php @@ -1516,7 +1516,7 @@ public function setReprSerializer(Raven_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'); } From c5f236cce42ac1726761b1a87ffc877c683ca16b Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 18 Apr 2018 15:57:22 +0200 Subject: [PATCH 09/11] Ignore failures under PHP 5.4/5.5 --- test/Raven/phpt/fatal_reported_with_async.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Raven/phpt/fatal_reported_with_async.phpt b/test/Raven/phpt/fatal_reported_with_async.phpt index 184d62220..14a2e1539 100644 --- 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-- += 50400 && PHP_VERSION_ID < 50600) die('Skipped: this fails under PHP 5.4/5.5, we cannot fix it'); ?> --FILE-- Date: Thu, 19 Apr 2018 12:30:00 +0200 Subject: [PATCH 10/11] Move inspection of client state inside the shutdown function --- test/Raven/phpt/fatal_reported_with_async.phpt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Raven/phpt/fatal_reported_with_async.phpt b/test/Raven/phpt/fatal_reported_with_async.phpt index 14a2e1539..01ed9822a 100644 --- a/test/Raven/phpt/fatal_reported_with_async.phpt +++ b/test/Raven/phpt/fatal_reported_with_async.phpt @@ -14,6 +14,7 @@ require $vendor.'/vendor/autoload.php'; $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'); @@ -24,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'; } From 621277a1763b38570ba5c2e2e4211d346b87db98 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 20 Apr 2018 17:20:13 +0200 Subject: [PATCH 11/11] Apply review fixes --- lib/Raven/Client.php | 6 ++---- lib/Raven/ErrorHandler.php | 2 +- test/Raven/Tests/ClientTest.php | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/Raven/Client.php b/lib/Raven/Client.php index 163b7e76c..3de659474 100644 --- a/lib/Raven/Client.php +++ b/lib/Raven/Client.php @@ -200,7 +200,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') && extension_loaded('zip'); + $this->useCompression = function_exists('gzcompress'); $this->sdk = Raven_Util::get($options, 'sdk', array( 'name' => 'sentry-php', @@ -1524,9 +1524,7 @@ private function triggerAutoload() if (function_exists('mb_detect_encoding')) { mb_detect_encoding('string'); } - if (function_exists('mb_convert_encoding')) { - mb_convert_encoding('string', 'UTF8'); - } + if (function_exists('mb_convert_encoding')) { mb_convert_encoding('string', 'UTF8'); } diff --git a/lib/Raven/ErrorHandler.php b/lib/Raven/ErrorHandler.php index 3214ec180..6ab777004 100644 --- a/lib/Raven/ErrorHandler.php +++ b/lib/Raven/ErrorHandler.php @@ -139,7 +139,7 @@ public function handleFatalError() @$error['file'], @$error['line'] ); - $this->client->useCompression &= PHP_VERSION_ID > 70000; + $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 index 8438393ee..f1a9c8183 100644 --- a/test/Raven/Tests/ClientTest.php +++ b/test/Raven/Tests/ClientTest.php @@ -2072,7 +2072,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') && extension_loaded('zip')) { + if (function_exists('gzcompress')) { $decoded = gzuncompress($decoded); $this->assertEquals($json_stringify, $decoded, 'Can not decompress compressed blob'); } else {