HttpFulfilledPromise constructor parameter should be mixed

[aferrandini]

Q	A
Bug?	yes
New Feature?	no
Version	1c63817

Actual Behavior

When trying to test an async request with a callable for onFulfilled, if this callable returns something different to a ResponseInterface will fail.

For example, we are using the Promise fullfilled callable to check the response is status code 200 and then convert the received body to another object and returns the other object.

Expected Behavior

The constructor must accept mixed content, not only ResponseInterface subclasses.

Steps to Reproduce

1. Create a Client mock and a ResponseInterface mock, then and the response mock to the client mock.
2. Call sendAsyncRequest, this method should return a Promise, then call method then with a anonymous function for onFullfilled parameter.
3. The anonymous function should return something different to ResponseInterface.

return $response->then(
    function (ResponseInterface $response) use ($request) {
        if (200 <= $response->getStatusCode() && $response->getStatusCode() < 300) {
            return 'success';
        } 

        return 'fail';
    }
);

* This is just an example, in our case, we are deserializing a response body with is a JSON to an object and returning this object.

Possible Solutions

Change the constructor definition to allow mixed data.

<?php

namespace Http\Client\Promise;

//...

final class HttpFulfilledPromise implements Promise
{
    // ...

    /**
     * @param ResponseInterface $response
     */
    public function __construct($response)
    {
        $this->response = $response;
    }

    // ...

[dbu]

by design, the promises must return http responses. this is not a generic promises library but a custom interface for httplug. it is an excellent idea to have your library/application not use http objects to carry information. but rather than building into the httplug library, you should write an adapter type class that internally uses httplug and exposes domain objects to the rest of your code. if you need promises in your code, use a generic promises library and wrap the httplug promise in that promise.

does this make sense? or am i misunderstanding the issue?

[sagikazarmark]

Hi @aferrandini

Thanks for reaching out. May I ask why would you like to use promises like that?

By design promises inside the plugin chain (and during the request lifecycle) should not accept or return any other values than ResponseInterface/HttplugException. This is because we have to keep the HttpAsyncClient contract which states the returned Promise will resolve to one of the above.

Since you need to return an object anyway (not log the response for example, which does not require the response to be there at a specific time, but somewhere in the future), you lose the advantage of an async request, since you need to synchronize at some point.

You can still use async though, but you have to wait for the promise resolution, and then you can do the deserialization.

If you want to see an example how we handle deserialization, take a look at https://github.com/FriendsOfApi/boilerplate

Although this is a limitation of some kind, it allows us to provide a consistent contract. If you absolutely need this inside a plugin, you can try wrapping the returned promise into a custom one, invoke the success and failure handlers of the original one, and do whatever you want from that point.

[aferrandini]

First of all, thanks for your quickly responses.

In our case, we have an abstract client that handles both sync/async requests. In fact, the request always is async, but the abstract client waits for the promise to convert this call to sync.
Using the promise this way, the specific-domain client is using the onfullfilled promis parameter to deserialize the object from the response body at this point or returning the promise with the anonymous function which deserialize the body.

We will try to wrap the promise with a custom promise to handle specific domain objects and create a custom contrat for our code :)

Thanks