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

[Blazor] Updated Blazor Server reconnect UI #55723

Merged
merged 10 commits into from
May 28, 2024

Conversation

MackinnonBuck
Copy link
Member

@MackinnonBuck MackinnonBuck commented May 14, 2024

Updated Blazor Server reconnect UI

Improves the default Blazor Server reconnect experience according to common customer feedback.

Description

Makes the following improvements to the Blazor Server reconnect UI:

  • Rather than waiting three seconds before attempting reconnection, then waiting an additional default of 20 seconds between successive attempts, the new default settings use a configurable exponential backoff strategy:
    • Retry as quickly as possible for the first 10 attempts
    • Retry every 5 seconds for the next 10 attempts
    • Retry every 30 seconds until reaching the user-configured max retry count
    • Note: Customers can configure their own retry interval calculation function to override the default behavior
  • When the user navigates back to the disconnected app from another app or browser tab, a reconnect attempt is immediately made
  • If the server can be reached, but reconnection fails because server disposed the circuit, a refresh occurs automatically
  • The default reconnect UI shows the number of seconds until the next reconnect attempt instead of the number of attempts remaining
  • The styling of the default reconnect UI has been modernized

Fixes #55721

Customer Impact

Customers of apps built using Blazor Server often complain about the reconnection experience, which has motivated Blazor developers to open issues like #32113 suggesting improvements to reduce the amount of time the customer spends looking at the reconnect UI. This PR addresses many of those common concerns by performing reconnection attempts more aggressively. Unless apps have overridden the default reconnection options, they will automatically get thew new reconnect behavior by upgrading to .NET 9. In addition, the default reconnect UI styling has been updated. Styling changes will not affect apps that have overridden the default reconnect UI.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

This change only affects the reconnection experience when using Blazor Server or Server interactivity in a Blazor Web App. We have existing automated tests verifying core reconnect functionality. It's possible that some customers may have been relying on the previous defaults, but they'll still be able to override the new defaults if desired.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@MackinnonBuck MackinnonBuck requested a review from a team as a code owner May 14, 2024 23:42
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label May 14, 2024
@MackinnonBuck
Copy link
Member Author

Here are some additional open questions after some internal discussion:

  • Is exponential backoff really necessary?
    • How big of a problem would it be if, by default, we constantly attempted reconnects without any waiting period? For apps with a limited number of users, one would probably prefer the faster reconnection, even if it means the server gets bombarded with reconnection attempts. Larger apps that need better scalability could reconfigure this default behavior (or use Blazor WebAssembly).
  • Rather than showing a full screen overlay, should we only disable parts of the UI that are interactive?
    • Would need to consider the feasibility of implementing this - how do we know what "disabled" should look like for arbitrary UI?
  • Could we wait to show the overlay until the user performs an interaction that requires the circuit?
    • This would allow the user to continue viewing the site if they're not interacting with it
    • We could optionally show a less intrusive reconnection indicator (e.g., an icon in the corner of the page) before the user attempts this type of interaction

Other notes:

  • We think it's best to always refresh the browser when reconnection fails due to the circuit being disposed.
    • The initial argument against this is that this might end up wiping browser state before the user gets a chance to save their work
    • However, that the reconnect overlay prevents the user from being able to save their work anyway.
    • Thought: Absent of a stateful reconnection mechanism, is there a benefit to having the customer refresh manually? That way they're in control of the action that clears page state. It could be frustrating if the website refreshed upon returning to a tab, only to erase all the fields you had populated.
      • Maybe this could be configurable? e.g., we expose a JS callback that lets you decide what to do when reconnection gets rejected by the server. You could have different behavior depending on the page. Perhaps the automatic refresh is on by default only when stateful reconnection is enabled.

@javiercn
Copy link
Member

Rather than showing a full screen overlay, should we only disable parts of the UI that are interactive?
Would need to consider the feasibility of implementing this - how do we know what "disabled" should look like for arbitrary UI?

We could set a custom property so that people can style the elements appropriately with container style queries https://developer.chrome.com/docs/css-ui/style-queries

Support for this is on its way https://caniuse.com/css-container-queries-style and will likely be there when we ship. On our part, setting up a custom property does not tie us into anything and people will be able to leverage this feature once it arrives on stable versions.

@@ -297,6 +307,10 @@ export class CircuitManager implements DotNet.DotNetCallDispatcher {
this.disconnect();
}

private setCircuitStatus(status: 'connecting' | 'connected' | 'disconnected') {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I acknowledge that calls to this are somewhat scattered across the implementation - unfortunately there are a lot of ways in which the circuit can fail to connect, and connection is handled different when connecting for the first time vs reconnecting after a disconnect. Ideally the code would be structured in a way where changes in circuit state were handled in a more centralized manner, that would fairly significantly increase the size of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the current possible values are 'connecting', 'connected', and 'disconnected'. In the period between reconnect attempts, the status is 'disconnected'. Also, the status is 'connecting' when the circuit is performing its initial connection.

I think this behavior accurately reflects the true state of the circuit, but I wonder if it's more useful to make the status '' when connected (or as long as a disconnection has never happened), 'reconnecting' when the circuit is disconnected but the reconnect handler is running, and 'disconnected' after reconnection has failed. Thoughts on this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it's not super clear to me what the expected usage pattern would be for this new custom CSS property. It seems that it would primarily be useful when using container style queries, which not yet supported in all major browsers.

We could alternatively put an attribute on the <html> element like data-circuit-status and then developers could use an attribute selector to customize styles depending on what that attribute's value is.

I'm probably going to scope this out of the PR for now, and we can think about adding an attribute or custom CSS property at a later time.

@MackinnonBuck MackinnonBuck changed the base branch from main to release/9.0-preview5 May 23, 2024 21:48
@mkArtakMSFT mkArtakMSFT added the Servicing-approved Shiproom has approved the issue label May 28, 2024
@mkArtakMSFT mkArtakMSFT merged commit 2dc300d into release/9.0-preview5 May 28, 2024
26 checks passed
@mkArtakMSFT mkArtakMSFT deleted the mbuck/new-reconnect-ui branch May 28, 2024 16:43
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview5 milestone May 28, 2024
@wtgodbe
Copy link
Member

wtgodbe commented May 29, 2024

/backport to main

Copy link
Contributor

Started backporting to main: https://github.com/dotnet/aspnetcore/actions/runs/9289346719

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Blazor] Improve Blazor Server reconnect UI
5 participants