-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Graceful reconnection alternative #12853
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
Graceful reconnection alternative #12853
Conversation
I want to make sure we get a signoff re: security for something we're adding this late in the game. I'm impressed on the quick turnaround on this 😆 |
src/Components/Server/src/Builder/ComponentEndpointConventionBuilder.cs
Outdated
Show resolved
Hide resolved
src/Components/Server/src/Builder/ComponentEndpointRouteBuilderExtensions.cs
Show resolved
Hide resolved
src/Components/Server/src/Builder/ComponentEndpointConventionBuilder.cs
Outdated
Show resolved
Hide resolved
src/Components/Server/test/ComponentEndpointRouteBuilderExtensionsTest.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor feedback, this looks really good so far.
Tests? Both for the new endpoint and for the disconnection functionality in general? We might need to add a "test" endpoint to the server that can be used to query circuits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this! Seems so much clearer than trying to guess based on heartbeats.
I'm approving now because this looks so good, but I'm sure you'll address the comments too.
Based on @anurse's comments elsewhere, maybe at some point in the future SignalR could introduce a new method for notifying the app about definitely-graceful disconnections, if it turns out we can reliably distinguish between things like "user switched phone into flight mode" (ungraceful) and "user navigated away from page" (graceful). If that does become true in the future, maybe that would be even more reliable than using beacons, since it would be harder for a misbehaving client to mislead us. We'd be free to switch to a different implementation, since this beacon technique is purely an internal implementation detail. For now, the beacon definitely looks like the best bet to me. |
97a21fd
to
102efca
Compare
🆙 📅 Will merge tomorrow unless there's important feedback |
102efca
to
18d145b
Compare
Uh oh!
There was an error while loading. Please reload this page.