dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.28k stars 9.96k forks source link

APIs for components to persist state across circuits manually #30344

Open javiercn opened 3 years ago

javiercn commented 3 years ago

This is tracking the next piece of https://github.com/dotnet/aspnetcore/issues/27576. (Pointing out that this issue has additional 20 reactions)

We want to provide the ability for circuits to be paused and stored in "permanent" storage to make possible certain scenarios:

Our goal is that circuits stored this way can be restored in a similar fashion as when they are restored after prerendering.

Important note

This is not a suggestion that circuits could automatically be persisted in their entirety. In general that's not possible as much of the system state is not even serializable, and we don't require developers only to work with serializable data. Rather, what we may be able to do here is provide some APIs by which you can load and save state in your components, so that if a user loses their circuit, we can reload that manually-saved state in the new circuit. It could not be a global or automatic thing.

ghost commented 3 years ago

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.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

mkArtakMSFT commented 2 years ago

@javiercn should this be closed in favor of the linked issue ? We now have both on the .NET 7 milestone

nkosi23 commented 2 years ago

@mkArtakMSFT this ticket is different in that it includes scope explicitly excluded from the other ticket. This ticket will enable both seamless scale out and transparent user-friendly failovers on application update deployments. These are 2 huge features that will finally enable Blazor Server to be used for mid and high-traffic consumer-facing applications.

Thank to this ticket, many people will use Azure for their consumer applications considering the built-in SignalR service and the auto-scaling capabilities. Considering the huge productivity boost provided by Blazor Server I believe this ticket will position Blazor Server and Azure as the leading Web Development stack. With this regard, shipping this in .NET 7 would make the timing perfect.

Generally speaking, this ticket will essentially allow users to treat the servers hosting Blazor Server Apps like stateless servers even though Blazor Server itself would remain stateful. This enables the two aforementioned use cases (which are major ones) and will also simplify a lot of other essential things from an infrastructure management / devops perspective.

This ticket is awaited by a lot of people as can be seen in the popularity of this reddit thread.

javiercn commented 2 years ago

https://github.com/dotnet/aspnetcore/issues/41791

fdahlen commented 2 years ago

@javiercn What happened to this being part of .NET 7? If possible, could you provide some details?

Me and several others are waiting for this to happen. Blazor Server Side simply does not seem production ready for a Kubernetes environment without this kind of feature.

javiercn commented 2 years ago

@fdahlen thanks for the interest in the feature.

As part of .NET 7.0 the team has been focused on delivering Blazor support for .NET Maui and now that we have completed that we are looking at the set of things we can deliver for .NET 7.0.

Unfortunately, as much as we want to deliver this feature, we determined that it would be risky to do so at this point of the release cycle and that we would not have enough time to react to feedback meaningfully on a feature this size, so we've preferred to be conservative and push this out to .NET 8.0.

We might start work in this area at the end of .NET 7.0 or the beginning of .NET 8.0 and we might make some changes in .NET 7.0 to enable early experimentation via an experimental package, similar to how we have done for other features.

That said, at this point we will reassess this feature as we look at the planning for .NET 8.0.

I hope this helps bring some clarity into our thinking.

3hxx commented 2 years ago

This has been an ongoing issue now for a while it seems, someone before has literally come up with a solution to fix it, please can you reconsider this as a priority otherwise the mobile experience for websites that use server side are significantly worsened.

Mike-E-angelo commented 2 years ago

Can someone be as so kind as to confirm that this issue will address server restarts in the case of deployments? Right now when I deploy to Azure AppService, the reconnecting message displays to the user, and the session is lost. The user must refresh their browser (or click the link prompting them to do so).

I am currently in alpha, so there are expectations as such, and I have a little wiggle room at the moment for this. However, as time goes on, wiping the entire set of established connections with each and every deployment will be a bit of a bummer. I am already ancy with the prospect with each and every deployment push and I have close to zero traffic at the moment. I might provide a system message/toast that is displayed to the user when a deployment is about to occur.

It is my understanding that this issue will address this disruption. Is that correct? I am also wondering if I missed something obvious in my research about this issue and if there is potential mitigation. I do see there is Azure SignalR Service, but I do not think that addresses this problem. As I understand it, Azure SignalR Service simply provides a way of managing connections and sending them to the hosting server in a scalable way. If that hosting server is refreshed due to a deployment, the state is still lost and the message is still shown to the user.

Any clarification here would be appreciated.

Mike-E-angelo commented 2 years ago

Can someone be as so kind as to confirm that this issue will address server restarts in the case of deployments?

Hi @javiercn I volunteer you. :D Please let me know if this issue captures/tracks this rather concerning disruption or if a new issue is needed. Thank you for any clarification and assistance you can provide.

Mike-E-angelo commented 2 years ago

Adding a little more grief here from a reported incident with a customer: https://www.reddit.com/r/Blazor/comments/x78buq/blazor_serverside_safarimacipad_connection_issue/

One last ping to @javiercn to see if this issue addresses the disruption caused by deployments or if a new issue is needed. To reiterate: upon deployment to Azure AppService, every connection to the AppService is lost and the user is presented a reconnect message, losing the state of any sessions that are open.

If I do not hear anything back, I will create a new issue and we can take it from there. Thank you again for any clarification you can provide.

Mike-E-angelo commented 2 years ago

I got such a great answer from @javiercn on my question that it is worth an extra mention/link here: https://github.com/dotnet/aspnetcore/issues/43873#issuecomment-1241909707

Mike-E-angelo commented 1 year ago

FWIW I am looking for any guidance on how to maximize uptime for my Blazor server-side application on Azure here: https://learn.microsoft.com/en-us/answers/questions/1036028/azure-auto-heal-multiple-instances-for-blazor-serv.html

(I've made mention of this issue here in the question there)

Any guidance/assistance/context would be appreciated if anyone has anything they would like to add.

MackinnonBuck commented 1 year ago

There seem to be several open issues relating to this topic, so I'm going to link them here. Many of these can probably be closed when this feature gets completed:

TanayParikh commented 1 year ago

Many of these can probably be closed when this feature gets completed

Should we just close out the other issues now if they'd be resolved by this issue? The goal being to minimize dup's and ensure community feedback is centralized.

MackinnonBuck commented 1 year ago

I'm not sure if I would say these issues are strictly dupes. For example, we could separately solve the automatic refresh problem without doing everything this issue describes. But depending on the scope of our solution to this issue, some of these other issues might become irrelevant or fixed as part of this one. Maybe this is still enough to warrant closing them - what do you think? We could also discuss this at the next engineering sync before taking any major actions here.

konradbartecki commented 1 year ago

Please also check the new comment on https://github.com/dotnet/aspnetcore/pull/44700#issuecomment-1291378451 as I added it after it was closed.

Alternatively even you won't want to merge #44700 I can help you test a currently untested scenario of "circuit rejected/evicted because it was disconnected and 3 minutes of inactivity passed" since I already did some research on it.

Mike-E-angelo commented 1 year ago

Oh wow, we had a possible PR for this issue to be somewhat addressed in .NET7 and it was closed? Am I understanding that correctly?

Webreaper commented 1 year ago

Oh wow, we had a possible PR for this issue to be somewhat addressed in .NET7 and it was closed? Am I understanding that correctly?

If you read the PR comments, it makes sense - as MSFT are planning to address this at a lower and more fundamental level, in .Net 8.

Mike-E-angelo commented 1 year ago

I did see that @Webreaper and while that is the understanding and expectation thus far, it is still quite a ways away (a year or so away?)... if something can be done in the meantime that would very much be preferred (would be my vote).

TanayParikh commented 1 year ago

Please note, .NET 7 is already locked down at this point. We aren't merging any new features into 7.0 as we prepare the GA bits.

TimPosey2 commented 1 year ago

@TanayParikh : Fully understand that .NET 7 is already RTM. However, this feature is so important, can it be an out-of-band release (e.g., v7.1) instead of having to wait another full year?

konradbartecki commented 1 year ago

Technically you could actually have a workaround of reloading the page automatically when we detect that circuit was evicted aka disposed aka rejected. That should be applicable to dotnet version from 5 at least to 7+

You can implement your own ReconnectionHandler, see: src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectionHandler.ts

Although I do strongly believe that the current approach of requiring a user to press reload button after he moved away from the page on a mobile phone for more than 3 minutes is a really bad user experience and got a lot of people confused, as a workaround they could implement their own reconnection handler.

Now no one will be aware of that if they won't deep dive into the Blazor's source code and there are no docs available for that issue either. Blazor is simply unusable on mobile browsers.

My idea was to reload the browser's page on connection rejected, since I do not see any other reason why it should not be done this way.

Alternatively if we consider that as breaking backward compatibility too much, then we could add an opt-in switch to reload the browser on circuit rejection:

<body>
    ...

    <script src="_framework/blazor.server.js" autostart="false"></script>
    <script>
      Blazor.start({
        reconnectionOptions: {
          maxRetries: 3,
          retryIntervalMilliseconds: 2000,
          reloadOnCircuitRejected: true // opt-in
        }
      });
    </script>
</body>

So like described here: https://learn.microsoft.com/en-us/aspnet/core/blazor/fundamentals/signalr?view=aspnetcore-6.0#modify-the-reconnection-handler-blazor-server

You could inject an implementation of:

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

import { ReconnectionHandler, ReconnectionOptions } from './CircuitStartOptions';
import { ReconnectDisplay } from './ReconnectDisplay';
import { DefaultReconnectDisplay } from './DefaultReconnectDisplay';
import { UserSpecifiedDisplay } from './UserSpecifiedDisplay';
import { Logger, LogLevel } from '../Logging/Logger';
import { Blazor } from '../../GlobalExports';

export class DefaultReconnectionHandler implements ReconnectionHandler {
  private readonly _logger: Logger;

  private readonly _reconnectCallback: () => Promise<boolean>;

  private _currentReconnectionProcess: ReconnectionProcess | null = null;

  private _reconnectionDisplay?: ReconnectDisplay;

  constructor(logger: Logger, overrideDisplay?: ReconnectDisplay, reconnectCallback?: () => Promise<boolean>) {
    this._logger = logger;
    this._reconnectionDisplay = overrideDisplay;
    this._reconnectCallback = reconnectCallback || Blazor.reconnect!;
  }

  onConnectionDown(options: ReconnectionOptions, _error?: Error): void {
    if (!this._reconnectionDisplay) {
      const modal = document.getElementById(options.dialogId);
      this._reconnectionDisplay = modal
        ? new UserSpecifiedDisplay(modal, options.maxRetries, document)
        : new DefaultReconnectDisplay(options.dialogId, options.maxRetries, document, this._logger);
    }

    if (!this._currentReconnectionProcess) {
      this._currentReconnectionProcess = new ReconnectionProcess(options, this._logger, this._reconnectCallback, this._reconnectionDisplay);
    }
  }

  onConnectionUp(): void {
    if (this._currentReconnectionProcess) {
      this._currentReconnectionProcess.dispose();
      this._currentReconnectionProcess = null;
    }
  }
};

class ReconnectionProcess {
  readonly reconnectDisplay: ReconnectDisplay;

  isDisposed = false;

  constructor(options: ReconnectionOptions, private logger: Logger, private reconnectCallback: () => Promise<boolean>, display: ReconnectDisplay) {
    this.reconnectDisplay = display;
    this.reconnectDisplay.show();
    this.attemptPeriodicReconnection(options);
  }

  public dispose() {
    this.isDisposed = true;
    this.reconnectDisplay.hide();
  }

  async attemptPeriodicReconnection(options: ReconnectionOptions) {
    for (let i = 0; i < options.maxRetries; i++) {
      this.reconnectDisplay.update(i + 1);
      if (this.isDisposed) {
        break;
      }

      try {
        // reconnectCallback will asynchronously return:
        // - true to mean success
        // - false to mean we reached the server, but it rejected the connection (e.g., unknown circuit ID)
        // - exception to mean we didn't reach the server (this can be sync or async)
        const result = await this.reconnectCallback();
        if (!result) {
          // If the server responded and refused to reconnect, reload the page
          location.reload();
          return;
        }
        return;
      } catch (err: unknown) {
        // We got an exception so will try again momentarily
        this.logger.log(LogLevel.Error, err as Error);
      }
      await this.delay(options.retryIntervalMilliseconds);
    }

    this.reconnectDisplay.failed();
  }

  delay(durationMilliseconds: number): Promise<void> {
    return new Promise(resolve => setTimeout(resolve, durationMilliseconds));
  }
}
vindberg commented 1 year ago

Thanks for giving this workaround. I have implemented something similar for watchpedia.com -> Instead of automatic reload I'm tracking all links (_reconnectionDisplay -> show) and doing a full page load when the user clicks something. This way the page is only reacting when there is user interaction.

For any javascript events (method calls) I currently have to refresh the page with is not optimal. I'm working on triggering the call automatically after the refresh.

Mike-E-angelo commented 1 year ago

Wow, according to this post, it appears .NET 8 Preview 1 is right around the scheduling corner: https://github.com/dotnet/announcements/issues/241

We expect the repository to be persistent by the release of .NET 8 Preview 1, planned for February, 2023.

Mike-E-angelo commented 1 year ago

Hello, happy new year. :)

I wanted to check in on this issue as it appears to not have any activity since last November. I am currently averaging about $10/day in sales volume w/ my Blazor server-side application -- having passed $500 all-time volume yesterday 🎉 -- and am starting to have more customers complain about losing their connections when switching away from the application during form entry. I am also having to be very careful when doing deployments to not disrupt any active connections. It's becoming more and more of a challenge as I get more activity.

Thank you for any update you can provide.

TimPosey2 commented 1 year ago

Mike, check out: https://youtu.be/48G_CEGXZZM

On Wed, Jan 25, 2023, 4:17 AM Mike-E @.***> wrote:

Hello, happy new year. :)

I wanted to check in on this issue as it appears to not have any activity since last November. I am currently averaging about $10/day in sales volume w/ my Blazor server-side application -- having passed $500 all-time volume yesterday https://alpha.starbeam.one/dashboard 🎉 -- and am starting to have more customers complain about losing their connections when switching away from the application during form entry. I am also having to be very careful when doing deployments to not disrupt any active connections. It's becoming more and more of a challenge as I get more activity.

Thank you for any update you can provide.

— Reply to this email directly, view it on GitHub https://github.com/dotnet/aspnetcore/issues/30344#issuecomment-1403381871, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGVSYB2BFJB32Y7UA3GTZDWUD4TPANCNFSM4X7BMGBQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Mike-E-angelo commented 1 year ago

Thank you for that, @TimPosey2 it is appreciated and looks interesting. Is the implication/suggestion here that WebAssembly would be used in place of circuits for state? I hate the idea of streaming my compiled over to clients which is why I use server-side (if I have this misunderstood I would appreciate a correction).

It is unclear how this would address the circuit problem we're running into with this issue, so any further context would be appreciated. 👍

TimPosey2 commented 1 year ago

It appears that you'll be able to more explicitly select which mode you prefer. Based on the demo, it appears you could have specific pages that are Server-side (with the WebSocket circuit) and as soon as you navigate away from that page, then it closes out the circuit, which should preserve resources. It also shows that the first load would be Server-side until the client has cached the WASM files necessary to have it run client-side on the next load. The user would get a super-fast page load (and presumably MSFT will find a way to transition it seamlessly to client-side once the WASM files are loaded). If you're uncomfortable with having your entire client on the browser for intellectual property reasons, just look at doing API calls to a backend for proprietary formulas/calculations.

While it doesn't fully address circuit resumption, it's a massive step in the right direction I believe.

On Wed, Jan 25, 2023 at 11:02 AM Mike-E @.***> wrote:

Thank you for that, @TimPosey2 https://github.com/TimPosey2 it is appreciated and looks interesting. Is the implication/suggestion here that WebAssembly would be used in place of circuits for state? I hate the idea of streaming my compiled over to clients which is why I use server-side.

It is unclear how this would address the circuit problem we're running into with this issue, so any further context would be appreciated. 👍

— Reply to this email directly, view it on GitHub https://github.com/dotnet/aspnetcore/issues/30344#issuecomment-1403937517, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGVSYHABSNI4J54J6J27HTWUFMCDANCNFSM4X7BMGBQ . You are receiving this because you were mentioned.Message ID: @.***>

Mike-E-angelo commented 1 year ago

While it doesn't fully address circuit resumption, it's a massive step in the right direction I believe.

I like it. I like the idea of starting with fast server render and then taking over w/ WASM once it's ready, and then resuming via WASM on refresh. I also love the idea of only using WebSockets when necessary. To me, that seems like form entry pages which is causing the grief, but I have an ominous feeling that once I start picking at the details it may be more than that.

In my case, however, I am not sure what that will entail as I am not using API/REST calls, and everything is piped straight through an EFCore model and works amazingly well. If there is a way to use WASM w/o pulling over my model/code I am all for this, otherwise I get the feeling I am in for a rude awakening here. 😬

mycarrysun commented 1 year ago

If you're uncomfortable with having your entire client on the browser for intellectual property reasons, just look at doing API calls to a backend for proprietary formulas/calculations.

This defeats the point of blazor and is the entire reason why this issue is still open. They said it would be fixed in 8.1 so we shall wait and see.

tedmorris commented 1 year ago

This ticket implementation is a MUST HAVE for Server-Side Blazor. Please implement. Circuit reconnection is required to use Server-Side Blazor in any meaningful and resilient Production environment. We are patiently waiting for this feature in v8 before we move ahead with any Production release of apps in Server-Side Blazor. This would also make Server-Side Blazor a complete solution choice for many application scenarios instead of using WebAssembly.

manigandham commented 1 year ago

The lack of serializing and resuming a circuit has greatly reduced our usage of Blazor server-side.

The UX is very poor when users return after being inactive, or when deploying application updates.

3hxx commented 1 year ago

@SteveSandersonMS is there no work around for this currently? It's baked to deep?

CodeFontana commented 1 year ago

I'm amid re-writing a Blazor Server project, to be a WASM + API project, in order to avoid this problem. My users tend to leave their browser tab open during the workday, only to return and find it stuck 'reconnecting' or timed out. It's adding to our helpdesk volume despite being an easy refresh to fix.

I would happily trade my Blazor Server process increasing it's memory usage to be able to pause and resume circuits, as long as the ceiling was configurable. Ideally, I would set the max timeout to be greater than my JWT expiry. But anything to stop users from the impression that the app is down!

Webreaper commented 1 year ago

+1 here - I rebuilt my app to convert from Blazor Server to Blazor WASM precisely because of the issues around the page dying when I push a release.

Would be good if this could be solved though, as it would make Blazor server a practical implementation choice for business apps.

Webreaper commented 1 year ago

Mike, check out: https://youtu.be/48G_CEGXZZM

This looks totally awesome, and is exactly what we need. GO GO GO @SteveSandersonMS 😁👍

manigandham commented 1 year ago

Mike, check out: https://youtu.be/48G_CEGXZZM

This looks totally awesome, and is exactly what we need. GO GO GO @SteveSandersonMS 😁👍

Blazor United is very cool but doesn't solve the same problem. Being able to transition from Server to WASM is great but it means that backend logic can no longer be as tightly integrated as in purely Server mode (like accessing a database for example).

We don't want to change APIs or add a new layer, we just want a stable way to suspend and restore component state across servers and sessions.

3hxx commented 1 year ago

If you're using Blazor Server and the circuit is broken due to the user tabbing out, Make that dialog sound like the users session has expired and that they need to refresh their page in order to continue. Users can be convinced this is a security feature of your site rather than a issue with the framework.

Something like "Session Expired. Refresh to continue".

Mike-E-angelo commented 1 year ago

Hi @javiercn are you able to provide an update on this issue? Thank you for any insight you can provide.

javiercn commented 1 year ago

@Mike-E-angelo work is happening in this area, but we are trying to rationalize how this interacts with some of the other work we are doing as we will likely be able to reuse a lot of work in that space for this feature.

Mike-E-angelo commented 1 year ago

Thank you for taking the time to update us on the status @javiercn. I know you and everyone there is busy and there are a lot of moving pieces, so your time is appreciated.

I am very much hoping you and the team there are able to wrangle the pieces together and find success in facing all the clear and obvious challenges. 🙏👍

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

Mike-E-angelo commented 1 year ago

Hi @javiercn & @mkArtakMSFT can we get a comment on what happened here with the status, please?

3hxx commented 1 year ago

I would also like to know. Cheers

danroth27 commented 1 year ago

Hi folks. For .NET 8 we'll be focusing only on the aspects of this that relate to server-side rendering and the Blazor United efforts. Unfortunately, the other features mentioned here like migration across servers won't make it into .NET 8. Basically, we've decided we need to prioritize the Blazor United effort higher than these items for .NET 8.

Webreaper commented 1 year ago

@danroth27 Is it fair to say that this prioritisation decision is being made because it's deemed that Blazor United will solve many of the problems raised in this issue? i.e., if a hybrid model of Server/Wasm is available, then it'll be easier to structure applications so that when circuits are dropped the impact will be far less?

SteveSandersonMS commented 1 year ago

@Webreaper Yes exactly

danroth27 commented 1 year ago

@danroth27 Is it fair to say that this prioritisation decision is being made because it's deemed that Blazor United will solve many of the problems raised in this issue? i.e., if a hybrid model of Server/Wasm is available, then it'll be easier to structure applications so that when circuits are dropped the impact will be far less?

Yes, that's correct. At the same time, we understand that there are many existing Blazor Server apps that would still benefit from pause & resume capabilities, so this is still a painful cut (especially given that we've had to push it out of previous releases as well 😞). But we think the Blazor United will have broader impact, and we want to make sure that effort is successful.

Mike-E-angelo commented 1 year ago

@danroth27 and @SteveSandersonMS thank you for your comments and update. I want to make sure I understand the WASM story w/ Blazor United before making a full comment here. It is my understanding that WASM works by downloading my codebase to the client where it can be decompiled. Do I have this understood correctly? If so, I am not sure that is a viable replacement/direction for what we have currently using Blazor Server-side which keeps all the code safe and sound on the server. If I have something misunderstood here I would certainly appreciate it.