WyriHaximus / react-guzzle-http-client

Meta package for ReactPHP HttpClient adapters/handles for Guzzle 4, 5, and 6.
MIT License
10 stars 8 forks source link

Use clue/buzz instead of react http/client #6

Closed WyriHaximus closed 7 years ago

WyriHaximus commented 8 years ago

clue/buzz wraps react/http-client but adds some extra goodies and makes this package slimmer by taking work out of hand.

donkkill13 commented 7 years ago

Worked for me when I changed, in the RequestFactory, this:

    protected function createSender(array $options, HttpClient $httpClient, LoopInterface $loop)
    {
        $connector = $this->getProperty($httpClient, 'connector');
        if (isset($options['proxy'])) {
            $resolver = $this->extractResolver($connector);
            $connector = (new SocksClient($options['proxy'], $loop, $connector, $resolver))->createConnector();
        }
        return Sender::createFromLoopConnectors($loop, $connector);
    }

to

    protected function createSender(array $options, HttpClient $httpClient, LoopInterface $loop)
    {
        $connector = $this->getProperty($httpClient, 'connector');

        if (isset($options['proxy']) && isset($options['proxyType'])) {
            switch ($options['proxyType'])
            {
                case 'socks' : $connector = new SocksClient($options['proxy'], $connector); break;
                case 'http' : $connector = new ProxyClient($options['proxy'], $connector); break;
            }
        }

        return Sender::createFromLoopConnectors($loop, $connector);
    }

also added

use Clue\React\HttpProxy\ProxyConnector as ProxyClient; at the top

which means in composer.json, you'd need to add

"clue/http-proxy-react": "^0.1.0",

It also requires the proxyType to be defined as I didn't get into checking the proxy and stuff..

doesn't use the DNS either I think.

I'd love for this proxy support to be finished, so this is just something to bring the issue back up I guess.

WyriHaximus commented 7 years ago

@donkkill13 awesome thank you :tada: . I'll try to finish this tomorrow or otherwise later next week and get this new feature out the door :+1: .

WyriHaximus commented 7 years ago

P.S. I was fiddling with this PR last week and found something minor that needs fixing so I'll address that as well.

hotrush commented 7 years ago

@donkkill13 @WyriHaximus tested this approach with http proxy, working ok :+1:

WyriHaximus commented 7 years ago

@hotrush awesome! Thank you for the confirmation 👍

donkkill13 commented 7 years ago

I do have an issue and that is sending info bigger than like 60k bytes for some reason, it just throws "An error has occured in the underlying stream", I tried changing the buffersize to null in react's stream.php, but it didn't work.

Other than that, it is fine :D

WyriHaximus commented 7 years ago

@donkkill13 could you do this with that exception echo (string)$e; and post the (censored where needed) output here? Exceptions are nested and casting them to a string prints all the nested exceptions :smile: .

WyriHaximus commented 7 years ago

Decided to go with the following format for HTTP proxies:

['proxy' => 'http://server.proxy:8080',]

And for SOCKS:

['proxy' => 'socks://server.proxy:8080',]

Not fully there yet, test fail but getting there :smile:

WyriHaximus commented 7 years ago

FYI got both SOCKS and HTTP proxies working. Ironing out some small issues now before pushing the working code to this PR.

WyriHaximus commented 7 years ago

@hotrush && @donkkill13 are you using this package directly or one of my guzzle wrappers? Asking because this feature will mean a BC break so tagging a new major release. Users of my guzzle wrappers won't notice but if you use it directly you probably will

donkkill13 commented 7 years ago

I am using your HttpClientAdapter @ https://github.com/WyriHaximus/react-guzzle-psr7/tree/master/src

Had to download the file and use it separately so your composer.json didnt interfer with my downloaded libs :D

And Ill print the error later today, just saw your post

donkkill13 commented 7 years ago

The error is:

Next RuntimeException: Unable to write to stream: fwrite(): SSL operation failed with code 1. OpenSSL Error messages:
error:1409F07F:SSL routines:ssl3_write_pending:bad write retry in /home/customer/discord/vendor/react/stream/src/Buffer.php:121
Stack trace:
#0 [internal function]: React\Stream\Buffer->handleWrite(Resource id #218, Object(React\EventLoop\StreamSelectLoop))
#1 /home/customer/discord/vendor/react/event-loop/src/StreamSelectLoop.php(240): call_user_func(Array, Resource id #218, Object(React\EventLoop\StreamSelectLoop))
#2 /home/customer/discord/vendor/react/event-loop/src/StreamSelectLoop.php(201): React\EventLoop\StreamSelectLoop->waitForStreamActivity(NULL)
#3 /home/customer/discord/src/DiscordBot/Ratchet/Server/IoServer.php(84): React\EventLoop\StreamSelectLoop->run()
#4 /home/customer/discord/launchBotServer.php(41): Ratchet\Server\IoServer->run()
#5 {main}
WyriHaximus commented 7 years ago

Had to download the file and use it separately so your composer.json didnt interfer with my downloaded libs :D

How does my composer.json interfer with your downloaded libs?

hotrush commented 7 years ago

@WyriHaximus i am using guzzle 6 adapter

WyriHaximus commented 7 years ago

Great thanks 👍

donkkill13 commented 7 years ago

I'm not exactly sure, but I know of one reason and it was that it required this lib, which I needed to edit :D

The other reason was that I had guzzlehttp 6.2, which shouldnt be a problem though

WyriHaximus commented 7 years ago

@donkkill13 @hotrush could you test the latest changes I've pushed into this PR? For my they work great but I've only tested it with SSH (SOCKS) and the Synology DSM Proxy server package (HTTP).

hotrush commented 7 years ago

@WyriHaximus i am receiving next error

PHP Catchable fatal error:  Argument 3 passed to WyriHaximus\React\Guzzle\HttpClient\RequestFactory::create() must be an instance of React\Dns\Resolver\Resolver, instance of React\HttpClient\Client given, called in /home/rush/Projects/tests/react-scrapper-test/vendor/wyrihaximus/react-guzzle-psr7/src/HttpClientAdapter.php on line 139 and defined in /home/rush/Projects/tests/react-scrapper-test/vendor/wyrihaximus/react-guzzle-http-client/src/RequestFactory.php on line 42

code:

use GuzzleHttp\Client as GuzzleClient;
use GuzzleHttp\HandlerStack;
use React\Dns\Resolver\Factory;
use WyriHaximus\React\GuzzlePsr7\HttpClientAdapter;

$dnsResolverFactory = new Factory();
$dnsResolver = $dnsResolverFactory->createCached('8.8.8.8', $this->loop);
$handler = new HttpClientAdapter($this->loop, null, $dnsResolver);
$stack = HandlerStack::create($handler);
$this->client = new GuzzleClient([
    'handler' => $stack,
]);
WyriHaximus commented 7 years ago

@hotrush my bad, the code contains a breaking change, I've create a PR to resolve that on the Guzzle 6 adapter: https://github.com/WyriHaximus/react-guzzle-psr7/pull/19

hotrush commented 7 years ago

@WyriHaximus works for me with http proxies. Great job :+1: waiting for tagging this)

WyriHaximus commented 7 years ago

@hotrush awesome, glad to hear :smiley:

hotrush commented 7 years ago

@WyriHaximus Hi, how this going?) When do you plan to release a new version?

WyriHaximus commented 7 years ago

@hotrush somewhere this week, need to restore one feature to keep it from turning into a BC break :smile:

hotrush commented 7 years ago

@WyriHaximus perfect! thanks :+1:

hotrush commented 7 years ago

I'm really looking forward to this release, but don't think that can help a lot :neutral_face:

WyriHaximus commented 7 years ago

4.0.0

hotrush commented 7 years ago

Hi @WyriHaximus. Great job, thanks for release. I think you need to do a change in your https://github.com/WyriHaximus/react-guzzle-psr7 package to be compatitable with new release.

Problem 1
    - Installation request for wyrihaximus/react-guzzle-psr7 ^1.0 -> satisfiable by wyrihaximus/react-guzzle-psr7[1.0.0].
    - wyrihaximus/react-guzzle-psr7 1.0.0 requires wyrihaximus/react-guzzle-http-client ^3.0.0 -> satisfiable by wyrihaximus/react-guzzle-http-client[3.0.0, 3.0.1, 3.0.2, 3.0.3] but these conflict with your requirements or minimum-stability.
WyriHaximus commented 7 years ago

@hotrush yes there is a pullrequest open for it: https://github.com/WyriHaximus/react-guzzle-psr7/pull/19

WyriHaximus commented 7 years ago

@hotrush https://github.com/WyriHaximus/react-guzzle-psr7/releases/tag/2.0.0

hotrush commented 7 years ago

@WyriHaximus you rock!