ZF-4286: PHP warning in Zend_Mail_Protocol_Abstract->_connect()


The call to stream_socket_client() should be prepended with an @-sign to avoid warnings on connection timeout and probably also on other errors.

Kind regards, Thomas Gelf


Warnings are not enabled in production. Thus hiding a warning IMO only makes development harder.

You didn't even look at the source before closing this, did you?

The _connect() function checks whether opening the socket failed and throws an exception if so. Exceptions are fine and can be catched, native PHP warnings are absolutely useless here and would keep filling up my logs. Not every productional PHP code is a web application - I have a lot of code executed by various cronjobs and daemons, all of them running with E_ALL and E_STRICT enabled. If some warning appears our sysadmins immediately get a notification by mail.

Cheers, Thomas

As you've set this ticket to "unassigned" without any comment I took some time to reread "ZF coding standards", just to be sure. Regarding "Errors and Exceptions" they clearly state:

"The Zend Framework codebase must be E_STRICT compliant. Zend Framework code should not emit PHP warning (E_WARNING, E_USER_WARNING), notice (E_NOTICE, E_USER_NOTICE), or strict (E_STRICT) messages when error_reporting is set to E_ALL | E_STRICT."

Even if the "should not" is not enforcing it for me this point is pretty clear and no further discussion is needed.

I'll change this ticket to be reassigned automatically and also change priority and type - it's not an improvement, it's a coding standards violation.

Best regards, Thomas Gelf

Attached patch file for current SVN.

Probably the best situation here is to use error_get_last() when throwing the exception indicating that opening the stream failed. I think the patch Thomas has, combined with that information, would fit both criteria. (error_get_last() will get the last error message, even when error suppression occurred).

Thank you, Matthew! However: error_get_last() isn't required, stream_socket_client() is already writing errors to $errorStr, and this error is thrown as Zend_Mail_Protocol_Exception in case of an eventual error.

stream_socket_client() now got the shutup operator.

I also did take a look at the additional options. error_get_last() might give a message if there is a possibility for stream_socket_client() to fail without a warning. Would be better to add that to Zend_Exception. It would also help with all other suppressed warnings, as no component uses or returns that information yet.

Writing a test for this task would mean using a error_handler, as I couldn't find an other way to know if a warning was raised, but suppressed. Not worth it.

Changing issues in preparation for the 1.7.0 release.