Simplify error handling for dns_get_record()

[aivchen]

No description provided.

[SebLevDev]

We have the same issue on running test with fake domain eg: example.com

TypeError: Egulias\EmailValidator\Validation\DNSRecords::__construct(): Argument #1 ($records) must be of type array, bool given, called in /www/vendor/egulias/email-validator/src/Validation/DNSGetRecordWrapper.php on line 24

/www/vendor/egulias/email-validator/src/Validation/DNSRecords.php:11
/www/vendor/egulias/email-validator/src/Validation/DNSGetRecordWrapper.php:24
/www/vendor/egulias/email-validator/src/Validation/DNSCheckValidation.php:148
/www/vendor/egulias/email-validator/src/Validation/DNSCheckValidation.php:130
/www/vendor/egulias/email-validator/src/Validation/DNSCheckValidation.php:97
/www/vendor/egulias/email-validator/src/Validation/MultipleValidationWithAnd.php:54
/www/vendor/egulias/email-validator/src/EmailValidator.php:37 

[egulias]

Hi @aivchen thanks for your PR. I have fixed the checks so please update with the latest code to have the checks pass.
Thanks!

[aivchen]

Hi @egulias

I've rebased my branch but the build failed again. Seems you have empty CODACY_PROJECT_TOKEN in github secrets.

[egulias]

That error is driving me nuts. The secret has vale and is correct. In fact it has been working.
But from time to time it just fails.
Or fails for jobs not run by me.
Anyway, re-run the jobs.
Thanks!

[aivchen]

That error is driving me nuts. The secret has vale and is correct. In fact it has been working. But from time to time it just fails. Or fails for jobs not run by me. Anyway, re-run the jobs. Thanks!

https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions#using-secrets-in-a-workflow

Secrets can't be used in forked repos:)

[egulias]

Thanks @aivchen didn't know that (I haven't spent too much time with gh actions).
Apparently the option is to use environments and use environment secrets. But I cannot test that.
I have pushed a change to use an environment. Do you mind to update?
Much appreciated :)

[egulias]

So I understand set_error_handler is possibly interfering with the broader project that might itself be setting that same error handler. Now, the function is releasing / restoring the handler at the end.
The aim is to catch these errors.
Suppressing them defeats the purpose. So, what do you think if we find a way for this to be optional with the suppressed option being the default?

[aivchen]

So I understand set_error_handler is possibly interfering with the broader project that might itself be setting that same error handler.

Not really. In the set_error_handler function we throw an exception which we catch directly in the same file. The old code with comments.

        set_error_handler(
            static function (int $errorLevel, string $errorMessage): never {
                // Here we throw an exception if dns_get_record function failed.
                throw new \RuntimeException("Unable to get DNS record for the host: $errorMessage");
            }
        );
        try {
            // Get all MX, A and AAAA DNS records for host
            return new DNSRecords(dns_get_record($host, $type));
        } catch (\RuntimeException $exception) {
            return new DNSRecords([], true); // And here we catch the exception thrown in set_error_handler.
        } finally {
            restore_error_handler();
        }

Therefore, my changes do not introduce any changes in logic - just a shorter entry.

[aivchen]

Actually, I didn't see that we pass true param in the catch block. Let me fix it.

[aivchen]

Done

[egulias]

I reviewed the PHP bug (https://bugs.php.net/bug.php?id=73149 ) being addressed by the workaround and it is still happening.
In short, the bug allows an unexpected error to bypass the error suppression (@) which being un-handled then breaks the execution not only of the library but of the system using it. Which would then require our users to wrap, just in case, the call in a try/catch statement.
That's something I try to avoid the users of the library to do.

So we cannot simplify the code unless the bug is fixed or a different approach is found.