Partial fix Some errors are logged only in Debug Mode

[guiltar]

Hello Kier and Mike!

Maybe this issue can't be called a bug in a full meaning but it leads to ignoring errors by addon devs and incorrect work of some core methods.

For example, there was a method className::methodName()
It was extended in some addon.
Then in newer XenForo version it gained argument className::methodName($isDelete = false)
In this case the old addon (without args in declaration) works fine without Debug and doesn't log anything!

Please, look at the XenForo_Application::handlePhpError()
It has code:

            if (!self::debugMode())
            {
                if ($errorType & E_STRICT
                    || (defined('E_DEPRECATED') && $errorType & E_DEPRECATED)
                    || (defined('E_USER_DEPRECATED') && $errorType & E_USER_DEPRECATED))
                {
                    $trigger = false;
                }
                else if ($errorType & E_NOTICE || $errorType & E_USER_NOTICE)
                {
                    $trigger = false;
                    $e = new ErrorException($errorString, 0, $errorType, $file, $line);
                    XenForo_Error::logException($e, false);
                }
            }

What I suggest is to add logging to the first "if" statement like in the second "else if":

            if (!self::debugMode())
            {
                if ($errorType & E_STRICT
                    || (defined('E_DEPRECATED') && $errorType & E_DEPRECATED)
                    || (defined('E_USER_DEPRECATED') && $errorType & E_USER_DEPRECATED))
                {
                    $trigger = false;
                    $e = new ErrorException($errorString, 0, $errorType, $file, $line);
                    XenForo_Error::logException($e, false);
                }
                else if ($errorType & E_NOTICE || $errorType & E_USER_NOTICE)
                {
                    $trigger = false;
                    $e = new ErrorException($errorString, 0, $errorType, $file, $line);
                    XenForo_Error::logException($e, false);
                }
            }

 

[Mike]

I don't agree completely. Generally speaking, there are errors that are targeted for the developer themselves and not the end user because in theory they shouldn't -- or aren't always -- going to interfere with execution.

The example of incorrect class definitions is something that could be moved to the notice level as it may well interfere, but I don't think that deprecation errors are something that should be logged for the end user themselves; developers will always have a responsibility to test their add-ons.

 

[guiltar]

Unfortunately developers often don't have time to check all the code and all the user-interface on every XF upgrage.
Many of them usually check the main functional and watch the error log then.
Maybe there is a hope to have this logging optional disabled by default

 

[Mike]

Strict mode errors will now be logged but not stop execution. (As they apply during compilation, simply loading the class would cause an error if we stopped execution; execution will still potentially fail when the function itself is called.)

Deprecation errors will still only be logged in debug mode.