reactphp / http

Event-driven, streaming HTTP client and server implementation for ReactPHP.
https://reactphp.org/http/
MIT License
740 stars 142 forks source link

unexpected stream closing #533

Closed unglaublicherdude closed 2 months ago

unglaublicherdude commented 2 months ago

Hi, I already opened a Discussion here, because I am not sure if this is a bug or expected behavious.

You can find the example code in our repository. Basically what I am confused about, is this line becuase after a http-request with a second Browser instance (yes I know, they share the same Loop) the stream from request 1 is closed without me doing anything with it.

        $browser1 = new Browser();
        $browser2 = new Browser();

        $response1 = await($browser1->requestStreaming("GET", "https://secure.eicar.org/eicar.com.txt"));
        $body1 = $response1->getBody();
        $this->assertEquals(true, $body1->isReadable());

        $response2 = await($browser2->requestStreaming("GET", "https://secure.eicar.org/eicar.com.txt"));
        $this->assertEquals(false, $body1->isReadable());
        $body2 = $response2->getBody();
        $this->assertEquals(true, $body2->isReadable());

Is there any way we can prevent that besides just doing a pause() and resume()? That SDK is currently build, so that customers create and provide the stream to our ForStream function, so we don't have that in our hands yet.

If there is now other way, my Idea would be to Wrap provide our own implementation from the ReadableStreamInterface enforcing the a pause on __construct and resume when actually doing something with it.

unglaublicherdude commented 2 months ago

I added now a

$body1->pause();

after the request, but that doesn't change the behaviour.

SimonFrings commented 2 months ago

Hey @unglaublicherdude, thanks for bringing this up :+1:

[...] the stream from request 1 is closed without me doing anything with it.

That is actually the correct behavior here. Breaking down your example above, you start by sending out a request and waiting for the streaming response to come in. Once this happens, you'll start receiving data until all data has been received. Checking if the stream is readable will be true in this case.

Then you're starting to send out a second HTTP request and wait for the next streaming response to arrive. This takes some time and the first request is already finished, so as long as we don't take control of the first stream, it will be closed as there's nothing more to do here. You can also listen to any events on this stream to see what's going on behind the scenes, see https://github.com/reactphp/stream?tab=readme-ov-file#readablestreaminterface for more details.

Is there any way we can prevent that besides just doing a pause() and resume()? [...]

Using pause() and resume() should work in this case. This also opens up the question if you really want to use a streaming approach here, but I can't really recommend much without knowing the use case and what you're trying to achieve.

If you're looking for even more help with this, we can also schedule a call and take a closer look at this together. We helped others through consulting with similar questions in the past, so if this is something you're interested in, you can visit https://clue.engineering/#appointment.

Hi, I already opened a Discussion here, because I am not sure if this is a bug or expected behavious.

I saw this other ticket as well and I think we should continue our conversation in there. This isn't currently a bug in our HTTP component, so I would close this ticket here and post my answer in https://github.com/orgs/reactphp/discussions/578 as well.

unglaublicherdude commented 2 months ago

Have you seen the latest commit on my original PR? I tried calling

$body1->pause(); 

but that had no effect on the behaviour.

unglaublicherdude commented 2 months ago

I have to use streaming, because the SDK I work on is basically just tunneling the files trough. The files come from anywhere and are put to a specific API to get some information about the file (verdict, file-type, mime-type), so I want to prevent buffering in the SDK because it doesn't need the files content.

Do you know why the pause does not have an effect? Is that a special for the ReadableBodyStream?