ZF-2463: Zend_Loader suppressing parse errors when loading class files

Description

When loading a broken class file, I'd expect it to fail with a parse error. However, it now it fails (r7558) silently with no clue as to why.

This was working in the last snapshot I downloaded (7453). It seems to have broken in the trunk since. The working method "includeFile()" below still exists in the file but seems to have been forgotten while I see various "@include*" elsewhere which appear to be causing the problem.


    /**
     * Attempt to include() the file.
     *
     * include() is not prefixed with the @ operator because if
     * the file is loaded and contains a parse error, execution
     * will halt silently and this is difficult to debug.
     *
     * Always set display_errors = Off on production servers!
     *
     * @param  string  $filespec
     * @param  boolean $once
     * @return boolean
     */
    protected static function _includeFile($filespec, $once = false)
    {
        if ($once) {
            return include_once $filespec;
        } else {
            return include $filespec ;
        }
    }

How I reproduced the fault:

BustedClass.php:


<?php

class BustedClass
{
somebarewordshouldcauseaparseerror;
}

Then ran this script:


require_once 'Zend/Loader.php';
Zend_Loader::registerAutoload();

$a = new BustedClass();

echo "finished\n";

This should produce:


Parse error: syntax error, unexpected T_STRING, expecting T_FUNCTION in /home/zf/www/sandbox/BustedClass.php on line 5

but in the current svn trunk it just fails silently now making debugging quite disconcerting. Could we have the old behaviour back please?

Comments

I can reproduce the problem on PHP 5.1.4 (WinXP)

It appears that line 83 (within {{loadClass()}}) is to blame:


            @include_once $file;

Yes, removing the error-suppression operator "{{@}}" from line 83 revealed the parse error on my system.

Also this lines I think:


        /**
         * Try finding for the plain filename in the include_path.
         */
        switch ($once) {
            case true:
                $result = @include_once $filename;
                break;
            case false:
            default:
                $result = @include $filename;
        }

Yes, I've changed those lines... marking as in progress...

Thanks. That seems to have fixed it.

I believe I have resolved this issue with SVN r7577. Sorry if the link doesn't work for some time as Fisheye reads the SVN repository for its revision cache.

Please update from SVN and provide feedback; if the changes are acceptable, the issue could be resolved with a small addition to documentation that illustrates how to fetch/suppress errors from a file included with Zend_Loader.

The behavioral change brief:

Before: All php errors from included files were suppressed and unavailable to developers, making it harder to debug. After: All non-fatal php errors are available to the user through the {{includeErrors}} property of a Zend_Exception instance, which is thrown when an included file produces such errors. Easier to debug, but forces developers to either be {{E_STRICT}} compliant, or catch the thrown exception.

Perhaps there should be a way to switch between these behaviors. For example, we could modify this such that an exception is not thrown, but errors are simply available to the user if (s)he should want to examine them:


// this would be much like the behavior before, only we have ability to look at errors
Zend_Loader::throwExceptions(false);
Zend_Loader::loadFile($someFile);
if (Zend_Loader::hasErrors()) {
    // do something with Zend_Loader::getErrors()
}

// here i don't care about E_STRICT compliance
Zend_Loader::registerAutoload();
$obj = My_Autoloaded_Class();

// now i care about E_STRICT compliance
Zend_Loader::throwExceptions(true);
try {
    Zend_Loader::loadClass('My_Other_Class');
} catch (Zend_Exception $e) {
    foreach ($e->includeErrors as $error) {
        // do something with $error
    }
}

alternatively, for the last example:


// now i care about E_STRICT compliance
Zend_Loader::loadClass('My_Other_Class');
foreach (Zend_Loader::getErrors() as $error) {
    // do something with $error
}

This way the class does not have to throw an exception and simplifies the API.

I'm not sure I'm qualified enough on this but what is the purpose? To convert errors to exceptions?

If so, then storing non-fatal errors for inspection seems fine. Whether or not they are thrown as an exception should by default be based on the active error reporting levels maybe?

My concerns at this point are two-fold:

Backward compatibility: Throwing an exception when an included file produces, for example, an {{E_NOTICE}} seems excessive behavior compared with the previous behavior of complete error suppression. What the developer needs is the ability to access these errors, but (s)he does not need to be forced to catch an exception that contains them.

Performance: - This error capturing behavior will most certainly degrade performance for users of {{Zend_Loader}}. Probably there should be the ability to turn it off where it is not needed.

Backward compatibility:

The current stable release (ZF 1.0.3) appears to only throw an exception if the file is not found/readble. There is no error suppression. Basically, it's just a wrapper to require_once with psuedo namespacing support from what I can make out. Any error reporting is left to PHP's error reporting state.

Personally that's the way I like it. If I want to see E_NOTICEs or whatever I will with v1.0.3 based on my error reporting level.

I also tried a grep in the trunk out of interest: mim@zev:/home/zf/lib/unstable/trunk$ grep -R Zend_Loader_Exception incubator/library/|grep -v ".svn" incubator/library/Zend/Form/Element.php: * @throws Zend_Loader_Exception on invalid type. mim@zev:/home/zf/lib/unstable/trunk$ grep -R Zend_Loader_Exception library/|grep -v ".svn" library/Zend/Loader/Exception.php:class Zend_Loader_Exception extends Zend_Exception library/Zend/Loader/PluginLoader/Exception.php: * @see Zend_Loader_Exception library/Zend/Loader/PluginLoader/Exception.php:class Zend_Loader_PluginLoader_Exception extends Zend_Loader_Exception

Performance: Can't we have this new functionality off by default then? that would be fastest. loadClass($class, $dirs = null) is how it is now. Perhaps, it could become: loadClass($class, $dirs = null, $throwOnErrorLevel = null) using http://uk2.php.net/manual/en/… values as desired?

SVN r7726 removes the custom error handling functionality for performance and (hopefully) backward compatibility. Please comment here with any problems. Thanks! :)

Resolving as fixed for next minor release. Please feel free to reopen if the issue is not resolved to your satisfaction.

Was this first fixed in 1.0.4 or 1.5?

Actually, I see it's marked as fixed for the next minor release, so I'm assuming it was first fixing in 1.5.0. Updating the issue now. Please update JIRA if this is not the case.

This has been reverted with ZF-2923, as warnings and other errors emitted with autoloading will prevent subsequent autoloaders in the spl_autoload chain to trigger. This change is effective in trunk as of r12769, and will be released with 1.8.0.