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

Do not return error id if we know we did not send the error #667

Merged
merged 3 commits into from
Oct 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions lib/Raven/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,11 @@ public function capture($data, $stack = null, $vars = null)
$this->process($data);

if (!$this->store_errors_for_bulk_send) {
$this->send($data);
if ($this->send($data) === false) {
$this->_last_event_id = null;

return null;
}
} else {
$this->_pending_events[] = $data;
}
Expand Down Expand Up @@ -1032,21 +1036,20 @@ public function send(&$data)
&& call_user_func_array($this->send_callback, array(&$data)) === false
) {
// if send_callback returns false, end native send
return;
return false;
}

if (!$this->server) {
return;
return false;
}

if ($this->transport) {
call_user_func($this->transport, $this, $data);
return;
return call_user_func($this->transport, $this, $data);
}

// should this event be sampled?
if (rand(1, 100) / 100.0 > $this->sample_rate) {
return;
return false;
}

$message = $this->encode($data);
Expand Down
85 changes: 85 additions & 0 deletions test/DummyRaven/Dummy_Raven_Client.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php

// XXX: Is there a better way to stub the client?
class Dummy_Raven_Client extends Raven_Client
{
private $__sent_events = array();
private static $input_stream;
public $dummy_breadcrumbs_handlers_has_set = false;
public $dummy_shutdown_handlers_has_set = false;

public function getSentEvents()
{
return $this->__sent_events;
}

public function send(&$data)
{
if (is_callable($this->send_callback) && call_user_func_array($this->send_callback, array(&$data)) === false) {
// if send_callback returns falsely, end native send
return false;
}

$this->__sent_events[] = $data;

if (!$this->server) {
return false;
}
}

public static function is_http_request()
{
return true;
}

public static function get_auth_header($timestamp, $client, $api_key, $secret_key)
{
return parent::get_auth_header($timestamp, $client, $api_key, $secret_key);
}

public function get_http_data()
{
return parent::get_http_data();
}

public function get_user_data()
{
return parent::get_user_data();
}

public function setInputStream($input)
{
static::$input_stream = isset($_SERVER['CONTENT_TYPE']) ? $input : false;
}

protected static function getInputStream()
{
return static::$input_stream ? static::$input_stream : file_get_contents('php://input');
}

public function buildCurlCommand($url, $data, $headers)
{
return parent::buildCurlCommand($url, $data, $headers);
}

// short circuit breadcrumbs
public function registerDefaultBreadcrumbHandlers()
{
$this->dummy_breadcrumbs_handlers_has_set = true;
}

public function registerShutdownFunction()
{
$this->dummy_shutdown_handlers_has_set = true;
}

/**
* Expose the current url method to test it
*
* @return string
*/
public function test_get_current_url()
{
return $this->get_current_url();
}
}
104 changes: 25 additions & 79 deletions test/Raven/Tests/ClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,85 +21,6 @@ function invalid_encoding()
}


// XXX: Is there a better way to stub the client?
class Dummy_Raven_Client extends Raven_Client
{
private $__sent_events = array();
private static $input_stream;
public $dummy_breadcrumbs_handlers_has_set = false;
public $dummy_shutdown_handlers_has_set = false;

public function getSentEvents()
{
return $this->__sent_events;
}

public function send(&$data)
{
if (is_callable($this->send_callback) && call_user_func_array($this->send_callback, array(&$data)) === false) {
// if send_callback returns falsely, end native send
return;
}
$this->__sent_events[] = $data;
}

public static function is_http_request()
{
return true;
}

public static function get_auth_header($timestamp, $client, $api_key, $secret_key)
{
return parent::get_auth_header($timestamp, $client, $api_key, $secret_key);
}

public function get_http_data()
{
return parent::get_http_data();
}

public function get_user_data()
{
return parent::get_user_data();
}

public function setInputStream($input)
{
static::$input_stream = isset($_SERVER['CONTENT_TYPE']) ? $input : false;
}

protected static function getInputStream()
{
return static::$input_stream ? static::$input_stream : file_get_contents('php://input');
}

public function buildCurlCommand($url, $data, $headers)
{
return parent::buildCurlCommand($url, $data, $headers);
}

// short circuit breadcrumbs
public function registerDefaultBreadcrumbHandlers()
{
$this->dummy_breadcrumbs_handlers_has_set = true;
}

public function registerShutdownFunction()
{
$this->dummy_shutdown_handlers_has_set = true;
}

/**
* Expose the current url method to test it
*
* @return string
*/
public function test_get_current_url()
{
return $this->get_current_url();
}
}

class Dummy_Raven_Client_With_Overrided_Direct_Send extends Raven_Client
{
public $_send_http_asynchronous_curl_exec_called = false;
Expand Down Expand Up @@ -1336,10 +1257,35 @@ public function testCaptureLastError()
* @covers Raven_Client::getLastEventID
*/
public function testGetLastEventID()
{
$client = new Dummy_Raven_Client('http://public:secret@example.com/1');
$client->capture(array('message' => 'test', 'event_id' => 'abc'));
$this->assertEquals('abc', $client->getLastEventID());
}

/**
* @covers Raven_Client::getLastEventID
*/
public function testGetLastEventIDWithoutServer()
{
$client = new Dummy_Raven_Client();
$client->capture(array('message' => 'test', 'event_id' => 'abc'));
$this->assertEquals(null, $client->getLastEventID());
}

/**
* @covers Raven_Client::getLastEventID
*/
public function testGetLastEventIDWithoutServerOverwritesEarlierEvents()
{
$client = new Dummy_Raven_Client('http://public:secret@example.com/1');
$client->capture(array('message' => 'test', 'event_id' => 'abc'));
$this->assertEquals('abc', $client->getLastEventID());

$client->server = null;

$client->capture(array('message' => 'test', 'event_id' => 'abc'));
$this->assertEquals(null, $client->getLastEventID());
}

/**
Expand Down
3 changes: 2 additions & 1 deletion test/Raven/phpt/fatal_reported_twice_regression.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ while (!file_exists($vendor.'/vendor')) {
$vendor = dirname($vendor);
}
require $vendor.'/test/bootstrap.php';
require_once __DIR__.'/../../DummyRaven/Dummy_Raven_Client.php';

error_reporting(E_ALL);
$client = new \Raven_Client();
$client = new \Dummy_Raven_Client('http://public:secret@example.com/1');
set_error_handler(function () use ($client) {
echo 'Previous error handler is called' . PHP_EOL;
echo 'Error is ' . ($client->getLastEventID() !== null ? 'reported correctly' : 'NOT reported');
Expand Down
2 changes: 2 additions & 0 deletions test/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@

require_once dirname(__FILE__).'/../lib/Raven/Autoloader.php';
Raven_Autoloader::register();

require_once dirname(__FILE__).'/DummyRaven/Dummy_Raven_Client.php';