-
Notifications
You must be signed in to change notification settings - Fork 5k
Fix NET 7.0 streamed http response reading #80638
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
Conversation
I think some testing is needed for this feature to prevent future regressions. |
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsThe JS stream in the NET 7.0 Blazor runtime blocks until either the stream has ended, or the C# buffer is full, preventing the developer from reading the responses as soon as they are ready. A workaround exists by limiting the read buffer size, but it prevents the usage of The endlessly running Fix for: #79238 Known issues:
|
Waiting until C# buffer is full is wrong in the current code, thanks for figuring it out @rabirland ! I will open my own PR for fixing it, together with another issue and also try to create unit test for it. |
if (remaining_source === 0) { | ||
res.__chunk = await res.__reader.read(); | ||
res.__source_offset = 0; | ||
return 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we could not return zero here as it means end of stream on C# side.
https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readasync?view=net-7.0
I also think we rather keep the loop over __reader.read();
in case that browser returned zero length chunk.
I worry it may happen when server sends zero sized chunk.
I think we need to block the promise/task until we receive at least one byte or until we get done
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to block the promise/task until we receive at least one byte or until we get done.
Yes, unless zero bytes were requested (in which case you can either return immediately or wait for at least one byte to be available but not consume it), Stream.Read must not return 0 unless it's EOF, and Stream.ReadAsync must not complete the task with 0 unless it's EOF.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting until C# buffer is full is wrong in the current code, thanks for figuring it out @rabirland !
You're welcome. To remove the zero return, you gotta bridge the if (!res.body) check. After that, res__reader.read() will block until data is received.
The if (remaining_source === 0) need to re-run the chunk read, instead of returning and waiting for the C# side to call it again.
|
I'm working on it here #80693 |
The JS stream in the NET 7.0 Blazor runtime blocks until either the stream has ended, or the C# buffer is full, preventing the developer from reading the responses as soon as they are ready. A workaround exists by limiting the read buffer size, but it prevents the usage of
StreamReader
s and still causes theStreamContent.ReadAsync()
to not return with a zero.The endlessly running
await StreamContent.ReadAsync(...)
also interferes with the other asynchronous operations, causing them to halt as well.Fix for: #79238
Known issues:
ReadableStreamDefaultReader.read()
also blocks the execution if there is no data available, and currently there is no known API to check if there are bytes received or not.