Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply other fix for #391 #576

Merged
merged 11 commits into from
Apr 20, 2018
37 changes: 30 additions & 7 deletions lib/Raven/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,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)) {
Expand Down Expand Up @@ -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');

$this->sdk = Raven_Util::get($options, 'sdk', array(
'name' => 'sentry-php',
Expand Down Expand Up @@ -222,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()
Expand Down Expand Up @@ -258,6 +259,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;
}

Expand Down Expand Up @@ -990,7 +996,7 @@ public function encode(&$data)
return false;
}

if (function_exists("gzcompress")) {
if ($this->useCompression) {
$message = gzcompress($message);
}

Expand Down Expand Up @@ -1506,4 +1512,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');
}
}
}
7 changes: 6 additions & 1 deletion lib/Raven/CurlHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)) {
Expand Down
2 changes: 2 additions & 0 deletions lib/Raven/ErrorHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to comment why PHP version matters here.

Copy link
Collaborator Author

@Jean85 Jean85 Apr 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, now we have a regression test with this PR, so removing that check will make the test fail under PHP 5.

Basically, the reason is that under PHP 7 handling errors is no more a "special place" (thanks to \Throwable probably), so autoloading and other normal stuff work there too.

$this->handleException($e, true);
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/Raven/Tests/ClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
$decoded = gzuncompress($decoded);
$this->assertEquals($json_stringify, $decoded, 'Can not decompress compressed blob');
} else {
Expand Down
12 changes: 10 additions & 2 deletions test/Raven/phpt/fatal_reported_with_async.phpt
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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');
Expand All @@ -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';
}
Expand Down