Fix Blazor Server Reconnection Mechanism #36126
Conversation
TanayParikh
requested review from
a team,
BrennanConroy and
halter73
as code owners
| } | ||
|
|
||
| const reconnect = async (existingConnection?: HubConnection): Promise<boolean> => { | ||
| Blazor.reconnect = async (existingConnection?: HubConnection): Promise<boolean> => { |
There was a problem hiding this comment.
We were setting this after we tried to init the DefaultReconnectionHandler which needs this reconnect function, leading to some null refs.
TanayParikh
added
area-blazor
javiercn
left a comment
javiercn
left a comment
There was a problem hiding this comment.
Looks good.
I trust you've done due diligence to manually test this, since it's hard/impossible to test it with automation.
as long as the signalr folks are happy with the changes there.
|
|
/backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1230887022 |
|
@TanayParikh backporting to release/6.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix Blazor Server Reconnection Mechanism
warning: Cannot merge binary files: src/Components/Web.JS/dist/Release/blazor.server.js (HEAD vs. Fix Blazor Server Reconnection Mechanism)
Using index info to reconstruct a base tree...
M src/Components/Web.JS/dist/Release/blazor.server.js
M src/Components/Web.JS/src/Boot.Server.ts
M src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectDisplay.ts
M src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectionHandler.ts
Falling back to patching base and 3-way merge...
Auto-merging src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectionHandler.ts
Auto-merging src/Components/Web.JS/src/Platform/Circuits/DefaultReconnectDisplay.ts
Auto-merging src/Components/Web.JS/src/Boot.Server.ts
CONFLICT (content): Merge conflict in src/Components/Web.JS/src/Boot.Server.ts
Auto-merging src/Components/Web.JS/dist/Release/blazor.server.js
CONFLICT (content): Merge conflict in src/Components/Web.JS/dist/Release/blazor.server.js
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix Blazor Server Reconnection Mechanism
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@dotnet/aspnet-build could this please be merged. The helix test keeps timing out at 4 hours (tried retry). The commit prior to the merge commit passed as expected: 259c84f |
|
@TanayParikh I suspect the problem is a known issue. I retried the failed jobs because Core-Eng drained the macOS 10.14 queue. |
Ah okay. The backport PR is also facing the same issue of 4h timeouts on the helix queue. I've retriggered that as well, 🤞 it works as expected now. |
ghost
added this to the
Fixes #32545