Fix TempData and SupplyParameterFromSession persistence for streaming SSR case#66832
Fix TempData and SupplyParameterFromSession persistence for streaming SSR case#66832dariatiurina wants to merge 17 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes persistence timing for [SupplyParameterFromSession] and TempData during streaming server-side rendering (SSR) by moving persistence from HttpResponse.OnStarting callbacks to explicit persistence calls made after rendering/streaming completes.
Changes:
- Added explicit post-render persistence calls for session-supplied cascading values and TempData in
RazorComponentEndpointInvoker. - Removed
Response.OnStarting-based persistence registration fromSessionCascadingValueSupplierand TempData creation. - Updated session-related unit tests to call
PersistAllValues()explicitly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Components/Endpoints/src/RazorComponentEndpointInvoker.cs | Adds explicit session + TempData persistence after rendering/streaming completes. |
| src/Components/Endpoints/src/SessionCascadingValueSupplier.cs | Removes OnStarting registration so persistence is driven externally. |
| src/Components/Endpoints/src/TempData/TempDataProviderServiceCollectionExtensions.cs | Removes OnStarting persistence and adds PersistTempData(HttpContext) helper. |
| src/Components/Endpoints/test/Session/SessionSubscriptionTest.cs | Updates subscription test to use explicit persistence instead of firing OnStarting. |
| src/Components/Endpoints/test/Session/SessionCascadingValueSupplierTest.cs | Renames/updates test to validate explicit persistence behavior. |
…hub.com/dariatiurina/aspnetcore into 66745-streamingssr-tempdata-sessiondata
ilonatommy
left a comment
There was a problem hiding this comment.
I will divide my review into smaller parts:
- Temp data persistance happens too early.
- Session storage persistance happens too early.
The comments focus on 1) for now.
ilonatommy
left a comment
There was a problem hiding this comment.
One more pass for TempData
| // so that TempData / [SupplyParameterFromSession] values written during async | ||
| // rendering can still be persisted after Response.HasStarted. No-op when session | ||
| // middleware is not registered. | ||
| TryRegisterSessionEstablishment(context); |
There was a problem hiding this comment.
This now runs for every component endpoint even if the endpoint doesn't use the supply-param storage. We could avoid it by calling it in subscription methods. We can move TryRegisterSessionEstablishment to SessionCascadingValueSupplier. Sessions supplier's responsibility is connected with session establishment so a method releasing cookie matches that responsibility.
I'm not sure though if it won't be too late, please check the case of streaming deferred children. Add a test: SSR page with streaming that renders a streaming child component. That child starts streaming in OnInitializedAsync and holds SupplyParameterFromTempData.
In case that scenario would fail, please try to think how to avoid over-releasing the cookie, we don't want behavioral change for endpoints not using this feature.
I was considering if we should prevent this call in non-streaming scenarios but there's no need if we already move it to subscription. The cookie must be released in these cases anyway so it's fine to do it a bit earlier.
There was a problem hiding this comment.
I moved cookie creation to GetOrCreateTempData and CreateSubscription. Now only when TempData and [SupplyParameterFromSession] are registered and called, we create cookie for session. This does break case, when component renders in the streaming context (see StreamingSSR_DeferredChildSubscription_DoesNotPersistSession_OnFirstRequest test that was added in this PR). This is much less of a problem then enabling cookies for every page, when users enable session for their application.
There was a problem hiding this comment.
This does break case, when component renders in the streaming context
If we continue trying to have this feature working in streaming then we have to clearly define what scenarios are supported. Docs need clear information when it's safe to use the parameter or prevent them from using the parameter in scenarios that are silently no-op.
There was a problem hiding this comment.
Yep. Compare the behavior against MVC/RazorPages after await FlushAsync() on a razor page / mvc view
There was a problem hiding this comment.
@javiercn From my investigation MVC TempData just silently skips persisting elements after response has started.
|
@ilonatommy Here are options to consider for our problem. I am open to dropping support based on what MVC does and just improve handling (e.g. right now on the main we will encounter runtime error and we should log a warning in that case). How streaming SSR gets enabledStreaming is opt-in per component (subtree) via
Opt out for a subtree with The problem with session-backed state
The fix lives in
The helper short-circuits when the response has already started: if (session is null || context.Response.HasStarted)
{
return;
}So persistence works only when the subscription / cascading-value resolution happens before the first streaming flush. SupportedDeclaring This is what @page "/streaming-session-persistence"
@attribute [StreamRendering]
@code {
[SupplyParameterFromSession] public string? Email { get; set; }
[SupplyParameterFromTempData] private string SupplyParameterFromTempDataValue { get; set; } = string.Empty;
[CascadingParameter] public ITempData? TempData { get; set; }
protected override async Task OnInitializedAsync()
{
await Task.Delay(100); // first flush happens here
Email = "set-during-streaming";
SupplyParameterFromTempDataValue = "tempdata-set-during-streaming";
TempData!["Message"] = "streaming-tempdata-message";
}
}Non-streaming components are trivially supported as well — the response is buffered, so the cookie callback always fires in time. Not supportedIf the first use of session-backed state only happens in a part of the tree that materializes after the first flush (e.g. a child component conditionally rendered after the parent's This is what @* Parent — streaming; child only mounts after the first await *@
@page "/streaming-parent-with-deferred-child"
@attribute [StreamRendering]
@if (_done) { <StreamingDeferredSessionChild /> } else { <p>Streaming...</p> }
@code {
bool _done;
protected override async Task OnInitializedAsync() { await Task.Delay(50); _done = true; }
}@* Child — its [SupplyParameterFromSession] is only subscribed AFTER the parent flushed *@
@code {
[SupplyParameterFromSession] public string? DeferredChildEmail { get; set; }
protected override async Task OnInitializedAsync() { /* write after own await */ }
}Same limitation applies to a TempData cascading value first resolved in a deferred subtree. How MVC handles the same problemMVC has the same HTTP constraint and the same failure mode: if TempData is mutated after the response has started (e.g. after Guidance for users about the deferred-subscription limitation
|
I think the proposed rule really covers the supported scenarios. Suggestion: "first initial render tree" is an implementation detail of blazor, we could rephrase it to: With examples that you prepared it should be understandable.
Child component rendered in top-level (not inside an async
Prerendering doesn't stream by default if the component doesn't set |
|
@javiercn After discussion with Ilona I decided to go with analyzer that will tell users that their |
|
A few things here:
|
acfd2ea to
295808b
Compare
|
In the end I decided that doing analyzers can be too vague (I started doing them and got stuck on trying to write short and easily understandable message for this case), so we will have runtime warning that will tell users that we silently skip over persisting values and coverage in docs to tell of this complication. |
Fix TempData and SupplyParameterFromSession persistence for streaming SSR case
Summary
Fixes a bug where
[SupplyParameterFromSession],[SupplyParameterFromTempData], and theITempDatacascading parameter values set during streaming SSR were silently lost. Persistence previously relied onResponse.OnStartingcallbacks, which fire just before the first response chunk is flushed — so any values modified later (after anawaitinOnInitializedAsync, while streaming chunks are being sent) were never written.The fix moves persistence to run after all rendering (including streaming) completes, and pre-issues the session cookie before streaming begins so that
Set-Cookieheaders are not blocked byResponse.HasStarted.Why I think it's acceptable to always have session established, when customers set session in their app, because it will be established anyway in most cases and it is not a big memory or performance hit.
Changes
RazorComponentEndpointInvoker.csTryRegisterSessionEstablishment(HttpContext)registers aResponse.OnStartingcallback that performs a no-opsession.Set(...) + session.Remove(...)on a sentinel key (__AspNetCore.Components.Endpoints.SessionEstablishment). This flips the session middleware's establishment gate so the session cookie is issued before the first response chunk flushes — enabling server-side session writes to succeed after streaming begins. No-op when session middleware is not registered.SessionCascadingValueSupplier.PersistAllValues()(when the supplier is registered)TempDataProviderServiceCollectionExtensions.PersistTempData(context)SessionCascadingValueSupplier.cs— Removed the_onStartingRegisteredfield and theResponse.OnStarting(PersistAllValues)registration that used to happen insideCreateSubscription. Persistence is now driven explicitly by the invoker after rendering completes.TempDataProviderServiceCollectionExtensions.csResponse.OnStarting(...)callback that used to calltempDataService.SaveinGetOrCreateTempData.PersistTempData(HttpContext)— retrieves the per-requestITempDatafromHttpContext.Itemsand callsTempDataService.Save. If the activeITempDataProviderisCookieTempDataProviderandResponse.HasStartedis true, the method logs a warning (newCookieTempDataNotPersistedAfterResponseStartedevent) and returns without saving, since cookie TempData cannot work once response headers are frozen.partialto host theLoggerMessagesource-generated logger.TempDataService.cs—Savenow acceptsITempDatainstead of the concreteTempDatatype, using pattern matching (tempData is not TempData concrete || !concrete.WasLoaded) to guard theSave()call. This makes the signature match the new persistence entry point.Testing
New E2E tests in
StreamingSessionPersistenceTest.cs(usesRazorComponentEndpointsNoInteractivityStartup, enables--UseSessionStorageTempDataProvider=trueand--UseSession=true, and clears the.AspNetCore.Sessioncookie per test):StreamingSSR_PersistsSupplyParameterFromSession_AfterAsyncRenderingStreamingSSR_PersistsSupplyParameterFromTempData_AfterAsyncRenderingStreamingSSR_PersistsTempDataCascadingParameter_AfterAsyncRenderingNew negative E2E test in
TempDataCookieTest.cs:StreamingSSR_CookieTempData_DoesNotPersistValuesWrittenAfterFirstFlush— asserts that cookie-based TempData written during streaming is not persisted (expected behavior — the new warning is logged).New test page
StreamingSessionPersistence.razor([StreamRendering]) — writes a[SupplyParameterFromSession]value, a[SupplyParameterFromTempData]value, and anITempDatacascading parameter value afterawait Task.Delay(100)inOnInitializedAsync. The await is intentional: it puts the writes after the streaming phase has begun, which is the scenario that used to silently drop values.Updated unit tests
SessionCascadingValueSupplierTest— addedPersistAllValues_KeepsKey_WhenCallbackReturnsValue; renamedSetRequestContext_DoesNotRegisterOnStarting_UntilSubscriptionCreatedtoSetRequestContext_DoesNotPersist_UntilExplicitlyCalledto reflect the new explicit-call model.SessionSubscriptionTest.CreateSubscription_RegistersValueCallbackAndReturnsSubscription— replaced theFireOnStartingAsync()step with a directawait _supplier.PersistAllValues().SessionStorageTempDataProviderTest.Save_RemovesSessionKey_WhenNoDataToSaverenamed toSave_RemovesSessionEntry_WhenNoDataToSaveand now also asserts that reloading yields empty TempData.Behavior Change to Call Out
The session cookie (
.AspNetCore.Session) is now always issued on responses served by the Razor Components endpoint when session middleware is registered, even if the user code never callsISession.Set(...)or writes a[SupplyParameterFromSession]/ TempData value. This is the deliberate effect ofTryRegisterSessionEstablishment: it flips the session middleware's establishment gate duringOnStarting(via a no-opSet+Remove) so that subsequent session writes performed after streaming begins can still be persisted. Apps that previously never received a session cookie from these endpoints will now receive an empty session cookie.Note on cookie-based TempData
Cookie-based TempData fundamentally cannot work in streaming SSR —
Set-Cookieheaders cannot be written after the response body has started. The new warning (CookieTempDataNotPersistedAfterResponseStarted) makes this explicit and recommends switching toTempDataProviderType.SessionStorageviaRazorComponentsServiceOptions. Session-backed persistence ([SupplyParameterFromSession]and session-basedTempData) works correctly because it is server-side and the new session-establishment callback ensures the session cookie is issued before streaming begins.Fixes #66745