dotnet / WatsonTcp

WatsonTcp is the easiest way to build TCP-based clients and servers in C#.
MIT License
599 stars 117 forks source link

System.NotSupportedException when SerializeJson is called with an exception as parameter #226

Closed fisherman6v6 closed 1 year ago

fisherman6v6 commented 2 years ago

Hi! I've found that when a Connect() fails instead of throwing the correct SocketException, aSystem.NotSupportedException is thrown by System.Text.Json with the following message:

'Serialization and deserialization of 'System.Reflection.MethodBase' instances are not supported. Path: $.TargetSite.'

Apparently System.Text.Json does not handle correctly exception serialization as stated here for example https://stackoverflow.com/questions/71132371/why-does-system-text-json-throw-a-notsupportedexception-for-system-intptr-wh

Could this be corrected by avoiding the full exception serialization?

jchristn commented 2 years ago

Hi, this is a biproduct of moving away from Newtonsoft. Newtonsoft is capable of serializing exceptions, and System.Text.Json has some issues with it. You're welcome to implement your own serializer and attach it to SerializationHelper if you wish to either revert to Newtonsoft, or, if you wish to implement your own methods and converters to better handle them.

jchristn commented 2 years ago

Hi @fisherman6v6 alternatively if you have a converter for Exception that you'd like to include, I'd be happy to incorporate either directly or via a PR! I'm searching for one, because I think this is a common use case.

fisherman6v6 commented 1 year ago

Wouldn't be enough to pass back just the exception object inside the Exception property and leave the management to the user? In other words, is it necessary to serialize the whole exception?

jchristn commented 1 year ago

Hi @fisherman6v6 I misunderstood what you were saying. I see the issue now, there are several places where ExceptionEventArgs are passed which have a Json property, which doesn't work correctly without a custom converter. Addressing this now!

jchristn commented 1 year ago

Hi @fisherman6v6 NuGet v5.0.10 removes this property. Please let me know if there are other areas you see that I've missed where exception objects are serialized, or please close if this fixes the issue for you.

NuGet: https://www.nuget.org/packages/WatsonTcp/5.0.10 Commit: https://github.com/jchristn/WatsonTcp/commit/1aed91def41b0073020eb0cfd7b5f018ea4116d9