Skip to content
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

Calling InvokeAsync(StateHasChanged) causes page to fallback to default culture #28521

Closed
DaveNay opened this issue Dec 8, 2020 · 10 comments
Closed
Labels
affected-medium This issue impacts approximately half of our customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-localization ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. question severity-major This label is used by an internal tool Status: Resolved

Comments

@DaveNay
Copy link

DaveNay commented Dec 8, 2020

Describe the bug

In a Blazor Server application, I am updating page data from a background service. This update from the service ultimately ends up calling InvokeAsync(StateHasChanged) in the page. The page is also using IStringLocalizer<T> to provide translations for table headers. When the application runs, the user can select the current culture (e.g. German (de-DE)), and the table is displayed with translated headers. As soon as the background updates begin, the German culture is ignored, and the page is rendered using English (en-US) resources.

My expectation is that calling StateHasChanged will use the proper resources for rendering the page.

Localization_Debugging

To Reproduce

I have created a small application that reproduces this scenario:

https://github.com/DaveNay/Service_Localization_Debugging

In the sample application, you can switch to the FetchData page, and then change the culture to German. You will see that the page is rendered with the de-DE resources. 10 seconds after the application starts, the background updates will begin. As soon as the updates begin, the page is rendered using en-US resources.

Exceptions (if any)

None

Further technical details

  • .Net 5.0 (also .Net Core 3.1)
  • Visual Studio Pro 16.8.2
  • Windows 10 Enterprise 1809
  • dotnet_info.txt
@javiercn javiercn added the area-blazor Includes: Blazor, Razor Components label Dec 9, 2020
@mkArtakMSFT mkArtakMSFT added this to the Next sprint planning milestone Dec 9, 2020
@ghost
Copy link

ghost commented Dec 9, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@SteveSandersonMS SteveSandersonMS added affected-medium This issue impacts approximately half of our customers enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-major This label is used by an internal tool labels Jan 26, 2021 — with ASP.NET Core Issue Ranking
@mkArtakMSFT mkArtakMSFT added bug This issue describes a behavior which is not expected - a bug. and removed enhancement This issue represents an ask for new feature or an enhancement to an existing one investigate labels Jul 20, 2021
@beppemarazzi
Copy link

Found the same issue. Waiting for an official solution, i solved using this workaround:

using System;
using System.Globalization;
using System.Threading.Tasks;

namespace Microsoft.AspNetCore.Components
{
    //https://github.com/dotnet/aspnetcore/issues/28521
    public abstract class ComponentBaseWithCulture : ComponentBase
    {
        private CultureInfo? m_componentUICulture;
        protected override void OnInitialized()
        {
            m_componentUICulture = CultureInfo.CurrentUICulture;
            base.OnInitialized();
        }

        protected Task InvokeAsyncWithCulture(Action workItem)
        {
            return InvokeAsync(() => DispatchWithCulture(workItem));
        }
        private void DispatchWithCulture(Action workItem)
        {
            var prevUICulture = CultureInfo.CurrentUICulture;
            if (m_componentUICulture != null)
                CultureInfo.CurrentUICulture = m_componentUICulture;
            try
            {
                workItem();
            }
            finally
            {
                CultureInfo.CurrentUICulture = prevUICulture;
            }
        }
        protected Task InvokeAsyncWithCulture(Func<Task> workItem)
        {
            return InvokeAsync(() => DispatchWithCulture(workItem));
        }
        private Task DispatchWithCulture(Func<Task> workItem)
        {
            var prevUICulture = CultureInfo.CurrentUICulture;
            if (m_componentUICulture != null)
                CultureInfo.CurrentUICulture = m_componentUICulture;
            try
            {
                return workItem();
            }
            finally
            {
                CultureInfo.CurrentUICulture = prevUICulture;
            }
        }

    }
}

@TanayParikh TanayParikh added the Priority:1 Work that is critical for the release, but we could probably ship without label Oct 22, 2021
@mkArtakMSFT mkArtakMSFT added enhancement This issue represents an ask for new feature or an enhancement to an existing one triaged and removed bug This issue describes a behavior which is not expected - a bug. labels Nov 2, 2021
@ghost
Copy link

ghost commented Feb 16, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@javiercn
Copy link
Member

This is not a bug, but an issue with "from where" the code is calling InvokeAsync.

The problem is that the code triggers a global event from outside the scope of the circuit and a registered handler calls InvokeAsync to update the state.

The problem is that the call originates from a thread that doesn't have the correct ExecutionContext because it is external to the circuit. InvokeAsync ensures that code that updates the UI runs single threaded and captures the ExecutionContext when it is invoked as part of responding to a UI event or rendering a component.

However, when done in the fashion described here, there is no way for InvokeAsync to have access to the ExecutionContext normally associated with the circuit (because the source of the event that calls InvokeAsync is a separate thread).

The issue that we are observing here is because the culture on the External thread that originates the call (and that comes ultimately from the ExecutionContext of that thread) is not the one associated with the circuit and for that reason, the culture changes when the event is triggered.

This can be corrected by manually capturing the context at the time the event is being registered and restoring it manually when the event is raised, like this:

        var context = ExecutionContext.Capture();
        Cache.ForecastHasChanged += (forecasts) =>
        {
            var current = ExecutionContext.Capture();
            try
            {
                ExecutionContext.Restore(context);
                OnForecastChanged(forecasts);
            }
            finally
            {
                ExecutionContext.Restore(current);
            }
        };

At this point, when we register the event, we capture the ExecutionContext so that when the event is triggered (no matter where from) we can restore the original ExecutionContext, run the callback and restore the caller ExecutionContext afterwards.

It is key to understand that the issue here is with the callstack by the time InvokeAsync is called.

When that callstack originates from handling an event or rendering a component in the context of a circuit everything works as expected and automatically because the call to InvokeAsync happens within the circuit, where the ExecutionContext is correct and flows as part of the async chain.

When the callstack originates from outside of the circuit, the execution context is not going to be correct and InvokeAsync doesn't have a way determine the execution context since the logical chain broke.

That's why using the snippet above bridges the two pieces. During registration the correct context is captured. During execution the context is restored, the callback runs and then the caller restores their context back.

@javiercn javiercn added question ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. labels Apr 28, 2022
@ghost ghost added the Status: Resolved label Apr 28, 2022
@javiercn javiercn removed this from the .NET 7 Planning milestone Apr 28, 2022
@javiercn javiercn removed the Priority:1 Work that is critical for the release, but we could probably ship without label Apr 28, 2022
@beppemarazzi
Copy link

@javiercn
In my project i have a lot of background services waiting for some "state change" in the external world: when this happens, i want to push UI update to all clients that are watching to that resource... I think it's a pretty common pattern where blazor server rendering helps a lot!!!!
Googling for invokeasync statehaschanged leads to many articles explaining the same use of the pattern InvokeAsync(() => StateHasChanged();)
Also official docs explains the same here: technically speaking this example is flawed when using the Localization middleware like officially documented here!!!!

Please consider to implement in the framework provided base class (i.e. ComponentBase) something like my workaround here

@javiercn
Copy link
Member

In my project i have a lot of background services waiting for some "state change" in the external world: when this happens, i want to push UI update to all clients that are watching to that resource... I think it's a pretty common pattern where blazor server rendering helps a lot!!!!

This is not a common pattern that we see often, however despite that, it's your callback registration code the one that should be responsible for capturing the context at the time of registration and restoring it at the time of execution.

As I mentioned, the problem here is that the callback invocation originates from outside Blazor, at that point, we don't have access to the context you were using when you registered the callback in your event, which is what you are looking for.

The code snippet that I provided does just that. The important bit is that it happens at the time the callback is registered, not when InvokeAsync is called.

Also official docs explains the same here: technically speaking this example is flawed when using the Localization middleware like officially documented here!!!!

The samples sometimes might contain errors/imprecisions, especially when it comes to obscure details like these, but I don't think this is the case since the invocations happen within the context of the components, and again the important bit is whether the code that is calling InvokeAsync has correctly captured the context, which is not something we can control.

The code in DispatchWithCulture won't work in this case for the same reason I mentioned, the culture comes from an AsyncLocal and the thread from where it is being invoked is not originated from within the circuit for that user, so it doesn't have access to the context.

I understand that these details are very subtle, but I hope my comments at least helped clarify what's going on here and why the framework can't do anything about it and its the responsibility of the calling code to capture the context for later use.

@beppemarazzi
Copy link

beppemarazzi commented Apr 29, 2022

@javiercn Sorry for my insistence...

My code works well. In fact here i captured the culture inside OnInitialized when i'm on the circuit's thread with the user selected language on the stack

protected override void OnInitialized()
        {
            m_componentUICulture = CultureInfo.CurrentUICulture;
            base.OnInitialized();
        }

Then i temporally restore it on the renderer dispatcher when i need to call StateHasChanged

Please reconsider carefully this point: we need some framework provided utility to call StateHasChanged (or some other component internal state related activities) from background workers... and in this case we expect to do it in the same context of the user's circuit. UICulture included!!!

Read again at the doc and please put attention at the penultimate sentence (you have to scroll down after the razor snippet):

In the preceding example:

NotifierService invokes the component's OnNotify method outside of Blazor's synchronization context. InvokeAsync is used to switch to the correct context and queue a render. For more information, see ASP.NET Core Razor component rendering.

What is the supposed use case for switching to the correct context without restoring the circuit's culture???? IMHO when component's InvokeAsync is used you have also to restore "component's" culture (like in my workaround)

@javiercn
Copy link
Member

javiercn commented Apr 29, 2022

@beppemarazzi I missed that you captured it within OnInitialized.

That sample works because the Timer implementation does the right thing and captures the execution context when the timer is created. Your implementation should do the same.

If you define your own event that you trigger from a background thread is your responsibility to capture the execution context at the time the handler is registered.

Blazor can't/won't capture the right context because it is not involved when you register the callback. Specifically, there is no framework code running when this happens.

    Cache.ForecastHasChanged += (forecasts) =>

When you raise your event from a background thread, InvokeAsync doesn't have access to the execution context that you are looking for, because it is already "gone", the execution context at that point is the one from the thread running your background worker raising the event.

InvokeAsync captures and restores the context when you call it and it is not able to run the callback synchronously, but it can't do nothing if the original context at the time you are calling it is wrong, which is your case.

What is the supposed use case for switching to the correct context without restoring the circuit's culture???? IMHO when component's InvokeAsync is used you have also to restore "component's" culture (like in my workaround)

I also want to be very clear that this is not about the circuit culture, there are many ambient values that rely on AsyncLocals and that are backed by the ExecutionContext. It is the responsibility of your background worker to capture the ExecutionContext and restore it if it is going to be invoking the callback from a different context in the same way other framework implementations do so, like Timer.

InvokeAsync only job is to schedule the callback on the circuit synchronization context (that's what switch to the correct context in the docs means) and ensure that multiple components are not rendering in parallel.

Again, to summarize, it is not the framework responsibility to capture the ExecutionContext (because basically it can't), which is what makes the culture work. It is the responsibility of any component that wants to preserve any AsyncLocal value (like the one that backs the current culture) to capture the execution context and restore it before invoking a callback at a later point in time from a different context.

To summarize my comment:

  • We aren't going to provide any functionality to capture and restore the current culture or any other ambient value (backed by an async local) because it is not the framework responsibility to do so.
  • We provided an alternative way to achieve this scenario that works according to the established practices in .NET and we don't think Blazor needs to deviate from the recommended approach to do this.
  • You can use InvokeAsync to schedule a callback to run inside the circuit from a different callsite, however if you need any framework provided ambient value to be preserved, you are responsible from capturing the ExecutionContext and restoring it right before calling InvokeAsync.
  • This is the stablished pattern to accomplish this on .NET framework and we don't plan to deviate from it. For more details you can check other existing implementations like System.Threading.Timer that follow this pattern.

I understand that this might not match your expectations, however I tried to explain to the best of my ability how this works and where each responsibility lies and why.

@beppemarazzi
Copy link

beppemarazzi commented Apr 29, 2022

@javiercn: Your explanation was very clear an i understood it.

Probably we agree that any kind of "global state" smells, but this is how AddLocalization() paired with UseRequestLocalization() works (ultimately the ResourceManagerStringLocalizer uses the global CurrentCulture and CurrentUICulture stored into circuit's execution context by the Localization Middleware).

Given this, i've to disagree with you when you say:

We aren't going to provide any functionality to capture and restore the current culture or any other ambient value (backed by an async local) because it is not the framework responsibility to do so.

In fact IMHO there is a very particular case in the server rendered blazor's app model: we have one single server where we can centralize for all users some low level "active" logic (i.e. raising some events/notifications when something happens), mixed with "per user" render pipeline (i.e. components in the circuit). This is very powerful and you can write "live" apps where the data are "pushed" from server to clients very quickly.

So i think that some sort of framework provided utility capturing at the component's initialization the circuit's Culture informations (or if you prefer all the execution context), and restores it back when component's state related actions are dispatched to renderer sync context may be very useful (IMHO it may be the default behavior). A such framework utility could minimize the technical complex bolierplate "bridging" code in the app's codebase...

That said, i'm already happy with my solution (the ComponentBaseWithCulture used as base class for my blazor pages).... probably other users are not aware of this design and may fall in this pitfall!

Many thanks for your patience: i will not push further for this 😄

Edit: i'm not lone with the expectation of some framework defined utility to manage this:
#36259 (comment)

@ghost ghost locked as resolved and limited conversation to collaborators May 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-medium This issue impacts approximately half of our customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-localization ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. question severity-major This label is used by an internal tool Status: Resolved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants