Rewind streams in the formatter

Question

Rewind streams in the formatter

Closed this issue 9 years ago · 6 comments

Q	A
Bug?	yes
New Feature?	no
Version	1.3.0

Actual Behavior

From: php-http/HttplugBundle#84 (comment)

Just see the formatter and how the request / response is read.

As far as i have see, this formatter mess up with the stream, as __toString is called and the stream is not rewind so the content is retrieve only in the first stream ?

(I may have miss something again ^^)

Expected Behavior

Streams should be rewinded after they are read.

Answer 1 · 2016-07-15T10:26:26.000Z

Rewinding the stream is not hard, IF the stream is rewindable :)

If not the current design is broken as we have to replace the stream in the request passed to the formatter but since the request is immutable we cannot update the stream easily.

Then there is some work to do in the plugin calling the formatter which holds the responsability of rewinding / replacing the stream.

Answer 2 · 2016-07-15T10:27:50.000Z

Also cloning / rewinding the stream is something that it's used in the cache plugin, maybe we can refactor this part to have an helper stream / class doing this ?

Answer 3 · 2016-07-15T10:35:19.000Z

👍 For a helper class doing this. I did the same hack in an other PR: https://github.com/KnpLabs/php-github-api/pull/389/files#diff-192f7f95a475a8e6e67b97d1c5125367R22

Not too sure how to solve the broken design though... Maybe do not read the stream if it not is rewindable/seekable?

Answer 4 · 2016-07-15T14:08:00.000Z

Or copy the stream.

Answer 5 · 2016-07-15T14:10:58.000Z

Or copy the stream.

we should try to avoid that. the memory impact can be massive. i think we should by default leave the body alone, especially if its not rewindable, and probably have 3 options: don't show body, show body if stream can rewind, always show body (and copy streams if needed)

Answer 6 · 2016-07-15T14:25:18.000Z

We cant copy the string because the Request is immutable.

I've created a PR.