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

Fix spectator client not correctly reconnecting after shutdown #19187

Merged
merged 4 commits into from
Jul 18, 2022

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Jul 17, 2022

When switching servers over, I've notice a disparity in connections switching to the new spectator server (via datadog metrics). Users' clients would be stuck on the old spectator hub but also unable to make new spectator sessions.

Turns out this is due to incorrect use of SendAsync instead of InvokeAsync. The former does not block, nor propagate exceptions.

I'm not sure about the remaining SendAsync usage in SpectatorHub. We may want to switch them over as well.

I've tested this manually to work as expected.

@frenzibyte
Copy link
Member

frenzibyte commented Jul 18, 2022

Multiplayer client already uses InvokeAsync on all methods, so I think it wouldn't hurt to change them here as well (would also allow propagating hub exceptions like multiplayer client does when calling one of their methods)

@peppy
Copy link
Sponsor Member Author

peppy commented Jul 18, 2022

All the remaining usages of SendAsync were fire-and-forget, so seems best to update them, as that will have no effect on existing usages and is probably the more expected behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants