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

Chunked transmission lasts longer than timeout #4214

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sergiuszkierat
Copy link
Member

@sergiuszkierat sergiuszkierat commented Dec 19, 2024

Why I did it?
In order to have a test which might confirm an issue
with an interrupted request

How I did it:
I prepared NettyCatsRequestTimeoutTest with the folloing test scenario:

  • send first chunk (100 bytes)
  • sleep
  • send second chunk (100 bytes)

Closes #4169

@adamw
Copy link
Member

adamw commented Dec 20, 2024

There are two differences compared to the original report:

  1. it uses the play interpreter, not the netty one (there's netty underneath as well, but the interpreter is different)
  2. the request there is interrupted by the client, not by the server because of a timeout. However ... I think we won't be able to simulate this exact scenario, so a request timeout should do the trick as well

*Why I did it?*
In order to have a test which might confirm an issue
with an interrupted request

*How I did it:*
I prepared `NettyCatsRequestTimeoutTest` with the folloing test scenario:
 - send first chunk (100 bytes)
 - sleep
 - send second chunk (100 bytes)
- add PlayServerTest instead of NettyCatsServerTest
- improve fs2 implementation
@sergiuszkierat sergiuszkierat force-pushed the fix/chunked_transmission_timeout branch from 49a1e12 to ad42e13 Compare December 23, 2024 15:38
@sergiuszkierat
Copy link
Member Author

There are two differences compared to the original report:

  1. it uses the play interpreter, not the netty one (there's netty underneath as well, but the interpreter is different)
  2. the request there is interrupted by the client, not by the server because of a timeout. However ... I think we won't be able to simulate this exact scenario, so a request timeout should do the trick as well

Thanks for your precise comment. I've rewritten it to PlayServerTest. Anyway, there is no luck in reproducing issue.

.streamBody(Fs2Streams[IO])(inputStream)
.send(backend)
.map{ response =>
response.code shouldBe StatusCode.Ok
Copy link
Member

Choose a reason for hiding this comment

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

hm well if this test passes, something is wrong - we set the timeout to 1s, so we should never receive a response if it takes 2s to send it? unless the request timeout is for something else?

anyway, this doesn't test the scenario from the test case - where the transmission is interrupted half-way because of connection problems; I don't know if we can simulate this in a test case, but using a timeout is a good approximation. But probably a good way to check if we can at all reproduce the bug is to run: a long-running client sender process; a server process; then kill -9 the client process when it's half-way sending the data, and seeing on the server if received the incomplete data in the server logic

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.

[BUG] Do not parse interrupted request
2 participants