Skip to content

Commit

Permalink
fix: increasing log to warning when receiving known type without hand…
Browse files Browse the repository at this point in the history
…ler (#1072)

If it is a known type it is likely developer mistake, so we want to warn about it so it can be fixed
  • Loading branch information
James-Frowen committed Apr 14, 2022
1 parent 25ab6f3 commit 05db6cf
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
12 changes: 6 additions & 6 deletions Assets/Mirage/Runtime/MessageHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,18 @@ internal void InvokeHandler(INetworkPlayer player, int msgType, NetworkReader re
{
msgDelegate.Invoke(player, reader);
}
// check LogEnabled to stop allocations if not enabled
else if (logger.LogEnabled())
else
{
if (MessagePacker.MessageTypes.TryGetValue(msgType, out Type type))
{
// todo use warning here instead of log, it seems important to know a known type has no handler
// probably fine to leave unexpected message as log, but maybe we should handle it differently? we dont want someone spaming ids to find a handler they can do stuff with...
logger.Log($"Unexpected message {type} received from {player}. Did you register a handler for it?");
// this means we received a Message that has a struct, but no handler, It is likely that the developer forgot to register a handler or sent it by mistake
// we want this to be warning level
if (logger.WarnEnabled()) logger.LogWarning($"Unexpected message {type} received from {player}. Did you register a handler for it?");
}
else
{
logger.Log($"Unexpected message ID {msgType} received from {player}. May be due to no existing RegisterHandler for this message.");
// todo maybe we should handle it differently? we dont want someone spaming ids to find a handler they can do stuff with...
if (logger.LogEnabled()) logger.Log($"Unexpected message ID {msgType} received from {player}. May be due to no existing RegisterHandler for this message.");
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions Assets/Tests/Runtime/Serialization/MessageHandlerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,28 +71,28 @@ public void DisconnectsIfHandlerHasException(bool disconnectOnThrow)
}

[Test]
public void ThrowsWhenNoHandlerIsFound()
public void LogsWhenNoHandlerIsFound()
{
ExpectLog(() =>
{
int messageId = MessagePacker.GetId<SceneMessage>();
messageHandler.InvokeHandler(player, messageId, reader);
}
, $"Unexpected message {typeof(SceneMessage)} received from {player}. Did you register a handler for it?");
, LogType.Warning, $"Unexpected message {typeof(SceneMessage)} received from {player}. Did you register a handler for it?");
}

[Test]
public void ThrowsWhenUnknownMessage()
public void LogsWhenUnknownMessage()
{
const int id = 1234;
ExpectLog(() =>
{
messageHandler.InvokeHandler(player, id, reader);
}
, $"Unexpected message ID {id} received from {player}. May be due to no existing RegisterHandler for this message.");
, LogType.Log, $"Unexpected message ID {id} received from {player}. May be due to no existing RegisterHandler for this message.");
}

void ExpectLog(Action action, string log)
void ExpectLog(Action action, LogType type, string log)
{
ILogger logger = LogFactory.GetLogger(typeof(MessageHandler));
ILogHandler existing = logger.logHandler;
Expand All @@ -105,7 +105,7 @@ void ExpectLog(Action action, string log)

action.Invoke();

handler.Received(1).LogFormat(LogType.Log, null, "{0}", log);
handler.Received(1).LogFormat(type, null, "{0}", log);
}
finally
{
Expand Down

0 comments on commit 05db6cf

Please sign in to comment.