Skip to content

Reconnection will almost never work #11999

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

Closed
SteveSandersonMS opened this issue Jul 9, 2019 · 2 comments
Closed

Reconnection will almost never work #11999

SteveSandersonMS opened this issue Jul 9, 2019 · 2 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Jul 9, 2019

This comes out of #11793 where I'm trying to pin down what will make people actually successful at handling connection instability and migration of state across servers.

We've invested a lot in making server-side Blazor circuits theoretically capable of reconnecting and continuing after a transient connection loss, but with the current implementation, it gives up permanently if the first auto-reconnect attempt fails. And it continues to show the "Retry?" button which will do nothing if you click it.

This is because in Boot.Server.ts, the flow looks like:

  • Disconnection is detected
  • After 3 seconds, a timer calls reconnect, which in turn calls connection.start
  • connection.start will throw (assuming the network connection hasn't been restored already, and remember, we only waited 3 seconds so it almost certainly hasn't been).
  • The exception is caught and passed to unhandledError, which sets a flag renderingFailed = true
  • Now all subsequent calls to reconnect just bail out immediately without doing anything because of the renderingFailed flag.

So the timer which calls reconnect every 3 seconds won't do anything, and the "Retry?" button which also calls reconnect won't do anything.

Minimal fixes

As an absolute minimum, assuming we want to ship reconnection as a feature for 3.0, I think we must do the following. This is not a suggestion of the best UX; it's the smallest set of improvements I think we could ship with.

  • Don't switch into a "permanently failed" mode just because one reconnection attempt failed if the reason was unknown.
    • We'll need to distinguish exceptions due to network errors from exceptions due to components themselves throwing. Not calling unhandledError from initializeConnection, and instead catching upsteam, might be enough for that.
  • Change the "reconnect" UI so that alongside the "Retry" button, it also has a secondary button (or link text) saying "Reload page", which does a full-page reload.

Better fixes

If we wanted better reconnection UX, we should

  • Make the "retry" button show some acknowledgement when you click it
    • For example, disable it for 1 second following the click
    • If the manually-initiated retry fails, then change the dialog text to say "Reconnection failed. Try reloading the page if you're unable to reconnect.". It will still have the two options, "[Retry]" and "[Reload page]".
  • Explicitly detect connection failures due to "server says unknown circuit ID", and in those cases, auto-reload the whole page since we know reconnection isn't realistically going to work.
    • We need to avoid infinite refresh loops, e.g., put a flag in sessionStorage to say we did an auto-reload, clear that flag the next time a connection is successful, and don't do an auto-reload any time the flag is still there.

I recognize that some of this duplicates #10496 and #9256. They are down as 3.1.0 and backlog respectively, but I'm not convinced this is a shippable feature without addressing at least the "minimal fixes" above.

@SteveSandersonMS SteveSandersonMS added the area-blazor Includes: Blazor, Razor Components label Jul 9, 2019
@mkArtakMSFT mkArtakMSFT added this to the 3.0.0-preview9 milestone Jul 9, 2019
@mkArtakMSFT mkArtakMSFT added the bug This issue describes a behavior which is not expected - a bug. label Jul 9, 2019
@mkArtakMSFT
Copy link
Member

Let's tackle the Minimal fixes in Preview 9 and handle the rest post 3.0.

@SteveSandersonMS
Copy link
Member Author

Closing this one as done in #12420, with remaining suggestions for future enhancement now moved to #12442.

@SteveSandersonMS SteveSandersonMS added the Done This issue has been fixed label Jul 22, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests

2 participants