Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streams closed while data still in buffer #49

Closed
calcinai opened this issue Oct 20, 2015 · 3 comments
Closed

Streams closed while data still in buffer #49

calcinai opened this issue Oct 20, 2015 · 3 comments

Comments

@calcinai
Copy link

Hey, Not sure if this is desired behaviour or not, but I've run into an issue where the data in the buffer doesn't ever get written.

As per example:

$connector->create('www.google.com', 80)->then(function (React\Stream\Stream $stream) {
    $stream->write('...');
    $stream->close();
});

From what I can see, the $stream->write() appends the data to the buffer and doesn't do a lot else at that point; it waits for the next tick of the event loop to come and write the buffer out. Because of that, when you follow it immediately with $stream->close(), it closes it and flushes the buffer.

As I said, I'm not sure if this is expected behaviour, and I can see benefits in both. As a work around, I'm doing this:

$connector->create('www.google.com', 80)->then(function (Stream $stream) use ($template) {
    $stream->write('');
    $stream->getBuffer()->on('full-drain', function() use($stream) {
        $stream->close();
    });
});

Is this as simple as a doc update?

@clue
Copy link
Contributor

clue commented Oct 20, 2015

Is this as simple as a doc update?

Yes, this is expected behavior and hence a rather pointless example :-)

Using end() instead of close() would likely resolve this here, I suppose?

Other than that, we should probably add some documentation to the stream component and link there. What do you think?

@calcinai
Copy link
Author

Yeah, that actually makes sense once reading the stream documentation. It all totally makes sense to function the way it does, so I guess just updating the example would be the best thing to do.

Thanks for clarifying this for me.

@clue
Copy link
Contributor

clue commented Nov 21, 2015

Thanks for reporting back @calcinai.

The documentation has now been updated via #46, so I suppose this can be closed 👍

@clue clue closed this as completed Nov 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants