nette / mail

A handy library for creating and sending emails in PHP
https://doc.nette.org/mailing
Other
469 stars 71 forks source link

Remove @ error suppression when connecting. #49

Closed vicary closed 6 years ago

vicary commented 6 years ago

With error suppression, i.e. @stream_socket_client(), will have $errno set to 0 and $error being an empty string ''. This is an undocumented behaviour but leads to inconveniences during debug and development.

According to the document and my tests with PHP 7.0, 7.1 and 7.2 this function does not throw.

Removing the @ sign will have the function populates the actual $errno and $error which I believe is in fact the expected result.

dg commented 6 years ago

There is $error ?: error_get_last()['message'] so empty string doesn't matter.


I tried it and it did not confirm what you wrote, i.e. "with error suppression, i.e. @stream_socket_client(), will have $errno set to 0 and $error being an empty string". According my test error suppression has not this effect.

vicary commented 6 years ago

Because this change does have the actual error context throwing.

My experience shows they are in fact empty string and zero, do you think it is caused by some other bugs?

EDIT: Took one more look and found that composer does not have this release yet, but yet, with the suggested line of code it looks like $errno being zero when suppressed is not handled this way.

dg commented 6 years ago

I created tag v2.4.5

vicary commented 6 years ago

Thanks. But allow me, is the $errno being left alone intentionally?

dg commented 6 years ago

$errno is passed to SmtpException as second parameter. Or what do you mean?

vicary commented 6 years ago

Because the @ sign will make stream_socket_client() always leave $errno as 0, the new error_get_last() does not reveal the original error code.

dg commented 6 years ago

I tried it and it did not confirm what you wrote, i.e. "with error suppression, i.e. @stream_socket_client(), will have $errno set to 0 and $error being an empty string". According my test error suppression has not this effect.