[httpclient] Don't consume HTTP responses#502
Merged
baev merged 1 commit intoMar 26, 2021
Conversation
Contributor
Author
|
Something I noticed here: Is it a good idea to preserve requests/responses of any size? Maybe add a default limit to ~100kB, so the user won't shoot himself in the foot if tests contain large requests/responses? |
Contributor
Author
|
@baev Ping? I cannot fix the one failing check because I cannot change labels on the issue. |
baev
approved these changes
Mar 26, 2021
Member
|
closed & reopened to trigger actions |
Member
Yeah, thats actually sounds right. It would be a nice to have a possibility to configure body size limit with some suitable defaults. Feel free to provide a PR with that change ;) |
Member
|
@TobiX thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Since the
LoggableEntitydefined inAllureHttpClientResponsedoesn't overwrite all methods ofHttpEntityWrapper, there are code paths around it, which modifies the behaviour of response objects when this interceptor is attached to the HTTP client. I noticed this with code which was usingBufferedHttpEntitywhen reading from the responce and running into "Stream already consumed" errors...This change does away with the custom entity wrapper and uses
BufferedHttpEntity, which is designed for exactly this usecase. As an additional optimization, we don't replace theHttpEntityat all if the returned entity is already "repeatable".Checklist