Remove Typehinting

[tyx]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
License	MIT

As done on php-http/promise I removed the reponse typehinting.

I get the error when trying to make a retry middleware :

Uncaught TypeError: Argument 1 passed to Http\Client\Promise\HttpFulfilledPromise::__construct() must implement interface Psr\Http\Message\ResponseInterface, instance of Http\Client\Promise\HttpFulfilledPromise given

And I just saw that guzzle implementation does not have too.

[sagikazarmark]

Can you give us some examples on how you faced this bug? Fulfilled promises should not receive promises, but only responses IMO.

@joelwurtz is the expert on the subject though.

[shouze]

In the promise repo the type hinting has been removed in fact and yes you can see a use case in this PR: Tolerance/Tolerance#91

[joelwurtz]

It is certainly because you return an Promise inside a then callback, which is not supported in this implementation (and don't want too as it's induce a lot of complexity / code that have not this place here), so you have two choice here :

• Instead of returning the promise, return $promise->wait(); in your then callback (but you will loose "true" async)
• If you are concerned about async, use an existing promise implementation (like the one in the guzzle adapter or react adapater of php-http), or create your own. Then when the first promise is resolved, you can resolve the second with the value (or reject it).

[sagikazarmark]

In the promise repo the type hinting has been removed in fact

Yes, because there is no restriction what resolved value you put there.

Removing the typehint doesn't really solve your problem, as you would still receive a promise during unwrapping (unless that's what you want). Changing THAT logic would certainly be a BC break (not sure if anyone relies of receiving promises there though).

[tyx]

Ok, I don't get anything about Promise I should admit. I just confirm that this change make the code working.

If you are concerned about async, use an existing promise implementation (like the one in the guzzle adapter or react adapater of php-http), or create your own. Then when the first promise is resolved, you can resolve the second with the value (or reject it).

Ok let's do that but how can I tell php-http client to use another Promise implementation ?

[sagikazarmark]

Ok, I don't get anything about Promise I should admit. I just confirm that this change make the code working.

Does it actually work and does what it should do or just stops failing with this error? 😝

I commented on the linked merge request.

[tyx]

Does it actually work and does what it should do or just stops failing with this error? 😝

It really does the job it should ;) thanks for the review anyway !

In fact the main code involved was already merged and coded by someone else, I just try to make it ran with php-http as we only rely on this great lib in our apps.

[sagikazarmark]

It really does the job it should ;) thanks for the review anyway !

I have no idea how that's possible. The retirned value is not a value, it's a promise in your code. You would still have to resolve the value: $promise->wait()->wait()

[shouze]

@sagikazarmark if we have to make a double wait of that kind it means that a promise is not resolved where it should no?

[sagikazarmark]

It depends on the implementation. HTTPlug promises does not support nested promises, but some Guzzle implementations does:

https://github.com/guzzle/promises/blob/master/src/Promise.php#L64

What I find incredibly strange in your implementation is that you have two promises, the fullfillment is decided based on the "external" promise without checking the state of the "internal" promise here:

https://github.com/Tolerance/Tolerance/pull/91/files#diff-eefafbb24a67fa568614a664524e1adcR110

As far as I can see you expect the fact that you inject $value (which is a promise) to be automatically resolved (in the indicated state: fulfilled/rejected) while it is not yet settled.

Also, if I am correct we are talking about kind of an integration layer between two kinds of promises. For that I suggest you to check our integration with Guzzle 6: https://github.com/php-http/guzzle6-adapter/blob/master/src/Promise.php

[tyx]

Ok, thanks for you support 👍

I will drop the async support for php-http, I guess I had no other real option.