Require errorReportingLevel to be a subset of error_reporting

CHANGELOG.md

@@ -1,6 +1,17 @@
 Changelog
 =========
 
+## TBD
+
+This release changes how Bugsnag detects the error suppression operator in combination with the `errorReportingLevel` configuration option, for PHP 8 compatibility. Bugsnag's `errorReportingLevel` must now be a subset of `error_reporting` — i.e. every error level in `errorReportingLevel` must also be in `error_reporting`
+
+If you use the `errorReportingLevel` option, you may need to change your Bugsnag or PHP configuration in order to report all expected errors. See [PR #611](https://github.com/bugsnag/bugsnag-php/pull/611) for more details
+
+### Fixes
+
+* Make `Configuration::shouldIgnoreErrorCode` compatible with PHP 8 by requiring the `errorReportingLevel` option to be a subset of `error_reporting`
+  [#611](https://github.com/bugsnag/bugsnag-php/pull/611)
+
 ## 3.23.1 (2020-10-19)
 
 This release fixes several issues with Bugsnag's error handlers that caused it to affect the behaviour of shutdown functions ([#475](https://github.com/bugsnag/bugsnag-php/issues/475)) and CLI script exit codes ([#523](https://github.com/bugsnag/bugsnag-php/issues/523)). This does not apply if you are using the Laravel or Symfony integrations, as they use separate methods of error handling.

phpunit.xml.dist

@@ -29,4 +29,9 @@
             <directory suffix=".phpt">./tests/phpt</directory>
         </testsuite>
     </testsuites>
+
+    <php>
+        <!-- Use a very big number as we can't use the 'E_ALL' constant here -->
+        <ini name="error_reporting" value="2147483647" />
+    </php>
 </phpunit>

src/Configuration.php

@@ -569,11 +569,68 @@ public function getMetaData()
      */
     public function setErrorReportingLevel($errorReportingLevel)
     {
+        if (!$this->isSubsetOfErrorReporting($errorReportingLevel)) {
+            $missingLevels = implode(', ', $this->getMissingErrorLevelNames($errorReportingLevel));
+            $message =
+                'Bugsnag Warning: errorReportingLevel cannot contain values that are not in error_reporting. '.
+                "Any errors of these levels will be ignored: {$missingLevels}.";
+
+            error_log($message);
+        }
+
         $this->errorReportingLevel = $errorReportingLevel;
 
         return $this;
     }
 
+    /**
+     * Check if the given error reporting level is a subset of error_reporting.
+     *
+     * For example, if $level contains E_WARNING then error_reporting must too.
+     *
+     * @param int|null $level
+     *
+     * @return bool
+     */
+    private function isSubsetOfErrorReporting($level)
+    {
+        if (!is_int($level)) {
+            return true;
+        }
+
+        $errorReporting = error_reporting();
+
+        // If all of the bits in $level are also in $errorReporting, ORing them
+        // together will result in the same value as $errorReporting because
+        // there are no new bits to add
+        return ($errorReporting | $level) === $errorReporting;
+    }
+
+    /**
+     * Get a list of error level names that are in $level but not error_reporting.
+     *
+     * For example, if error_reporting is E_NOTICE and $level is E_ERROR then
+     * this will return ['E_ERROR']
+     *
+     * @param int $level
+     *
+     * @return string[]
+     */
+    private function getMissingErrorLevelNames($level)
+    {
+        $missingLevels = [];
+        $errorReporting = error_reporting();
+
+        foreach (ErrorTypes::getAllCodes() as $code) {
+            // $code is "missing" if it's in $level but not in $errorReporting
+            if (($code & $level) && !($code & $errorReporting)) {
+                $missingLevels[] = ErrorTypes::codeToString($code);
+            }
+        }
+
+        return $missingLevels;
+    }
+
     /**
      * Should we ignore the given error code?
      *
@@ -583,19 +640,19 @@ public function setErrorReportingLevel($errorReportingLevel)
      */
     public function shouldIgnoreErrorCode($code)
     {
-        $defaultReportingLevel = error_reporting();
-
-        if ($defaultReportingLevel === 0) {
-            // The error has been suppressed using the error control operator ('@')
-            // Ignore the error in all cases.
+        // If the code is not in error_reporting then it is either totally
+        // disabled or is being suppressed with '@'
+        if (!(error_reporting() & $code)) {
             return true;
         }
 
+        // Filter the error code further against our error reporting level, which
+        // can be lower than error_reporting
         if (isset($this->errorReportingLevel)) {
             return !($this->errorReportingLevel & $code);
         }
 
-        return !($defaultReportingLevel & $code);
+        return false;
     }
 
     /**

src/ErrorTypes.php

@@ -149,4 +149,76 @@ public static function getLevelsForSeverity($severity)
 
         return $levels;
     }
+
+    /**
+     * Get a list of all PHP error codes.
+     *
+     * @return int[]
+     */
+    public static function getAllCodes()
+    {
+        return array_keys(self::$ERROR_TYPES);
+    }
+
+    /**
+     * Convert the given error code to a string representation.
+     *
+     * For example, E_ERROR => 'E_ERROR'.
+     *
+     * @param int $code
+     *
+     * @return string
+     */
+    public static function codeToString($code)
+    {
+        switch ($code) {
+            case E_ERROR:
+                return 'E_ERROR';
+
+            case E_WARNING:
+                return 'E_WARNING';
+
+            case E_PARSE:
+                return 'E_PARSE';
+
+            case E_NOTICE:
+                return 'E_NOTICE';
+
+            case E_CORE_ERROR:
+                return 'E_CORE_ERROR';
+
+            case E_CORE_WARNING:
+                return 'E_CORE_WARNING';
+
+            case E_COMPILE_ERROR:
+                return 'E_COMPILE_ERROR';
+
+            case E_COMPILE_WARNING:
+                return 'E_COMPILE_WARNING';
+
+            case E_USER_ERROR:
+                return 'E_USER_ERROR';
+
+            case E_USER_WARNING:
+                return 'E_USER_WARNING';
+
+            case E_USER_NOTICE:
+                return 'E_USER_NOTICE';
+
+            case E_STRICT:
+                return 'E_STRICT';
+
+            case E_RECOVERABLE_ERROR:
+                return 'E_RECOVERABLE_ERROR';
+
+            case E_DEPRECATED:
+                return 'E_DEPRECATED';
+
+            case E_USER_DEPRECATED:
+                return 'E_USER_DEPRECATED';
+
+            default:
+                return 'Unknown';
+        }
+    }
 }

tests/ClientTest.php

@@ -1036,10 +1036,21 @@ public function testSetAutoCaptureSessions()
     public function testErrorReportingLevel()
     {
         $client = Client::make('foo');
+
         $this->assertSame($client, $client->setErrorReportingLevel(E_ALL));
         $this->assertFalse($client->shouldIgnoreErrorCode(E_NOTICE));
-        $this->assertSame($client, $client->setErrorReportingLevel(E_ALL && ~E_NOTICE));
+
+        $this->assertSame($client, $client->setErrorReportingLevel(E_ALL & ~E_NOTICE));
+        $this->assertTrue($client->shouldIgnoreErrorCode(E_NOTICE));
+
+        $this->assertSame($client, $client->setErrorReportingLevel(E_NOTICE));
+        $this->assertFalse($client->shouldIgnoreErrorCode(E_NOTICE));
+
+        $this->assertSame($client, $client->setErrorReportingLevel(0));
         $this->assertTrue($client->shouldIgnoreErrorCode(E_NOTICE));
+
+        $this->assertSame($client, $client->setErrorReportingLevel(null));
+        $this->assertFalse($client->shouldIgnoreErrorCode(E_NOTICE));
     }
 
     public function testMetaData()