Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise chaining is broken #32

Open
jswofford opened this issue May 26, 2017 · 0 comments
Open

Promise chaining is broken #32

jswofford opened this issue May 26, 2017 · 0 comments
Assignees

Comments

@jswofford
Copy link

Chaining promises appears to be either broken or unsupported. The documentation claims general Promise/A+ compliance, so I'm assuming the former. Regardless, I noticed that the standard ErrorPlugin was not behaving properly when curl-client was used as the client implementation. Execution of the plugin stack simply stopped when ErrorPlugin threw an exception.

Here's an example of the issue:

use Http\Client\Common\PluginClient;
use Http\Client\Common\Plugin\ErrorPlugin;
use Http\Client\Curl\Client;
use Zend\Diactoros\Request;
use Zend\Diactoros\Response\TextResponse;

$client = new PluginClient(new Client(), [new ErrorPlugin()]);
$promise = $client->sendAsyncRequest(new Request('http://localhost:10000/bogus.php', 'GET'))->then(null, function () {
    return new TextResponse('test');
});

// Results in ClientErrorException, which should be averted by the onRejected callback above.
var_dump((string) $promise->wait()->getBody());

After digging through the code, it looks like the root problem is that all promises in a chain share the same PromiseCore object. PromiseCore simply executes all onFulfilled callbacks from all promises in the order they were created when a valid HTTP response is received, or all onRejected callbacks if the request was unsuccessful. As such, all promises that share that core return the same response or throw the same exception. This approach has a number of problems:

  • Promise results are not immutable.
  • Either all onFulfilled callbacks are run, or all onRejected callbacks are run. There is no evaluation of which callback to execute at each step of the chain.
  • Multiple chains branching off the same promise interfere with each other.
  • Exceptions thrown by onFulfilled callbacks (like with ErrorPlugin) cause catastrophic failures.

Point 4 also leads to a far more insidious error that can occur when issuing multiple asynchronous requests at once. This might technically qualify as a separate issue since it stems from the handling of curl_multi_info_read, but I'm mentioning it here since it's a consequence of exceptions thrown from onFulfilled callbacks. Exceptions triggered during the resolution of one response can be thrown when requesting the resolution of a completely different response.

use Http\Client\Common\PluginClient;
use Http\Client\Common\Plugin\ErrorPlugin;
use Http\Client\Curl\Client;
use Zend\Diactoros\Request;

$client = new PluginClient(new Client(), [new ErrorPlugin()]);
$bad = $client->sendAsyncRequest(new Request('http://localhost:10000/bogus.php', 'GET'));
$good = $client->sendAsyncRequest(new Request('http://localhost:10000/server.php', 'GET'));

// Results in ClientErrorException from failure of $bad request.
var_dump((string) $good->wait()->getBody());

I'm presently working on a fix, but I wanted to make the issue known.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants