') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Fix NET 7.0 streamed http response reading by rabirland · Pull Request #80638 · dotnet/runtime · GitHub
Skip to content

Fix NET 7.0 streamed http response reading - #80638

Closed
rabirland wants to merge 1 commit into
dotnet:mainfrom
rabirland:main
Closed

Fix NET 7.0 streamed http response reading#80638
rabirland wants to merge 1 commit into
dotnet:mainfrom
rabirland:main

Conversation

@rabirland

@rabirlandrabirland commented Jan 13, 2023

Copy link
Copy Markdown

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 StreamReaders and still causes the StreamContent.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:

  • As in the original implementation, 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.

@ghostghost added area-System.Runtime.InteropServices.JavaScript community-contribution Indicates that the PR has been added by a community member labels Jan 13, 2023
@dnfadmin

dnfadmin commented Jan 13, 2023•
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@JamesNK

JamesNK commented Jan 14, 2023

Copy link
Copy Markdown
Member

I think some testing is needed for this feature to prevent future regressions.

@pavelsavarapavelsavara self-assigned this Jan 16, 2023
@pavelsavarapavelsavara added this to the 8.0.0 milestone Jan 16, 2023
@pavelsavarapavelsavara added the arch-wasm WebAssembly architecture label Jan 16, 2023
@ghost

ghost commented Jan 16, 2023

Copy link
Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

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 StreamReaders and still causes the StreamContent.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:

  • As in the original implementation, 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.
Author:rabirland
Assignees:pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript, community-contribution

Milestone:8.0.0

@pavelsavara

pavelsavara commented Jan 16, 2023

Copy link
Copy Markdown
Member

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.

pavelsavara
pavelsavara reviewed Jan 16, 2023
View reviewed changes
Comment threadsrc/mono/wasm/runtime/http.ts
if (remaining_source === 0) {
res.__chunk = await res.__reader.read();
res.__source_offset = 0;
return 0;

@pavelsavarapavelsavaraJan 16, 2023•
edited
Loading

Copy link
Copy Markdown
Member

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.

@stephentoubstephentoubJan 16, 2023

Copy link
Copy Markdown
Member

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.

@rabirlandrabirlandJan 16, 2023•
edited
Loading

Copy link
Copy Markdown
Author

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.

@pavelsavara

pavelsavara commented Jan 16, 2023

Copy link
Copy Markdown
Member
[00:57:20] fail: [FAIL] System.Net.Http.Functional.Tests.SocketsHttpHandler_HttpClientHandlerTest.ReadAsStreamAsync_HandlerProducesWellBehavedResponseStream(chunked: True, enableWasmStreaming: True)
[00:57:20] info: Assert.Equal() Failure
[00:57:20] info: Expected: 101
[00:57:20] info: Actual: 0

Log https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-80638-merge-da0dddf4d6fa48fa8f/WasmTestOnBrowser-System.Net.Http.Functional.Tests/1/console.181e1bb5.log?helixlogtype=result

@pavelsavara

pavelsavara commented Jan 16, 2023

Copy link
Copy Markdown
Member

I'm working on it here #80693

@ghostghost locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@stephentoubstephentoubstephentoub left review comments
@lewinglewingAwaiting requested review from lewinglewing is a code owner

Assignees

@pavelsavarapavelsavara

Labels

arch-wasmWebAssembly architecturearea-System.Runtime.InteropServices.JavaScriptcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@rabirland@dnfadmin@JamesNK@pavelsavara@stephentoub