From d3f0e56361e319924a0b1527121fa3ce3fadaf8d Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Wed, 31 Jan 2024 14:02:58 +0800 Subject: [PATCH] Backend(,Tests): stop using binary serialization We had a need in the past to serialize exceptions (as they could happen in off-line mode, when running as a cold-storage device), so that they could be reported later when the device comes online, but exceptions can't be seralizated to JSON (as explained in [1]), so we ended up using binary serialization (hooking it up in this past commit[2]). However, binary serialization is going away in .NET9[3] because of its potential security risk. Even though we doubt that for our use case we would be affected by this security vector, we: - Want to be prepared for the future. - Know that there were anyway edge cases where binary serialization was not actually working (e.g. see bug 240), and was causing crashes. We explored the idea of contributing an IException interface to the 'sentry-dotnet' repo [4] (this library is the replacement of SharpRaven, see [5]), so that we can serialize exceptions easily in JSON, for later deserializing them and send them straight to Sentry's API for report purposes, however: * We found adding the IException overloads to be extremely complicated due to the sheer amount of unit tests and things that Sentry has, that would need to be modified. * Given the above, we thought it would be too much work, and too much risk of not being accepted upstream. * Even if the IException overloads were accepted, the approach would still be a leaky abstraction because the type of the exception cannot be properly represented in a hypothetical IException's property, so we were/would ending up with hacky things such as an IsAggregateException:bool property, for example. But why end here and not have more bool types for other exceptions? Instead of the above nightmare we have decided to go for the simplest approach of all (the one that I should have done 3ish years ago when I was initially solving this problem, to avoid any OVERENGINEERING): just use good old Exception.ToString() method! This method provides, not only the type of the exception and its .Message property, also all its inner exceptions recursively. This is GOOD ENOUGH. Fixes https://github.com/nblockchain/geewallet/issues/240 Closes https://gitlab.com/nblockchain/geewallet/-/issues/174 [1] 403d5c7194cf0a53de548e65a3c222762df1d998 [2] 1f7b3b77e8def853fb8bed1c546902d06ce2b43c [3] https://twitter.com/SitnikAdam/status/1746874459640811575 [4] https://github.com/getsentry/sentry-dotnet [5] https://github.com/nblockchain/geewallet/pull/252 --- .../ExceptionMarshalling.fs | 180 +++++++++--------- .../GWallet.Backend.Tests-legacy.fsproj | 5 - .../GWallet.Backend.Tests.fsproj | 5 - src/GWallet.Backend.Tests/MarshallingData.fs | 141 ++++++-------- .../data/basicException.json | 9 +- .../data/customException.json | 9 +- .../data/customFSharpException.json | 9 +- .../data/customFSharpException_legacy.json | 13 -- .../data/fullException.json | 19 +- .../data/fullException_unixLegacy.json | 23 --- .../data/fullException_windowsLegacy.json | 23 --- .../data/innerException.json | 19 +- .../data/realException.json | 9 +- .../data/realException_unixLegacy.json | 13 -- .../data/realException_windowsLegacy.json | 13 -- src/GWallet.Backend/Infrastructure.fs | 38 +++- src/GWallet.Backend/Marshalling.fs | 58 +----- 17 files changed, 206 insertions(+), 380 deletions(-) delete mode 100644 src/GWallet.Backend.Tests/data/customFSharpException_legacy.json delete mode 100644 src/GWallet.Backend.Tests/data/fullException_unixLegacy.json delete mode 100644 src/GWallet.Backend.Tests/data/fullException_windowsLegacy.json delete mode 100644 src/GWallet.Backend.Tests/data/realException_unixLegacy.json delete mode 100644 src/GWallet.Backend.Tests/data/realException_windowsLegacy.json diff --git a/src/GWallet.Backend.Tests/ExceptionMarshalling.fs b/src/GWallet.Backend.Tests/ExceptionMarshalling.fs index d07a3a8aa..70126730f 100644 --- a/src/GWallet.Backend.Tests/ExceptionMarshalling.fs +++ b/src/GWallet.Backend.Tests/ExceptionMarshalling.fs @@ -1,14 +1,13 @@ namespace GWallet.Backend.Tests open System -open System.Runtime.Serialization open NUnit.Framework open GWallet.Backend -type CustomExceptionWithoutSerializationCtor = +type CustomExceptionWithoutInnerExceptionCtor = inherit Exception new(message) = @@ -17,8 +16,6 @@ type CustomExceptionWithoutSerializationCtor = type CustomException = inherit Exception - new(info: SerializationInfo, context: StreamingContext) = - { inherit Exception(info, context) } new(message: string, innerException: CustomException) = { inherit Exception(message, innerException) } new(message) = @@ -68,17 +65,17 @@ type ExceptionMarshalling () = cex Marshalling.Serialize ex - let msg = "Exceptions didn't match. Full binary form was " + let msg = "Expected Exception.ToString() differs from actual" #if LEGACY_FRAMEWORK - let legacyMsg = "(Legacy)Exceptions didn't match. Full binary form was " + let legacyIgnoreMsg = "Mono or old .NETFramework might vary slightly; there's no need to really do any regression testing here" #endif [] member __.``can serialize basic exceptions``() = let json = SerializeBasicException () Assert.That(json, Is.Not.Null) - Assert.That(json, Is.Not.Empty) - Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.BasicExceptionExampleInJson false msg) + Assert.That(json.Trim(), Is.Not.Empty) + Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.BasicExceptionExampleInJson msg) [] member __.``can deserialize basic exceptions``() = @@ -90,31 +87,24 @@ type ExceptionMarshalling () = Assert.Inconclusive "Fix the serialization test first" failwith "unreachable" - let ex: Exception = Marshalling.Deserialize basicExSerialized + let ex: MarshalledException = Marshalling.Deserialize basicExSerialized Assert.That(ex, Is.Not.Null) - Assert.That(ex, Is.InstanceOf()) - Assert.That(ex.Message, Is.EqualTo "msg") - Assert.That(ex.InnerException, Is.Null) - Assert.That(ex.StackTrace, Is.Null) + Assert.That(ex, Is.InstanceOf()) + Assert.That(ex.FullDescription.Trim().Length, Is.GreaterThan 0) + Assert.That( + MarshallingData.Sanitize ex.FullDescription, + Is.EqualTo (MarshallingData.Sanitize "System.Exception: msg") + ) [] member __.``can serialize real exceptions``() = let json = SerializeRealException () Assert.That(json, Is.Not.Null) - Assert.That(json, Is.Not.Empty) + Assert.That(json.Trim(), Is.Not.Empty) #if !LEGACY_FRAMEWORK - Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionExampleInJson false msg) + Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionExampleInJson msg) #else - if Config.IsWindowsPlatform () then - let serializedExceptionsAreSame = - try - MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionExampleInJson false msg - with - | :? AssertionException -> - MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionWindowsLegacyExampleInJson false legacyMsg - Assert.That serializedExceptionsAreSame - else - Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionUnixLegacyExampleInJson false legacyMsg) + Assert.Ignore legacyIgnoreMsg #endif [] @@ -127,20 +117,29 @@ type ExceptionMarshalling () = Assert.Inconclusive "Fix the serialization test first" failwith "unreachable" - let ex: Exception = Marshalling.Deserialize realExceptionSerialized + let ex: MarshalledException = Marshalling.Deserialize realExceptionSerialized Assert.That(ex, Is.Not.Null) - Assert.That(ex, Is.InstanceOf()) - Assert.That(ex.Message, Is.EqualTo "msg") - Assert.That(ex.InnerException, Is.Null) - Assert.That(ex.StackTrace, Is.Not.Null) - Assert.That(ex.StackTrace, Is.Not.Empty) + Assert.That(ex, Is.InstanceOf()) + Assert.That(ex.FullDescription.Trim().Length, Is.GreaterThan 0) +#if !LEGACY_FRAMEWORK + let expected = + sprintf + "System.Exception: msg at GWallet.Backend.Tests.ExceptionMarshalling.SerializeRealException() in %s/ExceptionMarshalling.fs:line 38" + MarshallingData.ThisProjPath + Assert.That( + MarshallingData.Sanitize ex.FullDescription, + Is.EqualTo (MarshallingData.Sanitize expected) + ) +#else + Assert.Ignore legacyIgnoreMsg +#endif [] member __.``can serialize inner exceptions``() = let json = SerializeInnerException () Assert.That(json, Is.Not.Null) - Assert.That(json, Is.Not.Empty) - Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.InnerExceptionExampleInJson false msg) + Assert.That(json.Trim(), Is.Not.Empty) + Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.InnerExceptionExampleInJson msg) [] member __.``can deserialize inner exceptions``() = @@ -152,31 +151,28 @@ type ExceptionMarshalling () = Assert.Inconclusive "Fix the serialization test first" failwith "unreachable" - let ex: Exception = Marshalling.Deserialize innerExceptionSerialized + let ex: MarshalledException = Marshalling.Deserialize innerExceptionSerialized Assert.That (ex, Is.Not.Null) - Assert.That (ex, Is.InstanceOf()) - Assert.That (ex.Message, Is.EqualTo "msg") - Assert.That (ex.StackTrace, Is.Null) - Assert.That (ex.InnerException, Is.Not.Null) - - Assert.That (ex.InnerException, Is.InstanceOf()) - Assert.That (ex.InnerException.Message, Is.EqualTo "innerMsg") - Assert.That (ex.InnerException.StackTrace, Is.Null) + Assert.That (ex, Is.InstanceOf()) + Assert.That(ex.FullDescription.Trim().Length, Is.GreaterThan 0) + Assert.That ( + MarshallingData.Sanitize ex.FullDescription, + Is.EqualTo (MarshallingData.Sanitize "System.Exception: msg ---> System.Exception: innerMsg --- End of inner exception stack trace ---") + ) [] member __.``can serialize custom exceptions``() = let json = SerializeCustomException () Assert.That(json, Is.Not.Null) - Assert.That(json, Is.Not.Empty) - Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomExceptionExampleInJson false msg) + Assert.That(json.Trim(), Is.Not.Empty) + Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomExceptionExampleInJson msg) [] - member __.``serializing custom exception not prepared for binary serialization, throws``() = - let exToSerialize = CustomExceptionWithoutSerializationCtor "msg" - let ex: MarshallingCompatibilityException = - Assert.Throws(fun _ -> Marshalling.Serialize exToSerialize |> ignore) - Assert.That(ex, Is.TypeOf()) - Assert.That(ex.Message, IsString.WhichContains "GWallet.Backend.Tests.CustomExceptionWithoutSerializationCtor") + member __.``serializing custom exception without inner ex ctor does not crash``() = + let exToSerialize = CustomExceptionWithoutInnerExceptionCtor "msg" + let serializedEx = (Marshalling.Serialize exToSerialize).Trim() + Assert.That(serializedEx, Is.Not.Null) + Assert.That(serializedEx.Trim().Length, Is.GreaterThan 0) [] member __.``can deserialize custom exceptions``() = @@ -188,21 +184,24 @@ type ExceptionMarshalling () = Assert.Inconclusive "Fix the serialization test first" failwith "unreachable" - let ex: Exception = Marshalling.Deserialize customExceptionSerialized + let ex: MarshalledException = Marshalling.Deserialize customExceptionSerialized Assert.That(ex, Is.Not.Null) - Assert.That(ex, Is.InstanceOf()) - Assert.That(ex.Message, Is.EqualTo "msg") - Assert.That(ex.InnerException, Is.Null) - Assert.That(ex.StackTrace, Is.Null) + Assert.That(ex, Is.InstanceOf()) + Assert.That( + MarshallingData.Sanitize ex.FullDescription, + Is.EqualTo (MarshallingData.Sanitize "GWallet.Backend.Tests.CustomException: msg") + ) [] member __.``can serialize F# custom exceptions``() = let json = SerializeCustomFSharpException () Assert.That(json, Is.Not.Null) - Assert.That(json, Is.Not.Empty) - - // strangely enough, message would be different between linux_vanilla_dotnet6 and other dotnet6 configs (e.g. Windows, macOS, Linux-github) - Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomFSharpExceptionExampleInJson true msg) + Assert.That(json.Trim(), Is.Not.Empty) +#if !LEGACY_FRAMEWORK + Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomFSharpExceptionExampleInJson msg) +#else + Assert.Ignore legacyIgnoreMsg +#endif [] member __.``can deserialize F# custom exceptions``() = @@ -214,36 +213,34 @@ type ExceptionMarshalling () = Assert.Inconclusive "Fix the serialization test first" failwith "unreachable" - let ex: Exception = Marshalling.Deserialize customExceptionSerialized + let ex: MarshalledException = Marshalling.Deserialize customExceptionSerialized Assert.That(ex, Is.Not.Null) - Assert.That(ex, Is.InstanceOf()) - Assert.That(ex.Message, Is.Not.Null) - Assert.That(ex.Message, Is.Not.Empty) - Assert.That(ex.InnerException, Is.Null) - Assert.That(ex.StackTrace, Is.Null) + Assert.That(ex, Is.InstanceOf()) + Assert.That(ex.FullDescription.Trim().Length, Is.GreaterThan 0) + + if ex.FullDescription.Contains "of type" then + // old version of .NET6? (happens in stockdotnet6 CI lanes) + Assert.That( + MarshallingData.Sanitize ex.FullDescription, + Is.EqualTo (MarshallingData.Sanitize "GWallet.Backend.Tests.CustomFSharpException: Exception of type 'GWallet.Backend.Tests.CustomFSharpException' was thrown.") + ) + else + Assert.That( + MarshallingData.Sanitize ex.FullDescription, + Is.EqualTo (MarshallingData.Sanitize "GWallet.Backend.Tests.CustomFSharpException: CustomFSharpException") + ) - // TODO: test marshalling custom exceptions with custom properties/fields, and custom F# exception with subtypes [] member __.``can serialize full exceptions (all previous features combined)``() = let json = SerializeFullException () Assert.That(json, Is.Not.Null) - Assert.That(json, Is.Not.Empty) - + Assert.That(json.Trim(), Is.Not.Empty) #if !LEGACY_FRAMEWORK - Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionExampleInJson false msg) + Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionExampleInJson msg) #else - if Config.IsWindowsPlatform () then - let serializedExceptionsAreSame = - try - MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionExampleInJson false msg - with - | :? AssertionException -> - MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionWindowsLegacyExampleInJson false legacyMsg - Assert.That serializedExceptionsAreSame - else - Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionUnixLegacyExampleInJson false legacyMsg) + Assert.Ignore legacyIgnoreMsg #endif [] @@ -256,12 +253,23 @@ type ExceptionMarshalling () = Assert.Inconclusive "Fix the serialization test first" failwith "unreachable" - let ex: Exception = Marshalling.Deserialize fullExceptionSerialized + let ex: MarshalledException = Marshalling.Deserialize fullExceptionSerialized Assert.That(ex, Is.Not.Null) - Assert.That(ex, Is.InstanceOf ()) - Assert.That(ex.Message, Is.Not.Null) - Assert.That(ex.Message, Is.Not.Empty) - Assert.That(ex.InnerException, Is.Not.Null) - Assert.That(ex.StackTrace, Is.Not.Null) - Assert.That(ex.StackTrace, Is.Not.Empty) + Assert.That(ex, Is.InstanceOf ()) + Assert.That(ex.FullDescription.Trim().Length, Is.GreaterThan 0) + +#if !LEGACY_FRAMEWORK + Assert.That( + MarshallingData.Sanitize ex.FullDescription, + Is.EqualTo ( + MarshallingData.Sanitize + <| sprintf + "GWallet.Backend.Tests.CustomException: msg ---> GWallet.Backend.Tests.CustomException: innerMsg --- End of inner exception stack trace --- at GWallet.Backend.Tests.ExceptionMarshalling.SerializeFullException() in %s/ExceptionMarshalling.fs:line 61" + MarshallingData.ThisProjPath + ) + ) +#else + Assert.Ignore legacyIgnoreMsg +#endif + diff --git a/src/GWallet.Backend.Tests/GWallet.Backend.Tests-legacy.fsproj b/src/GWallet.Backend.Tests/GWallet.Backend.Tests-legacy.fsproj index 7657a1ea9..04013b1f6 100644 --- a/src/GWallet.Backend.Tests/GWallet.Backend.Tests-legacy.fsproj +++ b/src/GWallet.Backend.Tests/GWallet.Backend.Tests-legacy.fsproj @@ -75,15 +75,10 @@ - - - - - diff --git a/src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj b/src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj index a61297f78..cea683187 100644 --- a/src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj +++ b/src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj @@ -39,18 +39,13 @@ - - - - - diff --git a/src/GWallet.Backend.Tests/MarshallingData.fs b/src/GWallet.Backend.Tests/MarshallingData.fs index 90e1e6a37..6bffab36b 100644 --- a/src/GWallet.Backend.Tests/MarshallingData.fs +++ b/src/GWallet.Backend.Tests/MarshallingData.fs @@ -16,23 +16,41 @@ module MarshallingData = let private executingAssembly = Assembly.GetExecutingAssembly() let private version = VersionHelper.CURRENT_VERSION let private binPath = executingAssembly.Location |> FileInfo - let private prjPath = Path.Combine(binPath.Directory.FullName, "..") |> DirectoryInfo + let private prjDir = Path.Combine(binPath.Directory.FullName, "..", "..", "..") |> DirectoryInfo let private RemoveJsonFormatting (jsonContent: string): string = - jsonContent.Replace("\r", String.Empty) - .Replace("\n", String.Empty) - .Replace("\t", String.Empty) + jsonContent + .Replace("\r\n", String.Empty) + .Replace("\n", String.Empty) + .Replace("\\r\\n", String.Empty) + .Replace("\\n", String.Empty) + .Replace("\t", String.Empty) let private InjectCurrentVersion (jsonContent: string): string = jsonContent.Replace("{version}", version) + let private NormalizePaths (jsonContent: string): string = + jsonContent.Replace("\\", "/") + + let internal ThisProjPath = + NormalizePaths prjDir.FullName + let private InjectCurrentDir (jsonContent: string): string = - jsonContent.Replace("{prjDirAbsolutePath}", prjPath.FullName.Replace("\\", "/")) + jsonContent.Replace("{prjDirAbsolutePath}", ThisProjPath) + + let rec private TrimOutsideAndInside(str: string) = + let trimmed = str.Replace(" ", " ").Trim() + if trimmed = str then + trimmed + else + TrimOutsideAndInside trimmed let internal Sanitize = RemoveJsonFormatting >> InjectCurrentVersion >> InjectCurrentDir + >> NormalizePaths + >> TrimOutsideAndInside let private ReadEmbeddedResource resourceName = Fsdk.Misc.ExtractEmbeddedResourceFileContents resourceName @@ -50,12 +68,6 @@ module MarshallingData = let RealExceptionExampleInJson = ReadEmbeddedResource "realException.json" - let RealExceptionUnixLegacyExampleInJson = - ReadEmbeddedResource "realException_unixLegacy.json" - - let RealExceptionWindowsLegacyExampleInJson = - ReadEmbeddedResource "realException_windowsLegacy.json" - let InnerExceptionExampleInJson = ReadEmbeddedResource "innerException.json" @@ -65,81 +77,46 @@ module MarshallingData = let CustomFSharpExceptionExampleInJson = ReadEmbeddedResource "customFSharpException.json" - let CustomFSharpExceptionLegacyExampleInJson = - ReadEmbeddedResource "customFSharpException_legacy.json" - let FullExceptionExampleInJson = ReadEmbeddedResource "fullException.json" - let FullExceptionUnixLegacyExampleInJson = - ReadEmbeddedResource "fullException_unixLegacy.json" - - let FullExceptionWindowsLegacyExampleInJson = - ReadEmbeddedResource "fullException_windowsLegacy.json" - - let rec TrimOutsideAndInside(str: string) = - let trimmed = str.Replace(" ", " ").Trim() - if trimmed = str then - trimmed + let SerializedExceptionsAreSame actualJsonString expectedJsonString (msg: string) = + let actualJson = JObject.Parse actualJsonString + Assert.That(actualJson, Is.Not.Null) + let expectedJson = JObject.Parse expectedJsonString + Assert.That(expectedJson, Is.Not.Null) + + let fullDescPath = "Value.FullDescription" + let actualJsonToken = actualJson.SelectToken fullDescPath + Assert.That(actualJsonToken, Is.Not.Null) + let expectedJsonToken = expectedJson.SelectToken fullDescPath + Assert.That(expectedJsonToken, Is.Not.Null) + + let actualFullDesc = + actualJsonToken.ToString() + |> Sanitize + let expectedFullDesc = + expectedJsonToken.ToString() + |> Sanitize + + // old version of .NET6? (happens in stockdotnet6 CI lanes) + if actualFullDesc.Contains "of type" then + let expected = + expectedFullDesc.Replace( + "CustomFSharpException: CustomFSharpException", + "CustomFSharpException: Exception of type 'GWallet.Backend.Tests.CustomFSharpException' was thrown." + ) + Assert.That( + TrimOutsideAndInside actualFullDesc, + Is.EqualTo (TrimOutsideAndInside expected), + msg + ) else - TrimOutsideAndInside trimmed - - let SerializedExceptionsAreSame actualJsonString expectedJsonString ignoreExMessage msg = - - let actualJsonException = JObject.Parse actualJsonString - let expectedJsonException = JObject.Parse expectedJsonString - - let fullBinaryFormPath = "Value.FullBinaryForm" - let tweakStackTraces () = - - let fullBinaryFormBeginning = "AAEAAAD/////AQAA" - let stackTracePath = "Value.HumanReadableSummary.StackTrace" - let stackTraceFragment = "ExceptionMarshalling.fs" - - let tweakStackTraceAndBinaryForm (jsonEx: JObject) (assertBinaryForm: bool) = - let stackTraceJToken = jsonEx.SelectToken stackTracePath - Assert.That(stackTraceJToken, Is.Not.Null, sprintf "Path %s not found in %s" stackTracePath (jsonEx.ToString())) - let initialStackTraceJToken = stackTraceJToken.ToString() - if initialStackTraceJToken.Length > 0 then - Assert.That(initialStackTraceJToken, IsString.WhichContains stackTraceFragment, - sprintf "comparing actual '%s' with expected '%s'" actualJsonString expectedJsonString) - let endOfStackTrace = initialStackTraceJToken.Substring(initialStackTraceJToken.IndexOf stackTraceFragment) - let tweakedEndOfStackTrace = - endOfStackTrace - .Replace(":line 42", ":41 ") - .Replace(":line 41", ":41 ") - .Replace(":line 65", ":64 ") - .Replace(":line 64", ":64 ") - stackTraceJToken.Replace (tweakedEndOfStackTrace |> JToken.op_Implicit) - - let binaryFormToken = jsonEx.SelectToken fullBinaryFormPath - Assert.That(binaryFormToken, Is.Not.Null, sprintf "Path %s not found in %s" fullBinaryFormPath (jsonEx.ToString())) - let initialBinaryFormJToken = binaryFormToken.ToString() - if assertBinaryForm then - Assert.That(initialBinaryFormJToken, IsString.StartingWith fullBinaryFormBeginning) - binaryFormToken.Replace (fullBinaryFormBeginning |> JToken.op_Implicit) - - tweakStackTraceAndBinaryForm actualJsonException true - tweakStackTraceAndBinaryForm expectedJsonException false - - tweakStackTraces() - - // strangely enough, message would be different between linux_vanilla_dotnet6 and other dotnet6 configs (e.g. Windows, macOS, Linux-github) - if ignoreExMessage then - let exMessagePath = "Value.HumanReadableSummary.Message" - let actualMsgToken = actualJsonException.SelectToken exMessagePath - Assert.That(actualMsgToken, Is.Not.Null, sprintf "Path %s not found in %s" exMessagePath (actualJsonException.ToString())) - let expectedMsgToken = expectedJsonException.SelectToken exMessagePath - Assert.That(expectedMsgToken, Is.Not.Null, sprintf "Path %s not found in %s" exMessagePath (expectedJsonException.ToString())) - actualMsgToken.Replace(String.Empty |> JToken.op_Implicit) - expectedMsgToken.Replace(String.Empty |> JToken.op_Implicit) - - let actualBinaryForm = (actualJsonException.SelectToken fullBinaryFormPath).ToString() - Assert.That( - TrimOutsideAndInside(actualJsonException.ToString()), - Is.EqualTo (TrimOutsideAndInside(expectedJsonException.ToString())), - msg + actualBinaryForm - ) + Assert.That( + TrimOutsideAndInside actualFullDesc, + Is.EqualTo (TrimOutsideAndInside expectedFullDesc), + msg + ) true diff --git a/src/GWallet.Backend.Tests/data/basicException.json b/src/GWallet.Backend.Tests/data/basicException.json index 3bcfd9ce3..5b224da7b 100644 --- a/src/GWallet.Backend.Tests/data/basicException.json +++ b/src/GWallet.Backend.Tests/data/basicException.json @@ -2,12 +2,7 @@ "Version": "{version}", "TypeName": "GWallet.Backend.MarshalledException", "Value": { - "HumanReadableSummary": { - "ExceptionType": "System.Exception", - "Message": "msg", - "StackTrace": "", - "InnerException": null - }, - "FullBinaryForm": "{binaryMarshalledException}" + "DateTimeUtc": "", + "FullDescription": "System.Exception: msg" } } \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/customException.json b/src/GWallet.Backend.Tests/data/customException.json index 417fa733d..a7a3e7547 100644 --- a/src/GWallet.Backend.Tests/data/customException.json +++ b/src/GWallet.Backend.Tests/data/customException.json @@ -2,12 +2,7 @@ "Version": "{version}", "TypeName": "GWallet.Backend.MarshalledException", "Value": { - "HumanReadableSummary": { - "ExceptionType": "GWallet.Backend.Tests.CustomException", - "Message": "msg", - "StackTrace": "", - "InnerException": null - }, - "FullBinaryForm": "{binaryMarshalledException}" + "DateTimeUtc": "", + "FullDescription": "GWallet.Backend.Tests.CustomException: msg" } } \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/customFSharpException.json b/src/GWallet.Backend.Tests/data/customFSharpException.json index b4d999ff3..5c4ceaa35 100644 --- a/src/GWallet.Backend.Tests/data/customFSharpException.json +++ b/src/GWallet.Backend.Tests/data/customFSharpException.json @@ -2,12 +2,7 @@ "Version": "{version}", "TypeName": "GWallet.Backend.MarshalledException", "Value": { - "HumanReadableSummary": { - "ExceptionType": "GWallet.Backend.Tests.CustomFSharpException", - "Message": "Exception of type 'GWallet.Backend.Tests.CustomFSharpException' was thrown.", - "StackTrace": "", - "InnerException": null - }, - "FullBinaryForm": "{binaryMarshalledException}" + "DateTimeUtc": "", + "FullDescription": "GWallet.Backend.Tests.CustomFSharpException: CustomFSharpException" } } \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/customFSharpException_legacy.json b/src/GWallet.Backend.Tests/data/customFSharpException_legacy.json deleted file mode 100644 index d8020e55a..000000000 --- a/src/GWallet.Backend.Tests/data/customFSharpException_legacy.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "Version": "{version}", - "TypeName": "GWallet.Backend.MarshalledException", - "Value": { - "HumanReadableSummary": { - "ExceptionType": "GWallet.Backend.Tests.CustomFSharpException", - "Message": "CustomFSharpException", - "StackTrace": "", - "InnerException": null - }, - "FullBinaryForm": "{binaryMarshalledException}" - } -} \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/fullException.json b/src/GWallet.Backend.Tests/data/fullException.json index 9b7420098..0ebf138df 100644 --- a/src/GWallet.Backend.Tests/data/fullException.json +++ b/src/GWallet.Backend.Tests/data/fullException.json @@ -2,22 +2,7 @@ "Version": "{version}", "TypeName": "GWallet.Backend.MarshalledException", "Value": { - "HumanReadableSummary": { - "ExceptionType": "GWallet.Backend.Tests.CustomException", - "Message": "msg", - "StackTrace": " at GWallet.Backend.Tests.ExceptionMarshalling.SerializeFullException() in {prjDirAbsolutePath}/ExceptionMarshalling.fs:line 64", - "InnerException": { - "Case": "Some", - "Fields": [ - { - "ExceptionType": "GWallet.Backend.Tests.CustomException", - "Message": "innerMsg", - "StackTrace": "", - "InnerException": null - } - ] - } - }, - "FullBinaryForm": "{binaryMarshalledException}" + "DateTimeUtc": "", + "FullDescription": "GWallet.Backend.Tests.CustomException: msg\n ---> GWallet.Backend.Tests.CustomException: innerMsg\n --- End of inner exception stack trace ---\n at GWallet.Backend.Tests.ExceptionMarshalling.SerializeFullException() in {prjDirAbsolutePath}/ExceptionMarshalling.fs:line 61" } } \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/fullException_unixLegacy.json b/src/GWallet.Backend.Tests/data/fullException_unixLegacy.json deleted file mode 100644 index 96254498c..000000000 --- a/src/GWallet.Backend.Tests/data/fullException_unixLegacy.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "Version": "{version}", - "TypeName": "GWallet.Backend.MarshalledException", - "Value": { - "HumanReadableSummary": { - "ExceptionType": "GWallet.Backend.Tests.CustomException", - "Message": "msg", - "StackTrace": " at GWallet.Backend.Tests.ExceptionMarshalling.SerializeFullException () [0x00016] in {prjDirAbsolutePath}/ExceptionMarshalling.fs:line 65", - "InnerException": { - "Case": "Some", - "Fields": [ - { - "ExceptionType": "GWallet.Backend.Tests.CustomException", - "Message": "innerMsg", - "StackTrace": "", - "InnerException": null - } - ] - } - }, - "FullBinaryForm": "{binaryMarshalledException}" - } -} \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/fullException_windowsLegacy.json b/src/GWallet.Backend.Tests/data/fullException_windowsLegacy.json deleted file mode 100644 index 08d8142eb..000000000 --- a/src/GWallet.Backend.Tests/data/fullException_windowsLegacy.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "Version": "{version}", - "TypeName": "GWallet.Backend.MarshalledException", - "Value": { - "HumanReadableSummary": { - "ExceptionType": "GWallet.Backend.Tests.CustomException", - "Message": "msg", - "StackTrace": " at GWallet.Backend.Tests.ExceptionMarshalling.SerializeFullException()", - "InnerException": { - "Case": "Some", - "Fields": [ - { - "ExceptionType": "GWallet.Backend.Tests.CustomException", - "Message": "innerMsg", - "StackTrace": "", - "InnerException": null - } - ] - } - }, - "FullBinaryForm": "{binaryMarshalledException}" - } -} \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/innerException.json b/src/GWallet.Backend.Tests/data/innerException.json index b3635dbaf..2e6bd041f 100644 --- a/src/GWallet.Backend.Tests/data/innerException.json +++ b/src/GWallet.Backend.Tests/data/innerException.json @@ -2,22 +2,7 @@ "Version": "{version}", "TypeName": "GWallet.Backend.MarshalledException", "Value": { - "HumanReadableSummary": { - "ExceptionType": "System.Exception", - "Message": "msg", - "StackTrace": "", - "InnerException": { - "Case": "Some", - "Fields": [ - { - "ExceptionType": "System.Exception", - "Message": "innerMsg", - "StackTrace": "", - "InnerException": null - } - ] - } - }, - "FullBinaryForm": "{binaryMarshalledException}" + "DateTimeUtc": "", + "FullDescription": "System.Exception: msg\n ---> System.Exception: innerMsg\n --- End of inner exception stack trace ---" } } \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/realException.json b/src/GWallet.Backend.Tests/data/realException.json index 225008f8e..17fb659bc 100644 --- a/src/GWallet.Backend.Tests/data/realException.json +++ b/src/GWallet.Backend.Tests/data/realException.json @@ -2,12 +2,7 @@ "Version": "{version}", "TypeName": "GWallet.Backend.MarshalledException", "Value": { - "HumanReadableSummary": { - "ExceptionType": "System.Exception", - "Message": "msg", - "StackTrace": " at GWallet.Backend.Tests.ExceptionMarshalling.SerializeRealException() in {prjDirAbsolutePath}/ExceptionMarshalling.fs:line 41", - "InnerException": null - }, - "FullBinaryForm": "{binaryMarshalledException}" + "DateTimeUtc": "", + "FullDescription": "System.Exception: msg\n at GWallet.Backend.Tests.ExceptionMarshalling.SerializeRealException() in {prjDirAbsolutePath}/ExceptionMarshalling.fs:line 38" } } \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/realException_unixLegacy.json b/src/GWallet.Backend.Tests/data/realException_unixLegacy.json deleted file mode 100644 index b5a98ca99..000000000 --- a/src/GWallet.Backend.Tests/data/realException_unixLegacy.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "Version": "{version}", - "TypeName": "GWallet.Backend.MarshalledException", - "Value": { - "HumanReadableSummary": { - "ExceptionType": "System.Exception", - "Message": "msg", - "StackTrace": " at GWallet.Backend.Tests.ExceptionMarshalling.can serialize real exceptions () [0x0000c] in {prjDirAbsolutePath}/ExceptionMarshalling.fs:line 42", - "InnerException": null - }, - "FullBinaryForm": "{binaryMarshalledException}" - } -} \ No newline at end of file diff --git a/src/GWallet.Backend.Tests/data/realException_windowsLegacy.json b/src/GWallet.Backend.Tests/data/realException_windowsLegacy.json deleted file mode 100644 index 034b2c3dc..000000000 --- a/src/GWallet.Backend.Tests/data/realException_windowsLegacy.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "Version": "{version}", - "TypeName": "GWallet.Backend.MarshalledException", - "Value": { - "HumanReadableSummary": { - "ExceptionType": "System.Exception", - "Message": "msg", - "StackTrace": " at GWallet.Backend.Tests.ExceptionMarshalling.SerializeRealException()", - "InnerException": null - }, - "FullBinaryForm": "{binaryMarshalledException}" - } -} \ No newline at end of file diff --git a/src/GWallet.Backend/Infrastructure.fs b/src/GWallet.Backend/Infrastructure.fs index cc6dc3527..852d4e338 100644 --- a/src/GWallet.Backend/Infrastructure.fs +++ b/src/GWallet.Backend/Infrastructure.fs @@ -111,6 +111,13 @@ module Infrastructure = ReportInner sentryEvent #endif + let internal ReportRawException (marshalledEx: MarshalledException): bool = + let exReport = + SPrintF2 "Recovered exception details from off-line crash at %s UTC: %s" + (marshalledEx.DateTimeUtc.ToString()) + marshalledEx.FullDescription + ReportMessage exReport ErrorLevel.Fatal + let internal ReportError (errorMessage: string): bool = ReportMessage errorMessage ErrorLevel.Error @@ -177,10 +184,33 @@ module Infrastructure = let private ReportAllPastExceptions() = for loggedEx in ((GetTelemetryDir false).EnumerateFiles()) do let serializedException = File.ReadAllText loggedEx.FullName - let deserializedException = Marshalling.Deserialize serializedException - let reported = Report deserializedException ErrorLevel.Fatal - if reported then - loggedEx.Delete () + + let exType = Marshalling.ExtractType serializedException + + let maybeEx = + + // old format, we could try to map and report but meh, it's too + // much work, let's just delete and next time it happens it will + // be serialized properly this time around + if typeof.IsAssignableFrom exType then + ReportWarningMessage "Serialized exception discarded due to old format being used" + |> ignore + None + elif typeof = exType then + Marshalling.Deserialize serializedException + |> Some + else + failwith + <| SPrintF2 "Instance with unexpected type (%s) found in telemetry dir: %s" + exType.FullName serializedException + + match maybeEx with + | None -> + loggedEx.Delete() + | Some deserializedException -> + let reported = ReportRawException deserializedException + if reported then + loggedEx.Delete() let public SetupExceptionHook () = #if !DEBUG diff --git a/src/GWallet.Backend/Marshalling.fs b/src/GWallet.Backend/Marshalling.fs index 47ab70c10..264f0fafd 100644 --- a/src/GWallet.Backend/Marshalling.fs +++ b/src/GWallet.Backend/Marshalling.fs @@ -13,48 +13,17 @@ open Newtonsoft.Json.Serialization open GWallet.Backend.FSharpUtil.UwpHacks -type ExceptionDetails = - { - ExceptionType: string - Message: string - StackTrace: string - InnerException: Option - } - type MarshalledException = { - HumanReadableSummary: ExceptionDetails - FullBinaryForm: string - } - static member private ExtractBasicDetailsFromException (ex: Exception) = - let stackTrace = - if isNull ex.StackTrace then - String.Empty - else - ex.StackTrace - let stub = - { - ExceptionType = ex.GetType().FullName - Message = ex.Message - StackTrace = stackTrace - InnerException = None - } - - match ex.InnerException with - | null -> stub - | someNonNullInnerException -> - let innerExceptionDetails = - MarshalledException.ExtractBasicDetailsFromException someNonNullInnerException - - { - stub with - InnerException = Some innerExceptionDetails - } + DateTimeUtc: DateTime + // from ex.ToString(), which includes ex's type name, ex.Message, ex.InnerExceptions, etc + FullDescription: string + } static member Create (ex: Exception) = { - HumanReadableSummary = MarshalledException.ExtractBasicDetailsFromException ex - FullBinaryForm = BinaryMarshalling.SerializeToString ex + DateTimeUtc = DateTime.UtcNow + FullDescription = ex.ToString() } type DeserializationException = @@ -238,8 +207,7 @@ module Marshalling = let Deserialize<'T>(json: string): 'T = match typeof<'T> with | theType when typeof.IsAssignableFrom theType -> - let marshalledException: MarshalledException = DeserializeCustom(json, DefaultSettings) - BinaryMarshalling.DeserializeFromString marshalledException.FullBinaryForm :?> 'T + failwith "Binary (de)serialization of exceptions is not supported anymore" | _ -> DeserializeCustom(json, DefaultSettings) @@ -261,18 +229,6 @@ module Marshalling = | :? Exception as ex -> let exToSerialize = MarshalledException.Create ex let serializedEx = SerializeCustom(exToSerialize, DefaultSettings, DefaultFormatting) - - try - let _deserializedEx: 'T = Deserialize serializedEx - () - with - | ex -> - raise - <| MarshallingCompatibilityException ( - SPrintF1 - "Exception type '%s' could not be serialized. Maybe it lacks the required '(info: SerializationInfo, context: StreamingContext)' constructor?" - typeof<'T>.FullName, ex) - serializedEx | _ -> SerializeCustom(value, DefaultSettings, DefaultFormatting)