Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

PlainTextFormatter is interpretting some objects as json that are not. #191

Open
Nakano37 opened this issue Nov 23, 2024 · 7 comments
Open

Comments

@Nakano37
Copy link
Contributor

When trying to log certain objects (though seemingly not all) we get an Internal exception:

[Information] [2024-11-22 22:04:37.490] [13] - Internal Logging exceptionSerialization and deserialization of 'System.Type' instances is not supported. Path: $.ValueType. :    at System.Text.Json.ThrowHelper.ThrowNotSupportedException(WriteStack& state, NotSupportedException ex)
   at System.Text.Json.Serialization.JsonConverter`1.WriteCore(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.Serialize(Utf8JsonWriter writer, T& rootValue, Object rootValueBoxed)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.SerializeAsObject(Utf8JsonWriter writer, Object rootValue)
   at ZLogger.Internal.CodeGeneratorUtil.AppendAsJson(Utf8StringWriter`1& stringWriter, Object value, Type inputType)
   at ZLogger.MessageSequence.ToString(IBufferWriter`1 writer, MagicalBox box, Span`1 parameters)
   at ZLogger.LogStates.InterpolatedStringLogState.ToString(IBufferWriter`1 writer)
   at ZLogger.ZLoggerEntry`1.ToString(IBufferWriter`1 writer)
   at ZLogger.Formatters.PlainTextZLoggerFormatter.FormatLogEntry(IBufferWriter`1 writer, IZLoggerEntry entry)
   at ZLogger.ZLoggerEntry`1.FormatUtf8(IBufferWriter`1 writer, IZLoggerFormatter formatter)
   at ZLogger.AsyncStreamLineMessageWriter.WriteLoop()

Which comes from a line that looks like:
Debug.Logger.ZLogInformation($"[Match] AddPlayer({player} {player.AccountID})");
(errors on the {player} passed in).

player is PlayerEntity class that inherits from an Entity class that is tagged as with [Serializable] if that helps.
Entity implements a ToString override:

    public override string ToString()
    {
      var nameValue = GetAttribute(CoreAttributes.Name.Key).Value ?? "";
      return $"{entityName} [{(nameValue is LocalizableText loc ? loc.ID : nameValue)}]";
    }

if I change the ZLogInformation line to
Debug.Logger.ZLogInformation($"[Match] AddPlayer({player.ToString()} {player.AccountID})");
everything works as expected.

Is there someway to tell the formatter to not "AppendAsJson" and just ToString an object? I've been trying to figure out how to make a custom formatter to do this but it seems like the actual issue is inside of the internal sealed class MessageSequence, where it does:

                    ref var p = ref parameters[parameterIndex++];
                    if (p.BoxOffset == -2) // as "json"
                    {
                        CodeGeneratorUtil.AppendAsJson(ref stringWriter, p.BoxedValue, p.Type);
                    }
                    else if (!box.TryReadTo(p.Type, p.BoxOffset, p.Alignment, p.Format, ref stringWriter))
                    {
                        if (p.BoxedValue is string s)
                        {
                            stringWriter.AppendFormatted(s, p.Alignment, p.Format);
                        }
                        else if (p.BoxedValue is IEnumerable enumerable)
                        {
#if NET8_0
                            if (p.BoxedValue is ISpanFormattable spanFormattable)
                            {
                                stringWriter.AppendFormatted(p.BoxedValue, p.Alignment, p.Format);
                            }
                            else
                            {
                                CodeGeneratorUtil.AppendAsJson(ref stringWriter, p.BoxedValue, p.Type);
                            }
#else
                            CodeGeneratorUtil.AppendAsJson(ref stringWriter, p.BoxedValue, p.Type);
#endif
                        }
                        else
                        {
                            stringWriter.AppendFormatted(p.BoxedValue, p.Alignment, p.Format);
                        }
                    }
@Nakano37 Nakano37 changed the title PlainTextFormatter is interpretting some objects as objects that are not. PlainTextFormatter is interpretting some objects as json that are not. Nov 23, 2024
@Nakano37
Copy link
Contributor Author

Nakano37 commented Nov 23, 2024

I was able to fix the issue by forking ZLogger and modifing MessageSequence.ToString line 265 to do stringWriter.AppendFormatted(p.BoxedValue, p.Alignment, p.Format); instead of CodeGeneratorUtil.AppendAsJson(ref stringWriter, p.BoxedValue, p.Type);

Obviously it's not ideal to have to fork the entire project for this one line. If there was just some way to set an option or something that would prevent it from assuming Json...

@neuecc
Copy link
Member

neuecc commented Dec 5, 2024

line 265 seems to be around IEnumerable, but is player an IEnumerable?

@Nakano37
Copy link
Contributor Author

Yes, player has a long object heirarchy but at it's base level it is a collection of attributes which inherits from IEnumerable

@neuecc
Copy link
Member

neuecc commented Dec 18, 2024

Thank you.
Processing IEnumerable through JSON is a simple approach and I don't consider it the best, but there aren't many good alternatives.
This is because starting to do this would require implementing a serializer-like layer in ZLogger itself (while I have extensive knowledge about serializers and could implement it, I don't think this is something ZLogger should do).
As a workaround for this issue, as shown in the code, implementing ISpanFormattable will take precedence.

@Nakano37
Copy link
Contributor Author

I'd just add ISpanFormattable for these Entities but Unity does not seem to support it, nor do I seem able to install it through NuGet for Unity, at least not Unity 2022.3.21f1.

@neuecc
Copy link
Member

neuecc commented Dec 24, 2024

I see, in addition to ISpanFormattable, let's also add the IFormattable check.
This way, custom ToString will be prioritized on any platform.

@Nakano37
Copy link
Contributor Author

yeah that seems like it would work, thanks.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants