Skip to content

Commit faf594a

Browse files
Fix streaming SSR timing issue (#51333)
# Fix streaming SSR timing issue On certain rare occasions, streaming SSR could produce a blank page due to a timing issue. ## Description If a page component: * Has `@attribute [StreamRendering]` * Is being visited through enhanced navigtion * Has an `OnInitializedAsync` whose returned task is not pre-completed but does complete very quickly (e.g., `await Task.Yield()`) ... then there was a race condition. On rare occasions (seems to be about 1 in 20 page loads), the result would be a blank page. This didn't show up in our E2E tests because they always wait for fairly long async tasks so that the browser automation had time to observe the loading states. This PR adds a new E2E test that does recreate the issue and reliably fails - it does many, many page loads and basically always hits the problem at least once without the fix included here. Fixes #51345 ## Customer Impact Without this fix, a particular combination of features would be unreliable. ## Regression? - [ ] Yes - [x] No [If yes, specify the version the behavior has regressed from] ## Risk - [ ] High - [ ] Medium - [x] Low The only change is to fix what is definitely a bug. It's hard to see how this would have a negative effect, as no other code is built on top of this. It's not a public API - it's just part of how the Razor Components endpoint works. ## Verification - [x] Manual (required) - [x] Automated ## Packaging changes reviewed? - [ ] Yes - [ ] No - [x] N/A
1 parent cbfc558 commit faf594a

File tree

8 files changed

+100
-12
lines changed

8 files changed

+100
-12
lines changed

src/Components/Endpoints/src/RazorComponentEndpointInvoker.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ await EndpointHtmlRenderer.InitializeStandardComponentServicesAsync(
128128
}
129129
else
130130
{
131-
await _renderer.EmitInitializersIfNecessary(context, bufferWriter);
131+
_renderer.EmitInitializersIfNecessary(context, bufferWriter);
132132
}
133133

134134
// Emit comment containing state.

src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.Streaming.cs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ public void InitializeStreamingRenderingFraming(HttpContext httpContext, bool is
3939

4040
public async Task SendStreamingUpdatesAsync(HttpContext httpContext, Task untilTaskCompleted, TextWriter writer)
4141
{
42+
// Important: do not introduce any 'await' statements in this method above the point where we write
43+
// the SSR framing markers, otherwise batches may be emitted before the framing makers, and then the
44+
// response would be invalid. See the comment below indicating the point where we intentionally yield
45+
// the sync context to allow SSR batches to begin being emitted.
46+
4247
SetHttpContext(httpContext);
4348

4449
if (_streamingUpdatesWriter is not null)
@@ -56,9 +61,11 @@ public async Task SendStreamingUpdatesAsync(HttpContext httpContext, Task untilT
5661

5762
try
5863
{
59-
await writer.WriteAsync(_ssrFramingCommentMarkup);
60-
await EmitInitializersIfNecessary(httpContext, writer);
61-
await writer.FlushAsync(); // Make sure the initial HTML was sent
64+
writer.Write(_ssrFramingCommentMarkup);
65+
EmitInitializersIfNecessary(httpContext, writer);
66+
67+
// At this point we yield the sync context. SSR batches may then be emitted at any time.
68+
await writer.FlushAsync();
6269
await untilTaskCompleted;
6370
}
6471
catch (NavigationException navigationException)
@@ -77,15 +84,15 @@ public async Task SendStreamingUpdatesAsync(HttpContext httpContext, Task untilT
7784
}
7885
}
7986

80-
internal async Task EmitInitializersIfNecessary(HttpContext httpContext, TextWriter writer)
87+
internal void EmitInitializersIfNecessary(HttpContext httpContext, TextWriter writer)
8188
{
8289
if (_options.JavaScriptInitializers != null &&
8390
!IsProgressivelyEnhancedNavigation(httpContext.Request))
8491
{
8592
var initializersBase64 = Convert.ToBase64String(Encoding.UTF8.GetBytes(_options.JavaScriptInitializers));
86-
await writer.WriteAsync("<!--Blazor-Web-Initializers:");
87-
await writer.WriteAsync(initializersBase64);
88-
await writer.WriteAsync("-->");
93+
writer.Write("<!--Blazor-Web-Initializers:");
94+
writer.Write(initializersBase64);
95+
writer.Write("-->");
8996
}
9097
}
9198

@@ -286,7 +293,7 @@ private static bool IsProgressivelyEnhancedNavigation(HttpRequest request)
286293
{
287294
// For enhanced nav, the Blazor JS code controls the "accept" header precisely, so we can be very specific about the format
288295
var accept = request.Headers.Accept;
289-
return accept.Count == 1 && string.Equals(accept[0]!, "text/html;blazor-enhanced-nav=on", StringComparison.Ordinal);
296+
return accept.Count == 1 && string.Equals(accept[0]!, "text/html; blazor-enhanced-nav=on", StringComparison.Ordinal);
290297
}
291298

292299
private readonly struct ComponentIdAndDepth

src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,12 @@ protected override Task UpdateDisplayAsync(in RenderBatch renderBatch)
139139

140140
if (_streamingUpdatesWriter is { } writer)
141141
{
142+
// Important: SendBatchAsStreamingUpdate *must* be invoked synchronously
143+
// before any 'await' in this method. That's enforced by the compiler
144+
// (the method has an 'in' parameter) but even if it wasn't, it would still
145+
// be important, because the RenderBatch buffers may be overwritten as soon
146+
// as we yield the sync context. The only alternative would be to clone the
147+
// batch deeply, or serialize it synchronously (e.g., via RenderBatchWriter).
142148
SendBatchAsStreamingUpdate(renderBatch, writer);
143149
return FlushThenComplete(writer, base.UpdateDisplayAsync(renderBatch));
144150
}

src/Components/Web.JS/dist/Release/blazor.web.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Components/Web.JS/src/Services/NavigationEnhancement.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ export async function performEnhancedPageLoad(internalDestinationHref: string, i
162162
headers: {
163163
// Because of no-cors, we can only send CORS-safelisted headers, so communicate the info about
164164
// enhanced nav as a MIME type parameter
165-
'accept': 'text/html;blazor-enhanced-nav=on',
165+
'accept': 'text/html; blazor-enhanced-nav=on',
166166
},
167167
}, fetchOptions));
168168
let isNonRedirectedPostToADifferentUrlMessage: string | null = null;

src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavigationTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public void EnhancedNavRequestsIncludeExpectedHeaders()
8282
// Specifying text/html is to make the enhanced nav outcomes more similar to non-enhanced nav.
8383
// For example, the default error middleware will only serve the error page if this content type is requested.
8484
// The blazor-enhanced-nav parameter can be used to trigger arbitrary server-side behaviors.
85-
Assert.Contains("accept: text/html;blazor-enhanced-nav=on", allHeaders);
85+
Assert.Contains("accept: text/html; blazor-enhanced-nav=on", allHeaders);
8686
}
8787

8888
[Fact]

src/Components/test/E2ETest/ServerRenderingTests/StreamingRenderingTest.cs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Globalization;
45
using System.Net.Http;
6+
using System.Net.Http.Headers;
57
using System.Text;
8+
using System.Text.RegularExpressions;
69
using Components.TestServer.RazorComponents;
710
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
811
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
@@ -275,4 +278,42 @@ public void CanStreamDirectlyIntoSectionContentConnectedToNonStreamingOutlet()
275278
Navigate($"{ServerPathBase}/streaming-with-sections");
276279
Browser.Equal("This is some streaming content", () => Browser.Exists(By.Id("streaming-message")).Text);
277280
}
281+
282+
[Fact]
283+
public async Task WorksWithVeryBriefStreamingDelays()
284+
{
285+
// First check it works in the browser
286+
Navigate($"{ServerPathBase}/brief-streaming");
287+
var header = Browser.Exists(By.Id("brief-streaming"));
288+
for (var i = 1; i < 20; i++)
289+
{
290+
Browser.FindElement(By.LinkText("Load this page")).Click();
291+
292+
// Keep checking the same header to show this is always enhanced nav
293+
Assert.Equal("Brief streaming", header.Text);
294+
295+
Browser.True(() =>
296+
{
297+
var loadCount = int.Parse(Browser.FindElement(By.Id("load-count")).Text, CultureInfo.InvariantCulture);
298+
return loadCount >= i;
299+
});
300+
}
301+
302+
// That's not enough to be sure it was really correct, since it might
303+
// work in the browser even if the SSR framing is emitted in the wrong
304+
// place depending on exactly where it was emitted. To be sure, we'll
305+
// also validate the HTML response directly.
306+
var url = Browser.Url;
307+
var httpClient = new HttpClient();
308+
for (var i = 0; i < 100; i++)
309+
{
310+
// We expect to see the SSR framing marker right before the first <blazor-ssr>
311+
var req = new HttpRequestMessage(HttpMethod.Get, url);
312+
req.Headers.Accept.Clear();
313+
req.Headers.Add("accept", "text/html; blazor-enhanced-nav=on");
314+
var response = await httpClient.SendAsync(req);
315+
var html = await response.Content.ReadAsStringAsync();
316+
Assert.Matches(new Regex(@"</html><!--[0-9a-f\-]{36}--><blazor-ssr>"), html);
317+
}
318+
}
278319
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
@page "/brief-streaming"
2+
@attribute [StreamRendering]
3+
4+
<h3 id="brief-streaming">Brief streaming</h3>
5+
6+
<p>
7+
At one point there was a bug whereby, if streaming was enabled but only waited
8+
for a very short period, it could insert the SSR framing markers in the wrong place,
9+
making the output corrupt and causing the UI to be replace with a blank page.
10+
</p>
11+
<p>
12+
The test loads this page via enhanced nav many times, validating it always loads.
13+
</p>
14+
15+
<a href="brief-streaming">Load this page</a>
16+
17+
@if (isLoaded)
18+
{
19+
<p>
20+
Load counter: <span id="load-count">@loadCount</span>
21+
</p>
22+
}
23+
24+
@code {
25+
static int loadCount;
26+
bool isLoaded;
27+
28+
protected override async Task OnInitializedAsync()
29+
{
30+
await Task.Yield();
31+
loadCount++;
32+
isLoaded = true;
33+
}
34+
}

0 commit comments

Comments
 (0)