Skip to content

Provide clarity regarding Server-side blazor SignalR Keep-Alive & Client Timeout #30136

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
garrettlondon1 opened this issue Aug 24, 2023 · 14 comments · Fixed by #32237 or #32331
Closed
Assignees
Labels
Blazor doc-enhancement Pri2 Source - Docs.ms Docs Customer feedback via GitHub Issue

Comments

@garrettlondon1
Copy link

Important
The Keep-Alive interval (keepAliveIntervalInMilliseconds or KeepAliveInterval) isn't directly related to the reconnection UI appearing. The Keep-Alive interval doesn't necessarily need to be changed. If the reconnection UI appearance issue is due to timeouts, the server timeout can be increased and the Keep-Alive interval can remain the same. The important consideration is that if you change the Keep-Alive interval, make sure that the timeout value is at least double the value of the Keep-Alive interval and that the Keep-Alive interval on the server matches the client setting.

The KeepAliveInterval isn't directly related to the reconnection UI appearing.

I think these two ways of phrasing the situation around Blazor Server and the "Reconnecting" modal is confusing with words like "isn't directly related", "doesn't necessarily need to be changed".. What is the exact SignalR logic for deciding whether the Reconnecting modal appears and to what extent can Blazor Server developers control this to fit their specific use case.


Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

@dotnet-bot dotnet-bot added Blazor Source - Docs.ms Docs Customer feedback via GitHub Issue labels Aug 24, 2023
@garrettlondon1
Copy link
Author

Related dotnet/aspnetcore#48724

@guardrex
Copy link
Collaborator

guardrex commented Aug 24, 2023

Thanks, @garrettlondon1.

cc: @mkArtakMSFT

@danroth27 ... I see you agreed on the phrasing. That phrasing was provided (not necessarily for publication) by Brennan in a remark here ...

#27197 (comment)

... so perhaps I can work with Brennan to improve those remarks. However, do you want to table this for now and work on it later when .NET 8 work cools off of bit, or should we ping him now?

Brennan ... when you 👀 this ...

Can we remove the words "directly" and "necessarily," along with shortening the Keep-Alive comment, to firm up the concept that the Keep-Alive isn't related to the reconnection UI appearing? If so, a revision might go like ...

The Keep-Alive interval doesn't need to be changed to configure how long the app waits before showing the reconnection UI. If the reconnection UI appearance is due to timeouts, the ClientTimeoutInterval and HandshakeTimeout can be increased, and the KeepAliveInterval can remain the same. The important consideration is that if you change the Keep-Alive interval, make sure that the client timeout value is at least double the value of the Keep-Alive interval and that the Keep-Alive interval on the client matches the server setting.

Also, I'll also check again on the split between the Fundamentals > SignalR coverage and Host and Deploy > Blazor Server coverage. We want remarks logically placed, not duplicated, and cross-linked where needed. The fundamentals coverage is at ...

https://learn.microsoft.com/en-us/aspnet/core/blazor/fundamentals/signalr?view=aspnetcore-7.0#configure-signalr-timeouts-and-keep-alive-on-the-client

... and 🦖 NOTE TO SELF 🦖 ... probably need to cross-link from the section that only discusses the reconnection UI to where we discuss config that extends the timeouts ...

https://learn.microsoft.com/en-us/aspnet/core/blazor/fundamentals/signalr?view=aspnetcore-7.0#reflect-the-server-side-connection-state-in-the-ui

@danroth27
Copy link
Member

I can work with Brennan to improve those remarks

Yeah, after seeing this user feedback and reviewing the text the timeout guidance seems like it could be clarified and simplified. For example, if the Keep-Alive interval isn't related to the reconnection UI, then why is it mentioned here? It sounds like we also need better guidance on how to make the reconnection UI show up in a timely fashion. I think we should work with @javiercn and @halter73 on this.

do you want to table this for now and work on it later when .NET 8 work cools off of bit?

Yes, the .NET 8 docs work takes priority right now.

@guardrex
Copy link
Collaborator

guardrex commented Aug 24, 2023

why is it mentioned here?

I think perhaps because ...

  • Devs have been confused about what affects the reconnection UI appearing in relation to all of the timeouts.
  • If they change the client timeout, which does affect the reconnection UI appearing, then they need to react with the Keep-Alive timeout, too, per Brennan's following remark. I think that's what Brennan meant by "not directly related" ... it is indirectly related via the client timeout changing, which then affects how one would adjust the Keep-Alive timeout.

Also, this bit ... this whole IMPORTANT note ... is an in-passing informational message about the server timeouts, of which Keep-Alive is one that has to be discussed generally. This note contains a clarifying remark about the reconnection UI, but that's not really what this whole section is about. When I assess how this is covered, I might be able to improve how its laid out and cross-linked.

https://learn.microsoft.com/en-us/aspnet/core/blazor/host-and-deploy/server?view=aspnetcore-7.0#global-deployment-and-connection-failures

Yes, the .NET 8 docs work takes priority right now.

I'll ping him later ...... even the lowly 🦖 is buried in JS interop full-stack UI updates at the moment ⛏️🗻😅.

@guardrex guardrex added the Pri2 label Aug 24, 2023
@Rick-Anderson Rick-Anderson moved this to P2 - Medium Priority in Blazor.Docs Oct 18, 2023
@guardrex guardrex moved this from P2 - Medium Priority to In progress in Blazor.Docs Apr 3, 2024
@guardrex
Copy link
Collaborator

guardrex commented Apr 3, 2024

@garrettlondon1 ... I finally 😅 reached this. It's been a grueling year of major updates to cover BWAs/.NET 8.

I understand from the PU issue that you opened that they've marked it for for work for .NET 10 (2025), and I understand that you wished for a higher priority. At least on the docs side, we can make adjustments now to improve the content.

I'd like to do this in two steps (two PRs) ...

  • I'd like to move the SignalR configuration content in Global deployment and connection failures of the Host and deploy node into the Fundamentals > Signal topic along with the rest of the SignalR configuration. That makes sense ... all configuration in one article. The Host and deploy article should only keep things that pertain directly to hosting/deployment, such as the Azure SignalR Service. Also on this PR, I'd like to strike the confusing lines that you called out. I can see that those two sentences don't add any significant value to Brennan's important remark about the relationship between setting the client timeout interval and the handshake timeout. I'll just 🔪 the confusing lines and leave the remaining remarks.
  • After improving organization, then I think we should look at improving the guidance itself in a couple of ways ...
    • Javier and/or Brennan and I discussed several years ago the difference between Blazor circuit SignalR connection configuration and other SignalR connection configuration. I'm not convinced right now that enough has been done to make it clear in the guidance, so I'd like to take a look at possibly improving that distinction.
    • The current coverage in the Global deployment and connection failures section addresses the situation where a slow connection is spawning the reconnection UI faster than the dev wishes. They want to delay and give the connection more time to come back up. You're asking for ... correct me if I'm wrong ... you're asking for showing the reconnection UI faster ... sooner ... and that's still under consideration on the PU issue AFAICT. Even if it isn't possible today to configure the Blazor circuit for the outcome that you'd like, the article can still address it and refer to the PU issue saying that it's under consideration for a future release.

I'm going to issue the first PR and go ahead and merge it this morning, and then I'll get into the second PR and ping for the PU ... "product unit" btw ... ping the PU tomorrow to look at what I come up with for the second PR.

If you have any further remarks, you can apply them here. The PR that I'm about to set up will go in immediately, so we won't have time there to discuss anything. It's going to focus on the content organization and striking those foul sentences 😈.

@garrettlondon1
Copy link
Author

  • you're asking for showing the reconnection UI faster ... sooner ... and that's still under consideration on the PU issue AFAICT.

You are correct here. I explain Blazor Server to people like a video game, because that's exactly what the Websocket connection is.

  • It's not like other frontend technology where SignalR is a background process and can gracefully fail

Much appreciate the update, and looking forward to clarifying these on the docs!

The way that DOM updates are built up and then sent, all at once, upon a reconnection, is one of the most important things to note.

It's kind of like if you were a kid at an arcade, put $1 in the game, press the button, nothing happens, so you naturally keep pressing the button.

  • Your intention was to only play once, now you've played 15 times and lost all your money and now you're crying because the game glitched and you've lost everything.
  • That's Blazor Server in the enterprise, just waiting to happen with sensitive financial orders, healthcare data, you name it.

@guardrex
Copy link
Collaborator

guardrex commented Apr 3, 2024

At a very minimum, we can include a section on it and link the PU issue. One of the updates that I'm making now is taking that current guidance and giving it the explicit section title of Delay the appearance of the reconnection UI. That might be temporary because the 2nd PR tomorrow might make further content layout and section changes ... in fact, I think it's likely that further changes will be made tomorrow.

I haven't analyzed the full PU issue yet. Question ❓ .... The current guidance is about delaying the appearance of the UI by changing the ClientTimeoutInterval and the HandshakeTimeout on the server and changing withServerTimeout on the client ... IF I'm understanding the PU's remarks thus far. If that's right, what happens if these values are cut in HALF from the default values? That doesn't speed up the appearance of the reconnection UI? I just mean logically that would be the effect on face value from their guidance thus far.

@garrettlondon1
Copy link
Author

garrettlondon1 commented Apr 3, 2024

If that's right, what happens if these values are cut in HALF from the default values? That doesn't speed up the appearance of the reconnection UI? I just mean logically that would be the effect on face value from their guidance thus far.

That's a great question: and I don't 100% know the answer. From my testing, it does cut the reconnection modal appearing in half. But, due to the guidance of both of those values "not necessarily" effecting the reconnection modal, I'm not positive.

For a while, I set the keep-alive to 1 or 2 seconds, but I decided against it in Production. If I disconnected, the reconnection modal would appear very quickly.

The only confusing point in the websocket connection is when the websocket goes from "connected" state > "reconnecting", before the reconnection UI is shown. I can't even find a JS method in the ReconnectHandler to log when that happens

@guardrex
Copy link
Collaborator

guardrex commented Apr 3, 2024

I see.

I want to do some testing on this, too. What I might do on the 2nd PR is refactor away from "only delay" to content that reads more like "control the timing of" (i.e., increase or decrease the period). Then, Brennan can look at that and say if my hunches are correct and if the new language is good.

WRT an immediate display of the reconnection UI, I think they'll say that it isn't possible ... which is part of the PU issue, right? Again, I haven't read it in detail yet. I'm just trying to make the basic content improvements at the moment. I'll dig into that issue shortly.

I agree that the "not necessarily" language was a poor choice of words to take from Brennan's discussion remarks. I think Brennan was saying that they are indirectly related such that ...

if you change the Keep-Alive interval, make sure that the timeout value is at least double the value of the Keep-Alive interval and that the Keep-Alive interval on the server matches the client setting.

The fix that I'm making now to remove the two confusing sentences hopefully will take care of that problem. We'll see on review of the 2nd PR tomorrow.

@guardrex
Copy link
Collaborator

guardrex commented Apr 3, 2024

Thus far, I've cleaned up the organization. The content on "delaying" the reconnection UI is now in a better spot in the SignalR article with the rest of the SignalR configuration and reconnection UI guidance.

https://learn.microsoft.com/en-us/aspnet/core/blazor/fundamentals/signalr?view=aspnetcore-8.0#delay-the-appearance-of-the-reconnection-ui

Next, I'll see about further updates that will require Brennan to take a look. I'll probably place the PR this morning, sleep on it 🛌 💤 for final updates tomorrow morning, and then ping him to take a look.

@guardrex
Copy link
Collaborator

guardrex commented Apr 3, 2024

@garrettlondon1 ... I'm done with most of the basic updates (rough draft) on the PR. Testing confirms that cutting the values in half, which keeps configuration in line with Brennan's recommendations, does indeed spawn the reconnection UI in half the time on a lost connection. Therefore, the updates seem good so far.

I don't have a remark in the article yet on possible future enhancements with a cross-link to the PU issue. I'll pick back up with this work Thursday morning and see how that can be phrased and placed.

@github-project-automation github-project-automation bot moved this from In progress to Done in Blazor.Docs Apr 12, 2024
@garrettlondon1
Copy link
Author

garrettlondon1 commented Apr 12, 2024

Just reviewed your PR @guardrex .. if we could follow up with @MackinnonBuck regarding the reconnection UI, that would be much appreciated. I still feel like the reconnection UI is not explained at all in the docs.

This ticket was closed, but the most important part remains unanswered: "What is the exact SignalR logic for deciding whether the Reconnecting UI appears and to what extent can Blazor Server developers control this to fit their specific use case."

@guardrex
Copy link
Collaborator

@garrettlondon1 ... You're right. The current content only goes this far ...

The timing of the appearance of the reconnection UI is influenced by adjusting the Keep-Alive interval and timeouts on the client.

... and that's not explicit enough. I'll try again after we hear back from Mackinnon.

@guardrex guardrex reopened this Apr 12, 2024
@github-project-automation github-project-automation bot moved this from Done to Triage in Blazor.Docs Apr 12, 2024
@guardrex guardrex moved this from Triage to P2 - Medium Priority in Blazor.Docs Apr 12, 2024
@guardrex
Copy link
Collaborator

I've placed a PR with my best guess based on PU discussion and framework behavior here ... #32331. Let's take up further discussion on the PR when Mackinnon returns.

@guardrex guardrex moved this from P2 - Medium Priority to In progress in Blazor.Docs Apr 16, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in Blazor.Docs Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blazor doc-enhancement Pri2 Source - Docs.ms Docs Customer feedback via GitHub Issue
Projects
Archived in project
4 participants