Skip to content

Commit

Permalink
Backend(,Tests): stop using binary serialization
Browse files Browse the repository at this point in the history
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 #240
Closes https://gitlab.com/nblockchain/geewallet/-/issues/174

[1] 403d5c7
[2] 1f7b3b7
[3] https://twitter.com/SitnikAdam/status/1746874459640811575
[4] https://github.com/getsentry/sentry-dotnet
[5] #252
  • Loading branch information
knocte committed Feb 1, 2024
1 parent 417b4d1 commit 83d8fcf
Show file tree
Hide file tree
Showing 17 changed files with 206 additions and 380 deletions.
180 changes: 94 additions & 86 deletions src/GWallet.Backend.Tests/ExceptionMarshalling.fs
Original file line number Diff line number Diff line change
@@ -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) =
Expand All @@ -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) =
Expand Down Expand Up @@ -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

[<Test>]
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)

[<Test>]
member __.``can deserialize basic exceptions``() =
Expand All @@ -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<Exception>())
Assert.That(ex.Message, Is.EqualTo "msg")
Assert.That(ex.InnerException, Is.Null)
Assert.That(ex.StackTrace, Is.Null)
Assert.That(ex, Is.InstanceOf<MarshalledException>())
Assert.That(ex.FullDescription.Trim().Length, Is.GreaterThan 0)
Assert.That(
MarshallingData.Sanitize ex.FullDescription,
Is.EqualTo (MarshallingData.Sanitize "System.Exception: msg")
)

[<Test>]
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

[<Test>]
Expand All @@ -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<Exception>())
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<MarshalledException>())
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

[<Test>]
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)

[<Test>]
member __.``can deserialize inner exceptions``() =
Expand All @@ -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<Exception>())
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<Exception>())
Assert.That (ex.InnerException.Message, Is.EqualTo "innerMsg")
Assert.That (ex.InnerException.StackTrace, Is.Null)
Assert.That (ex, Is.InstanceOf<MarshalledException>())
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 ---")
)

[<Test>]
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)

[<Test>]
member __.``serializing custom exception not prepared for binary serialization, throws``() =
let exToSerialize = CustomExceptionWithoutSerializationCtor "msg"
let ex: MarshallingCompatibilityException =
Assert.Throws(fun _ -> Marshalling.Serialize exToSerialize |> ignore<string>)
Assert.That(ex, Is.TypeOf<MarshallingCompatibilityException>())
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)

[<Test>]
member __.``can deserialize custom exceptions``() =
Expand All @@ -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<CustomException>())
Assert.That(ex.Message, Is.EqualTo "msg")
Assert.That(ex.InnerException, Is.Null)
Assert.That(ex.StackTrace, Is.Null)
Assert.That(ex, Is.InstanceOf<MarshalledException>())
Assert.That(
MarshallingData.Sanitize ex.FullDescription,
Is.EqualTo (MarshallingData.Sanitize "GWallet.Backend.Tests.CustomException: msg")
)

[<Test>]
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

[<Test>]
member __.``can deserialize F# custom exceptions``() =
Expand All @@ -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<CustomFSharpException>())
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<MarshalledException>())
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

[<Test>]
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

[<Test>]
Expand All @@ -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<CustomException> ())
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<MarshalledException> ())
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


5 changes: 0 additions & 5 deletions src/GWallet.Backend.Tests/GWallet.Backend.Tests-legacy.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,10 @@
<EmbeddedResource Include="data\unsignedAndFormattedEtherTransaction.json" />
<EmbeddedResource Include="data\basicException.json" />
<EmbeddedResource Include="data\realException.json" />
<EmbeddedResource Include="data\realException_unixLegacy.json" />
<EmbeddedResource Include="data\realException_windowsLegacy.json" />
<EmbeddedResource Include="data\innerException.json" />
<EmbeddedResource Include="data\customException.json" />
<EmbeddedResource Include="data\customFSharpException.json" />
<EmbeddedResource Include="data\customFSharpException_legacy.json" />
<EmbeddedResource Include="data\fullException.json" />
<EmbeddedResource Include="data\fullException_unixLegacy.json" />
<EmbeddedResource Include="data\fullException_windowsLegacy.json" />
<Compile Include="ElectrumIntegrationTests.fs" />
<Compile Include="WarpWallet.fs" />
<Compile Include="CompoundBalanceCaching.fs" />
Expand Down
5 changes: 0 additions & 5 deletions src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,13 @@
<ItemGroup>
<EmbeddedResource Include="data\basicException.json" />
<EmbeddedResource Include="data\customFSharpException.json" />
<EmbeddedResource Include="data\customFSharpException_legacy.json" />
<EmbeddedResource Include="data\realException.json" />
<EmbeddedResource Include="data\realException_unixLegacy.json" />
<EmbeddedResource Include="data\realException_windowsLegacy.json" />
<EmbeddedResource Include="data\signedAndFormattedEtherTransaction.json" />
<EmbeddedResource Include="data\customException.json" />
<EmbeddedResource Include="data\unsignedAndFormattedSaiTransaction.json" />
<EmbeddedResource Include="data\unsignedAndFormattedBtcTransaction.json" />
<EmbeddedResource Include="data\unsignedAndFormattedEtherTransaction.json" />
<EmbeddedResource Include="data\fullException.json" />
<EmbeddedResource Include="data\fullException_unixLegacy.json" />
<EmbeddedResource Include="data\fullException_windowsLegacy.json" />
<EmbeddedResource Include="data\signedAndFormattedBtcTransaction.json" />
<EmbeddedResource Include="data\signedAndFormattedSaiTransaction.json" />
<EmbeddedResource Include="data\innerException.json" />
Expand Down
Loading

0 comments on commit 83d8fcf

Please sign in to comment.