search

zytzagoo / smtp-validate-email

A PHP library for performing email addresses validation via SMTP
GNU General Public License v3.0
437 stars 155 forks source link

Use error_clear_last() #77

Closed W1zzardTPU closed 2 years ago

W1zzardTPU commented 2 years ago

Great work. huge fan of the library, finally, no more bounced emails in my inbox ;)

During validation, the library can trigger various PHP errors, which get saved in the variable returned by error_get_last()

On execution end of our scripts we use error_get_last() to check if any errors occurred and log/report them accordingly.

Given that these errors are expected within your library, it might make sense to call error_clear_last() in those cases? Can't think of any negative side effects.

zytzagoo commented 2 years ago

Hmm... manual states (in comments): if an error handler (set_error_handler()) successfully handles an error, error_get_last() will not report it. So that's one way to handle it outside of the library probably...

Or, alternatively, why can't you just call error_clear_last() yourself? (after the call to validate(), or however you use the lib, but before the "execution end of our scripts" is reached...)

Maybe some example code (and/or triggered error) would help me understand the use-case better...

At first glance, I don't feel great about unconditionally calling error_clear_last() -- feels like it would break/hide errors from those that actually do want to have them reported in error_get_last().

W1zzardTPU commented 2 years ago

why can't you just call error_clear_last() yourself?

This is exactly what I'm doing right now, still felt like I should open an issue for discussion

triggered error

I should have mentioned that ;) It's about the socket connection attempt failing/timeout, etc

feels like it would break/hide errors from those that actually do want to have them reported in error_get_last().

Not sure if such a scenario exists. "Real" errors, yes of course, these are useful, but something that is not an error but an expected result from regular operation of a library? Not sure about that

zytzagoo commented 2 years ago

No, no, thanks for bringing the issue up, absolutely, it's a valid concern!

So, yeah, I think I get what you mean about "real" vs "expected" -- basically, this is about the @stream_socket_client() call whose "errors" are suppressed (and yet them still showing up in error_get_last())?

Meaning... a simple one-line change of calling error_clear_last(); around https://github.com/zytzagoo/smtp-validate-email/blob/master/src/Validator.php#L543 is what you're proposing as a solution for your problems?

I don't see/expect problems with that, indeed -- would you mind creating a PR doing that?

W1zzardTPU commented 2 years ago

basically, this is about the @stream_socket_client() call

Ah, I didn't see notice @ suppression .. @ still records the error into error_get_last(), so you definitely want to clear that.

Not a PHP bug btw: https://bugs.php.net/bug.php?id=68164

would you mind creating a PR doing that?

Too busy right now, credit is all yours ;)