WyriHaximus / ReactGuzzle

ReactPHP HttpClient Adapter for Guzzle
MIT License
34 stars 5 forks source link

Issues with request options handling and events #5

Closed elazar closed 9 years ago

elazar commented 10 years ago

\WyriHaximus\React\Guzzle\HttpClientAdapter implements \GuzzleHttp\Adapter\AdapterInterface. However, its send() method implementation has an incompatibility with that of AdapterInterface: it includes an $options parameter that AdapterInterface does not.

Request methods in \GuzzleHttp\Client, such as get(), call its createRequest() method, which handles creating a request object that includes the options passed in the call to the request method (i.e. get()). send() is then called and passed that request object, which leaves it to the adapter to handle request options and events. You can see an example of this in the send() implementation of \GuzzleHttp\Adapter\Curl\CurlAdapter, which calls RequestEvents::emitBefore() and other similar methods that trigger events, in particular those listened for by plugins such as the Oauth plugin. Lack of equivalent calls in the send() method of \WyriHaximus\React\Guzzle\HttpClientAdapter currently prevents plugins from working when it's used.

elazar commented 10 years ago

In case you were curious, the adapter is otherwise working fine. I don't know why I didn't see its output in Wireshark, but it is sending a request. The problem is that the lack of events firing prevents the Oauth plugin from working, and because the request isn't signed properly, I'm getting a 400 response from Twitter.

WyriHaximus commented 10 years ago

Makes sense and thanks for pointing this out to me! After I've finished #4 I'll get working on this. Might need to run oauth and such in a child process preventing it from blocking the I/o but I'll make sure it works flawless out of the box.

WyriHaximus commented 10 years ago

@elazar Can you confirm if it's fixed now for you?

elazar commented 10 years ago

@WyriHaximus Just a note, I'm currently blocked from testing this by WyriHaximus/PhergieDns#4.

cboden commented 10 years ago

Possibly related to reactphp/socket-client#11?

elazar commented 9 years ago

This appears to be working. I've got the Twitter plugin able to communicate with the Twitter API. Thanks!

WyriHaximus commented 9 years ago

Awesome :+1: