friends-of-reactphp / mysql

Async MySQL database client for ReactPHP.
MIT License
334 stars 66 forks source link

Handling RuntimeException Inside $onRejected Promise #155

Closed hasanparasteh closed 2 years ago

hasanparasteh commented 2 years ago

How can I handle Runtime Exceptions inside $onRejected part of ->then() method after querying database? I think there's a important piece missing inside firends-of-reactphp/mysql and that is exception handling!

If something goes wrong with mysql server and Runtime Exception occurs the whole system crashes down apart! Here's the error message I get in these type of situations...

{closure}(): Argument #1 ($error) must be of type React\MySQL\Exception, RuntimeException given, called in /home/user/app-name/vendor/react/promise/src/RejectedPromise.php on line 28

Where is the bug? I think it's somewhere around (or I'm doing something wrong):

https://github.com/friends-of-reactphp/mysql/blob/7b4428da4d625787a0ce1420b497dadcd70ff7bd/src/Io/Connection.php#L220

And this is how I query database:

return $this->connection
    ->query($query)
    ->then(function (QueryResult $result) {
        // getting information about query
    }, function (Exception $error) {
        return [
            'result' => false,
            'line' => $error->getLine(),
            'error' => $error->getMessage(),
            'details' => $error->getTraceAsString(),
        ];
});

Can anyone else check and verify this issue exists in mysql 8.0 and friends-of-reactphp/mysql 0.5.6? can you try restarting your SQL server during a long running query and see if this happens to you too?

Also looking at: Detecting unhandled rejections

clue commented 2 years ago

@hasanparasteh Thanks for reporting, this is an interesting one!

It looks like you're using the incorrect import use React\MySQL\Exception and should probably use the type signature \Exception $error or use the import use Exception instead.

I agree that this is an easy oversight and definitely something we should improve in the future! I'll take a look as this as part of #147 which is going to improve the API somewhat :+1:

I hope this helps! I'll assume this is resolved and will close this for now, please feel free to report back otherwise :+1: