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

Promise is not resolved after duplicate key error #91

Closed Malaf closed 4 years ago

Malaf commented 4 years ago

Senario:

This happens because under these conditions, the last query execution throws TyperError (Argument 2 passed to Amp\Mysql\ConnectionStatement::__construct() must be of the type string, null given) at vendor/amphp/mysql/src/Internal/Processor.php:1122

\Amp\Mysql\Internal\Processor::connect() which does not handle this TypeError. We could fix this easilly by using Promise\rethrow like this (but this would only fix the consequence of a bug, not the actual bug):

$coroutine = new Coroutine($this->read());
Promise\rethrow($coroutine);
($coroutine)->onResolve(function () {
    $this->close();

    foreach ($this->deferreds as $deferred) {
        if ($this->query === "") {
            $deferred->fail(new InitializationException("Connection went away"));
        } else {
            $deferred->fail(new ConnectionException("Connection went away... unable to fulfil this deferred ... It's unknown whether the query was executed..."));
        }
    }
});

To make this easier to reproduce and make sure environment is clean, I made a repository containing docker-compose configuration. I'm also using Promise\wait() to show that promise is not resolved Clone this repository and run the following within it's directory:

- docker-compose up -d
- docker exec -it app sh -c 'php composer.phar install'
- docker exec -it app sh -c 'php src/loop-stopped.php'

Output:

MySQL error (1062): #23000 Duplicate entry '0' for key 'test_amp_uniq_uindex'

Fatal error: Uncaught Error: Loop stopped without resolving the promise in /app/vendor/amphp/amp/lib/functions.php:188
Stack trace:
#0 /app/src/loop-stopped.php(36): Amp\Promise\wait(Object(Amp\Coroutine))
#1 {main}
  thrown in /app/vendor/amphp/amp/lib/functions.php on line 188

I made an attempt to figure out why this happens, and here are my thoughts:

1) Order of executing code.

The handleError continues processing previous error when next query have started to execute, It's reason why property 'query' is setted with null

Additional: I think that invoking of resolve/fail of deferred have to be last, beacause any actions can change state of Processor. Example: Amp\Mysql\Internal\Processor::handlerError

2) Some coroutine doesn't have particular processing of exception. Example:

(new Coroutine($this->read()))->onResolve(function () {
    $this->close();

    foreach ($this->deferreds as $deferred) {
        if ($this->query === "") {
            $deferred->fail(new InitializationException("Connection went away"));
        } else {
            $deferred->fail(new ConnectionException("Connection went away... unable to fulfil this deferred ... It's unknown whether the query was executed..."));
        }
    }
}); 
trowski commented 4 years ago

Just pushed a commit to master that should fix the problem. Please test and let me know!