amphp / mysql

An async MySQL client for PHP, optimizing database interactions with efficient non-blocking capabilities. Perfect for responsive, high-performance applications.
Other
358 stars 63 forks source link

Prevents warning when closing connection #32

Closed marcioAlmada closed 7 years ago

marcioAlmada commented 7 years ago

Prevents warning when closing connection:

fclose(): supplied resource is not a valid stream resource in ./vendor/amphp/mysql/lib/Processor.php:821

Otherwise aerys application gets unusable with error handlers like:

set_error_handler(function($errno, $errstr, $file, $line) {
    throw new \Exception("{$errstr} in {$file}:{$line}", $errno);
});
mention-bot commented 7 years ago

@marcioAlmada, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bwoebi to be a potential reviewer.

marcioAlmada commented 7 years ago

Funny thing is that this bug occurs within aerys but I can't reproduce in isolation 🤔

bwoebi commented 7 years ago

This is why one does use an error handler like:

set_error_handler(function($errno, $errstr, $file, $line) {
    if (error_reporting()) { // because the @-operator will set error_reporting() === 0
        throw new \Exception("{$errstr} in {$file}:{$line}", $errno);
    }
});

An if (is_resource()) check will only catch one of the potentially warning conditions. Hence it's better to not merge this and just have your error handler handle it properly.

marcioAlmada commented 7 years ago

It concerns me to see untreated warnings in the logs. But I think patching the error handler makes more sense than patching amphp/mysql.

:+1: