fix: throw InvalidOperationException if network client attempts to send messages while disconnected #1118
Conversation
Check if the connection reference is null before we send off the data. If it's null, throw a InvalidOperationException instead of letting it throw a NullReferenceException.
…ion Send attempts
| try | ||
| { | ||
| clientNetClient.Send(new byte[] { 0x13, 0x37, 0x69 }); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Assert.That(ex is InvalidOperationException); | ||
| } |
There was a problem hiding this comment.
This should use Assert.Throws instead
| [Test] | ||
| // Ensure that the created client game object and it's component(s) isn't null. | ||
| public void EnsureNetworkClientIsNotNull() | ||
| { | ||
| Assert.IsNotNull(clientGO); | ||
| Assert.IsNotNull(clientNetClient); | ||
| } | ||
|
|
||
| [Test] | ||
| // Ensure that the client object's Player reference is null. | ||
| public void EnsureNetworkClientPlayerIsNull() | ||
| { | ||
| Assert.IsNull(clientNetClient.Player); | ||
| } |
There was a problem hiding this comment.
we probably dont need these tests as they are just validating the setup,
I think debug.assert is better for stuff like this because these checks are more for debugging a test if it goes wrong
There was a problem hiding this comment.
Alright, makes sense… I’ll use Debug.Assert in the setup routine.
| if (Player == null) | ||
| throw new InvalidOperationException("Cannot send the data. Send was called while the NetworkClient connection was null. " + | ||
| "The connection may have been disconnected or terminated."); |
There was a problem hiding this comment.
Would bee good to have a "throw not connected" function for these 3 since the message iis the same, less duplicate code/strings if we ever want to change it
something like
void Send() {
if (Player == null)
ThrowSendNotConnected();
...
}
void ThrowSendNotConnected() {
throw new InvalidOperationException(...);
}There was a problem hiding this comment.
Ok, good point. I have no problems with that, I’ll amend it.
## [129.3.1](v129.3.0...v129.3.1) (2022-12-21) ### Bug Fixes * throw InvalidOperationException if network client attempts to send messages while disconnected ([#1118](#1118)) ([b7ae4de](b7ae4de))
|
🎉 This PR is included in version 129.3.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Fixes a NullReferenceException being fired since the NetworkClient's Player reference is null when trying to use the
Send(...)function while disconnected. Discovered by myself while implementing some backend stuff in Mirage Standalone.Instead, throw a InvalidOperationException and explain to the user that sending things when disconnected is bad.