From 142f0f8406418230e5e857fe9d483dd54f665400 Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 30 Jun 2015 19:23:34 -0700 Subject: [PATCH] Avoid constructing Error objects if we are going to skip notification --- src/Bugsnag/Client.php | 25 +++++++++++------------ src/Bugsnag/Configuration.php | 11 ++++++++++ src/Bugsnag/Error.php | 31 ++++------------------------- tests/Bugsnag/ConfigurationTest.php | 14 +++++++++++++ tests/Bugsnag/ErrorTest.php | 22 ++------------------ 5 files changed, 43 insertions(+), 60 deletions(-) diff --git a/src/Bugsnag/Client.php b/src/Bugsnag/Client.php index 87088259..d21db5e7 100644 --- a/src/Bugsnag/Client.php +++ b/src/Bugsnag/Client.php @@ -414,22 +414,24 @@ public function notifyError($name, $message, array $metaData = null, $severity = // Exception handler callback, should only be called internally by PHP's set_exception_handler public function exceptionHandler($exception) { + if(!$this->config->autoNotify) { + return; + } + $error = Bugsnag_Error::fromPHPException($this->config, $this->diagnostics, $exception); $error->setSeverity("error"); - - if (!$error->shouldIgnore() && $this->config->autoNotify) { - $this->notify($error); - } + $this->notify($error); } // Exception handler callback, should only be called internally by PHP's set_error_handler public function errorHandler($errno, $errstr, $errfile = '', $errline = 0) { - $error = Bugsnag_Error::fromPHPError($this->config, $this->diagnostics, $errno, $errstr, $errfile, $errline); - - if (!$error->shouldIgnore() && $this->config->autoNotify) { - $this->notify($error); + if(!$this->config->autoNotify || $this->config->shouldIgnoreErrorCode($errno)) { + return; } + + $error = Bugsnag_Error::fromPHPError($this->config, $this->diagnostics, $errno, $errstr, $errfile, $errline); + $this->notify($error); } // Shutdown handler callback, called when the PHP process has finished running @@ -440,13 +442,10 @@ public function shutdownHandler() $lastError = error_get_last(); // Check if a fatal error caused this shutdown - if (!is_null($lastError) && Bugsnag_ErrorTypes::isFatal($lastError['type'])) { + if (!is_null($lastError) && Bugsnag_ErrorTypes::isFatal($lastError['type']) && $this->config->autoNotify && !$this->config->shouldIgnoreErrorCode($lastError['type'])) { $error = Bugsnag_Error::fromPHPError($this->config, $this->diagnostics, $lastError['type'], $lastError['message'], $lastError['file'], $lastError['line'], true); $error->setSeverity("error"); - - if (!$error->shouldIgnore() && $this->config->autoNotify) { - $this->notify($error); - } + $this->notify($error); } // Flush any buffered errors diff --git a/src/Bugsnag/Configuration.php b/src/Bugsnag/Configuration.php index 11448300..0813e5b9 100644 --- a/src/Bugsnag/Configuration.php +++ b/src/Bugsnag/Configuration.php @@ -60,6 +60,17 @@ public function shouldNotify() return is_null($this->notifyReleaseStages) || (is_array($this->notifyReleaseStages) && in_array($this->releaseStage, $this->notifyReleaseStages)); } + public function shouldIgnoreErrorCode($code) + { + if (isset($this->errorReportingLevel)) { + return !($this->errorReportingLevel & $code); + } else { + return !(error_reporting() & $code); + } + + return false; + } + public function setProjectRoot($projectRoot) { $this->projectRoot = $projectRoot; diff --git a/src/Bugsnag/Error.php b/src/Bugsnag/Error.php index 30a907a5..5ccf3da7 100644 --- a/src/Bugsnag/Error.php +++ b/src/Bugsnag/Error.php @@ -16,7 +16,6 @@ class Bugsnag_Error public $metaData = array(); public $config; public $diagnostics; - public $code; public $previous; public $groupingHash; @@ -74,7 +73,7 @@ public function setGroupingHash($groupingHash) return $this; } - + public function setStacktrace($stacktrace) { $this->stacktrace = $stacktrace; @@ -82,13 +81,6 @@ public function setStacktrace($stacktrace) return $this; } - public function setCode($code) - { - $this->code = $code; - - return $this; - } - public function setSeverity($severity) { if (!is_null($severity)) { @@ -132,8 +124,7 @@ public function setPHPError($code, $message, $file, $line, $fatal = false) $this->setName(Bugsnag_ErrorTypes::getName($code)) ->setMessage($message) ->setSeverity(Bugsnag_ErrorTypes::getSeverity($code)) - ->setStacktrace($stacktrace) - ->setCode($code); + ->setStacktrace($stacktrace); return $this; } @@ -156,20 +147,6 @@ public function setPrevious($exception) return $this; } - public function shouldIgnore() - { - // Check if we should ignore errors of this type - if (isset($this->code)) { - if (isset($this->config->errorReportingLevel)) { - return !($this->config->errorReportingLevel & $this->code); - } else { - return !(error_reporting() & $this->code); - } - } - - return false; - } - public function toArray() { $errorArray = array( @@ -182,11 +159,11 @@ public function toArray() 'exceptions' => $this->exceptionArray(), 'metaData' => $this->cleanupObj($this->metaData), ); - + if (isset($this->groupingHash)) { $errorArray['groupingHash'] = $this->groupingHash; } - + return $errorArray; } diff --git a/tests/Bugsnag/ConfigurationTest.php b/tests/Bugsnag/ConfigurationTest.php index 55274f88..bc49a5b4 100644 --- a/tests/Bugsnag/ConfigurationTest.php +++ b/tests/Bugsnag/ConfigurationTest.php @@ -56,4 +56,18 @@ public function testNotifier() $this->assertEquals($this->config->notifier['name'], "Bugsnag PHP (Official)"); $this->assertEquals($this->config->notifier['url'], "https://bugsnag.com"); } + + public function testShouldIgnore() + { + $this->config->errorReportingLevel = E_ALL & ~E_NOTICE & ~E_STRICT & ~E_DEPRECATED; + + $this->assertTrue($this->config->shouldIgnoreErrorCode(E_NOTICE)); + } + + public function testShouldNotIgnore() + { + $this->config->errorReportingLevel = E_ALL; + + $this->assertfalse($this->config->shouldIgnoreErrorCode(E_NOTICE)); + } } diff --git a/tests/Bugsnag/ErrorTest.php b/tests/Bugsnag/ErrorTest.php index 2f508f16..a0ed4e60 100644 --- a/tests/Bugsnag/ErrorTest.php +++ b/tests/Bugsnag/ErrorTest.php @@ -33,24 +33,6 @@ public function testMetaDataMerging() $this->assertEquals($errorArray['metaData']["Testing"]["localArray"], "yo"); } - public function testShouldIgnore() - { - $this->config->errorReportingLevel = E_ALL & ~E_NOTICE & ~E_STRICT & ~E_DEPRECATED; - - $this->error->setPHPError(E_NOTICE, "Broken", "file", 123); - - $this->assertTrue($this->error->shouldIgnore()); - } - - public function testShouldNotIgnore() - { - $this->config->errorReportingLevel = E_ALL; - - $this->error->setPHPError(E_NOTICE, "Broken", "file", 123); - - $this->assertfalse($this->error->shouldIgnore()); - } - public function testFiltering() { $this->error->setMetaData(array("Testing" => array("password" => "123456"))); @@ -129,7 +111,7 @@ public function testPreviousException() $this->assertEquals($errorArray['exceptions'][1]['message'], 'secondly'); } } - + public function testErrorGroupingHash() { $this->error->setGroupingHash('herp#derp'); @@ -137,7 +119,7 @@ public function testErrorGroupingHash() $errorArray = $this->error->toArray(); $this->assertEquals($errorArray['groupingHash'], 'herp#derp'); } - + public function testErrorGroupingHashNotSet() { $errorArray = $this->error->toArray();