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

Configurable Reconnection Behavior #38305

Closed
myrup opened this issue Nov 11, 2021 · 20 comments
Closed

Configurable Reconnection Behavior #38305

myrup opened this issue Nov 11, 2021 · 20 comments
Labels
area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one ✔️ Resolution: Duplicate Resolved as a duplicate of another issue Status: Resolved
Milestone

Comments

@myrup
Copy link

myrup commented Nov 11, 2021

Describe the bug

After upgrading from asp.net core 3.1 to 6.0 Blazor is still working, but the browser can no longer locate Blazor.defaultReconnectionHandler._reconnectCallback .

We use this to implement browser refresh on reconnections.

Exceptions (if any)

TypeError: undefined is not an object (evaluating 'Blazor.defaultReconnectionHandler._reconnectCallback)

Further technical details

  • ASP.NET Core version: 6.0
dotnet --info Output
.NET SDK (reflecting any global.json):
 Version:   6.0.100
 Commit:    9e8b04bbff

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  12.0
 OS Platform: Darwin
 RID:         osx-arm64
 Base Path:   /usr/local/share/dotnet/sdk/6.0.100/

Host (useful for support):
  Version: 6.0.0
  Commit:  4822e3c3aa

.NET SDKs installed:
  6.0.100 [/usr/local/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download
@TanayParikh TanayParikh added the area-blazor Includes: Blazor, Razor Components label Nov 11, 2021
@TanayParikh
Copy link
Contributor

Hello @myrup, thanks for contacting us.

Blazor.defaultReconnectionHandler._reconnectCallback is an undocumented internal API, and we do not recommend using it directly within your application code as its subject to change without notice.

In this specific case, the reconnection logic was refactored as a part of https://github.com/dotnet/aspnetcore/pull/36126/files.

@TanayParikh TanayParikh added the ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. label Nov 11, 2021
@ghost ghost added the Status: Resolved label Nov 11, 2021
@myrup
Copy link
Author

myrup commented Nov 11, 2021

@TanayParikh What's the recommended way of making the browser reload on a reconnect then? When googling the problem the following pattern seems fairly reoccurring (now broken):

Blazor.defaultReconnectionHandler._reconnectCallback = function (d) {
    document.location.reload();
}

We need something like this while waiting for #27576

@TanayParikh
Copy link
Contributor

TanayParikh commented Nov 11, 2021

When googling the problem the following pattern seems fairly reoccurring (now broken):

Thanks for pointing that out. I see the following resource that recommend that approach:

I don't believe we have an official way to automatically reload the page on reconnect (someone can correct me if I'm wrong). If you absolutely need a workaround, you could try listening for when the #components-reconnect-modal display changes from none to block. Note this is entirely unsupported / undocumented, and may or may not work, or may stop working abruptly.

As you mentioned #27576 would optimally resolve this.

@TanayParikh TanayParikh removed ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. Status: Resolved labels Nov 11, 2021
@TanayParikh TanayParikh added this to the Backlog milestone Nov 11, 2021
@ghost
Copy link

ghost commented Nov 11, 2021

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.

@TanayParikh TanayParikh removed this from the Backlog milestone Nov 11, 2021
@TanayParikh TanayParikh added the ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. label Nov 11, 2021
@ghost ghost added the Status: Resolved label Nov 11, 2021
@TanayParikh
Copy link
Contributor

Wouldn't https://docs.microsoft.com/aspnet/core/blazor/fundamentals/signalr?view=aspnetcore-6.0#modify-the-reconnection-handler-blazor-server let you do this?

Yep you're right, I believe that'd work as well. Nice catch!

@myrup
Copy link
Author

myrup commented Nov 12, 2021

@BrennanConroy Yeah, I also considered using that, but I gave up on it because I find the documentation lacking on how to create a reconnectionHandler without breaking normal reconnection behaviour.

Blazor stops automatically reconnecting (reconnectionOptions are ignored) when I specify a reconnectionHandler. I don't know the wirings of a default onConnectionDown and onConnectionUp , so I don't know how to overload them without breaking things. I simply wish to hook into reconnections while using default reconnection logic.

@myrup
Copy link
Author

myrup commented Nov 13, 2021

To whom it may concern I managed to hack this together with:

Blazor.start().then(() => {
    Blazor.defaultReconnectionHandler._reconnectionDisplay = {
        show: () => {},
        update: (d) => {},
        rejected: (d) => document.location.reload()
    };
});

But I'm still using the internal api because it's undocumented how to overload reconnectionHandler (@BrennanConroy) without breaking things.

My concerns echo the unresolved #29991 , so maybe it should be reopened?

@TanayParikh
Copy link
Contributor

TanayParikh commented Nov 15, 2021

I don't know the wirings of a default onConnectionDown and onConnectionUp , so I don't know how to overload them without breaking things.

The default wirings are here:

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;
}

And it's set here:

Blazor.defaultReconnectionHandler = new DefaultReconnectionHandler(logger);
options.reconnectionHandler = options.reconnectionHandler || Blazor.defaultReconnectionHandler;

@pcarnella
Copy link

pcarnella commented Nov 17, 2021

To whom it may concern I managed to hack this together with:

Blazor.start().then(() => {
    Blazor.defaultReconnectionHandler._reconnectionDisplay = {
        show: () => {},
        update: (d) => {},
        rejected: (d) => document.location.reload()
    };
});

This isn't working for me, F12 error: Blazor has already started. I put the code in _layout:
...

<script src="_framework/blazor.server.js"></script>
<script>
    Blazor.start().then(() => {
        Blazor.defaultReconnectionHandler._reconnectionDisplay = {
            show: () => {},
            update: (d) => {},
            rejected: (d) => document.location.reload()
        };
    });
</script>
...

Any other ideas for forcing reload after publish/app pool recycle?

EDIT: Got it to work by adding autostart="false" to: <script src="_framework/blazor.server.js" autostart="false"></script>. Thanks @myrup !

@TanayParikh
Copy link
Contributor

TanayParikh commented Nov 17, 2021

For:

<script src="_framework/blazor.server.js"></script>

Add an autostart="false" attribute and value to the <script> tag for the Blazor script.

Please see https://docs.microsoft.com/aspnet/core/blazor/fundamentals/startup?view=aspnetcore-6.0 for more details.

@kostya9
Copy link

kostya9 commented Nov 21, 2021

The fix with setting a custom _reconnectionDisplay removes the nice overlay informing the user about the reconnect. I hacked together a wrapper that leaves the overlay intact and reloads the page if the server was rebooted and we can't recover:

Blazor.start().then(() => {
        Object.defineProperty(Blazor.defaultReconnectionHandler, '_reconnectionDisplay', {
            get() {
                return this.__reconnectionDisplay;
            },
            set(value) {
                this.__reconnectionDisplay = {
                    show: () => value.show(),
                    update: (d) => value.update(d),
                    rejected: (d) => document.location.reload()
                }
            }
        });
    });

@aryehsilver
Copy link

I get an error when using any of these options:
Uncaught TypeError: Cannot set properties of undefined (setting '_reconnectionDisplay')

@TanayParikh
Copy link
Contributor

I'll just reiterate @BrennanConroy's point above about the official recommended approach to achieve this behavior is documented here: https://docs.microsoft.com/aspnet/core/blazor/fundamentals/signalr?view=aspnetcore-6.0#modify-the-reconnection-handler-blazor-server

_reconnectionDisplay is a framework internal undocumented API subject to change without notice, and we do not recommend taking that approach.

@aryehsilver
Copy link

@TanayParikh But I don't want to reconstruct the whole reconnection display! Why can't there be an official way to customise it without reconstructing it?
It is a very sought after feature so why can't you give an official way to reload the page when it can't connect to the server?
This should even be the default in every application. Why would an end user want anything else? If it can't connect the app is useless so what else would you want to do besides reload?

@myrup
Copy link
Author

myrup commented Nov 24, 2021

@aryehsilver @TanayParikh @BrennanConroy I agree the official way is borderline useless because of its complexity to implement. I suggest making something like _reconnectionDisplay officially supported.

@aryehsilver
Copy link

I suggest making something like _reconnectionDisplay officially supported.

@myrup That will never happen, it would be too useful!

@TanayParikh TanayParikh removed ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. Status: Resolved labels Dec 8, 2021
@TanayParikh TanayParikh changed the title Blazor.defaultReconnectionHandler._reconnectCallback undefined Document Configurable Reconnection Behavior Dec 8, 2021
@TanayParikh TanayParikh added the Docs This issue tracks updating documentation label Dec 8, 2021
@TanayParikh
Copy link
Contributor

TanayParikh commented Dec 8, 2021

Re-opening to consider adding additional doc details on the reconnection behavior or provide alternate reconnection configurability.

Thread summary is there are concerns with the existing documented approach as it overrides the existing reconnection behavior.

@TanayParikh TanayParikh reopened this Dec 8, 2021
@TanayParikh TanayParikh removed the Docs This issue tracks updating documentation label Dec 8, 2021
@TanayParikh TanayParikh changed the title Document Configurable Reconnection Behavior Configurable Reconnection Behavior Dec 8, 2021
@mkArtakMSFT mkArtakMSFT added this to the Backlog milestone Dec 8, 2021
@ghost
Copy link

ghost commented Dec 8, 2021

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 mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Dec 8, 2021
@TanayParikh
Copy link
Contributor

Thanks all for the discussion, I'm closing this issue out as a duplicate, and tracking progress here: #9256

@TanayParikh TanayParikh added the ✔️ Resolution: Duplicate Resolved as a duplicate of another issue label Dec 8, 2021
@ghost ghost added the Status: Resolved label Dec 8, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jan 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one ✔️ Resolution: Duplicate Resolved as a duplicate of another issue Status: Resolved
Projects
None yet
Development

No branches or pull requests

8 participants