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

Request processing refactor #543

Merged
merged 3 commits into from Feb 9, 2021
Merged

Conversation

MiSikora
Copy link
Contributor

πŸ“„ Context

Due to mishaps in #542 I'm putting this for review. If there are any comments I'll fix them in this PR and then create a new one with fixes only that target develop. Below is the copy of description from #542.

ChuckerInterceptor was doing too much and it would be hard to integrate #523 in a sane manner.

πŸ“ Changes

I refactored code responsible for requests processing.

  • Added LimitingSource that replaces method on IOUtils that read limited amount of content. Due to that chucker_body_unexpected_eof message is removed as it is now impossible to read too much from in-memory buffers while processing the response.
  • Added RequestProcessor that moves away request logic from the interceptor.
  • HttpTransaction has isRequestBodyPlainText is now set to false by default and changed to true only when plain text is detected.
  • Added interceptor tests for request processing.
  • Added logging for handling exceptions while processing requests.

πŸ“Ž Related PR

#527

🚫 Breaking

No.

πŸ› οΈ How to test

Added some tests. Other than that, sample app should behave the same.

⏱️ Next steps

Similar PR for response to follow. Also PRs fixing some other issues mentioned in #527.

@cortinico
Copy link
Member

Great cleanup @MiSikora πŸ™ Thanks for moving this forward

Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM πŸ‘

@MiSikora MiSikora changed the base branch from fake-develop to develop February 9, 2021 17:47
@MiSikora MiSikora changed the base branch from develop to fake-develop February 9, 2021 17:49
@MiSikora MiSikora changed the base branch from fake-develop to develop February 9, 2021 17:53
@MiSikora MiSikora removed the ⛔️ do not merge A PR that should not be merged label Feb 9, 2021
@MiSikora MiSikora merged commit 689fc05 into develop Feb 9, 2021
@MiSikora MiSikora deleted the request-processing-refactor branch February 9, 2021 18:00
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

Successfully merging this pull request may close these issues.

None yet

3 participants