Merge pull request #667 from getsentry/do-not-set-errorid-when-not-se…

…nding Do not return error id if we know we did not send the error
getsentry · Oct 12, 2018 · 38c06ce · 38c06ce
2 parents 512d502 + a5f1c7d
commit 38c06ce
Show file tree

Hide file tree

Showing 5 changed files with 123 additions and 86 deletions.
diff --git a/lib/Raven/Client.php b/lib/Raven/Client.php
@@ -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;
         }
@@ -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);

diff --git a/test/DummyRaven/Dummy_Raven_Client.php b/test/DummyRaven/Dummy_Raven_Client.php
@@ -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();
+    }
+}
diff --git a/test/Raven/Tests/ClientTest.php b/test/Raven/Tests/ClientTest.php
@@ -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;
@@ -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());
     }
 
     /**

diff --git a/test/Raven/phpt/fatal_reported_twice_regression.phpt b/test/Raven/phpt/fatal_reported_twice_regression.phpt
@@ -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');

diff --git a/test/bootstrap.php b/test/bootstrap.php
@@ -15,3 +15,5 @@
 
 require_once dirname(__FILE__).'/../lib/Raven/Autoloader.php';
 Raven_Autoloader::register();
+
+require_once dirname(__FILE__).'/DummyRaven/Dummy_Raven_Client.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';