kazu-yamamoto / http2

HTTP/2.0 library including HPACK
BSD 3-Clause "New" or "Revised" License
86 stars 22 forks source link

Streaming in the client side #41

Closed kazu-yamamoto closed 1 year ago

kazu-yamamoto commented 1 year ago

@epoberezkin This should fix #40.

kazu-yamamoto commented 1 year ago

@epoberezkin If "exceeds maximum frame size" still occurs, it would be appreciated if you added a test case.

epoberezkin commented 1 year ago

This error only happened when I was sending a large body (~256kb) via requestBuilder as lazyByteString builder because requestStreaming was not working. Is it supposed to work? I can check it / write test.

kazu-yamamoto commented 1 year ago
  1. The bug of requestStreaming should be fixed by this PR.
  2. The bug of requestBuilde should be fixed somehow.

Anyway, would you review this PR?

kazu-yamamoto commented 1 year ago

@epoberezkin May I merge this PR?

epoberezkin commented 1 year ago

@kazu-yamamoto streaming is mostly working, so it didn't make it worse, I think :)

I am just now observing some intermittent bug with it I cannot quite put finger on. It seems to only happen when there are two clients connected and streaming concurrently to the same server...

I am not sure yet how to reproduce it in a simple example, but I could send a PR to our code with a sporadically failing test?

epoberezkin commented 1 year ago

sending large lazy bytestrings doesn't work (I tested with 256kb) - I would probably switch to them if they worked.

epoberezkin commented 1 year ago

Sometimes it is hanging on receiving streamed response from server even with one connected client...

epoberezkin commented 1 year ago

Alright, so if in streaming builder in the server I send several 16kb chunks and then send 16 bytes (specifically, I am sending authentication tag from crypto_box encryption) then the client hangs when trying to receive the last 16kb chunk (not 16 bytes).

If I pad the last 16 bytes tag to 16kb and then just throw away extra bytes in the client then everything works.

To reproduce you can just:

streamBody send done = do
  let chunk = replicate 16384 'c'
      tag = replicate 16 't'
  -- I don't think 9 is important here, this is just what I have, the client hangs on receiving the last one
  replicateM 9 $ send chunk 
  send tag
  done

Then when the client receives the last chunk with getResponseBodyChunk it hangs.

The last string it receives from the server in this case is "\NUL\NUL\NUL\v\NUL\NUL\NUL\ACKexceeds maximum frame size"

This is the same error that happens when I try to send lazy bytestrings.

I can check more if it helps.

epoberezkin commented 1 year ago

Also, when sending lots of chunks from the server in a loop, even without any concurrency from the client, for whatever reason it consistently hangs on some chunk when calling send (it never returns), but if I add threadDelay 1000 (1ms) after send it doesn't hang.

Lot's of chunks means, for example 6 responses in tight sequence of: 2 (8mb + 16kb) + 4 (1mb + 16kb).

With smaller sizes I couldn't reproduce it...

I will try to debug what is happening inside the library, maybe you can have a look too.

Thank you!

kazu-yamamoto commented 1 year ago

I have rebased and merged this PR. I don't close this PR to fix the other issue.

epoberezkin commented 1 year ago

great, thank you! looking for the right commit to use now :))...

epoberezkin commented 1 year ago

Found it in master 🤦‍♂️

Should I investigate what is happening in the library or you are looking already? I can point you to our test and tell you what needs to be changed to see both issues.