Rewind body resource after logged it.

[nuvolapl]

No description provided.

[cjost1988]

Thank you for contributing.
Please consider our contribution guideline: https://github.com/Mapudo/guzzle-bundle/blob/master/CONTRIBUTING.md

I'll have a look tomorrow.

[kobelobster]

@cjost1988 This would also solve #2

@nuvolapl Thanks for your contribution. We'll look into this PR today.

[nuvolapl]

@cjost1988 @tzfrs I don't know why i push both commit in one PR. Feel free to split it.

I didn't write tests because of... laziness :)

[kobelobster]

@nuvolapl Sorry for the late reply.

I'll be closing this issue for multiple reasons.

1. As discussed before, this PR contains no tests and solves two issues in one PR.
2. We discussed your approach about rewinding the body and came to the conclusion that a rewind after finishing readinging the body is the best idea. Instead our approach would be, that if someone chains his own middleware to the stack, he should do the rewind there. The reason for this is, that otherwise we could have performance issues by rewinding the body everytime. So, even if there's no other middleware we would still perform the rewind of the body.

• This is, of course, only if it isn't solving a bug/security issue we aren't aware of. If so, we'd love to discuss it.

3. We thought about the approach of checking the content type in twig and while this is a view problem, we don't think it would be the best approach to have logic inside the template. Furthermore, when another content type (e.g. XML, images etc.) would be supported by us as well, we would need to add another elseif case.

However, the issues you solved with this PR where issues we had on our todo list and since we now know that this is a wanted feature we would develop it soon. I hope I can finish this feature request this week, otherwise I'd finish this on the weekend.

[wizbit]

Having found this pull request and being annoyed that logging does not rewind the response, I looked up guzzle's github page to find the best way to read the contents.

(string)$response->getBody(); will rewind the response body for you.

Adding this comment in case it helps anyone else that finds it.

[kobelobster]

Hi @wizbit

We're open to change how the rewinding process works (end of middleware instead of start) but we have a hard time seeing which benefits it would give us. As said before, the downside to rewinding the body is that it will be done in every middleware even if

a) There is no further middleware
b) The middleware that will be called after the current one doesn't need the body.

That's why we think it's the best to rewind the body only on your middleware if you need it.

But, like I said, we're open to hear WHY you think the other approach would be better. We're def. open for that! 👍

[cjost1988]

@wizbit Thank you for participating in discussion.

We discussed rewinding the response stream after logging and didn't found any reason to do so. We don't rewind since it might be a performance issue rewinding every response even if the response is not used anymore after the middleware used it. Imagine requests that do not have a response that matter like CREATE: 201, after the logging it's very likely that no one is interested in the response content.

If we would change the responsibility for resetting the response pointer every client/middleware has to ensure that the pointer is reseted at the end of it's runtime what would also mean that any last middleware or client would rewind the response for nothing.

It would be interesting why you want that the client/middleware has to rewind the response after reading instead before, please let us know.