Skip to content

Commit e06a0cb

Browse files
committed
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 #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
1 parent 9c7c1f4 commit e06a0cb

17 files changed

+114
-368
lines changed
Lines changed: 40 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
namespace GWallet.Backend.Tests
22

33
open System
4-
open System.Runtime.Serialization
54

65
open NUnit.Framework
76

87
open GWallet.Backend
98

109

11-
type CustomExceptionWithoutSerializationCtor =
10+
type CustomExceptionWithoutInnerExceptionCtor =
1211
inherit Exception
1312

1413
new(message) =
@@ -17,8 +16,6 @@ type CustomExceptionWithoutSerializationCtor =
1716
type CustomException =
1817
inherit Exception
1918

20-
new(info: SerializationInfo, context: StreamingContext) =
21-
{ inherit Exception(info, context) }
2219
new(message: string, innerException: CustomException) =
2320
{ inherit Exception(message, innerException) }
2421
new(message) =
@@ -78,7 +75,7 @@ type ExceptionMarshalling () =
7875
let json = SerializeBasicException ()
7976
Assert.That(json, Is.Not.Null)
8077
Assert.That(json, Is.Not.Empty)
81-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.BasicExceptionExampleInJson false msg)
78+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.BasicExceptionExampleInJson msg)
8279

8380
[<Test>]
8481
member __.``can deserialize basic exceptions``() =
@@ -90,32 +87,17 @@ type ExceptionMarshalling () =
9087
Assert.Inconclusive "Fix the serialization test first"
9188
failwith "unreachable"
9289

93-
let ex: Exception = Marshalling.Deserialize basicExSerialized
90+
let ex: MarshalledException = Marshalling.Deserialize basicExSerialized
9491
Assert.That(ex, Is.Not.Null)
95-
Assert.That(ex, Is.InstanceOf<Exception>())
96-
Assert.That(ex.Message, Is.EqualTo "msg")
97-
Assert.That(ex.InnerException, Is.Null)
98-
Assert.That(ex.StackTrace, Is.Null)
92+
Assert.That(ex, Is.InstanceOf<MarshalledException>())
93+
Assert.That(ex.FullDescription, Is.EqualTo "System.Exception: msg")
9994

10095
[<Test>]
10196
member __.``can serialize real exceptions``() =
10297
let json = SerializeRealException ()
10398
Assert.That(json, Is.Not.Null)
10499
Assert.That(json, Is.Not.Empty)
105-
#if !LEGACY_FRAMEWORK
106-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionExampleInJson false msg)
107-
#else
108-
if Config.IsWindowsPlatform () then
109-
let serializedExceptionsAreSame =
110-
try
111-
MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionExampleInJson false msg
112-
with
113-
| :? AssertionException ->
114-
MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionWindowsLegacyExampleInJson false legacyMsg
115-
Assert.That serializedExceptionsAreSame
116-
else
117-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionUnixLegacyExampleInJson false legacyMsg)
118-
#endif
100+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionExampleInJson msg)
119101

120102
[<Test>]
121103
member __.``can deserialize real exceptions``() =
@@ -127,20 +109,21 @@ type ExceptionMarshalling () =
127109
Assert.Inconclusive "Fix the serialization test first"
128110
failwith "unreachable"
129111

130-
let ex: Exception = Marshalling.Deserialize realExceptionSerialized
112+
let ex: MarshalledException = Marshalling.Deserialize realExceptionSerialized
131113
Assert.That(ex, Is.Not.Null)
132-
Assert.That(ex, Is.InstanceOf<Exception>())
133-
Assert.That(ex.Message, Is.EqualTo "msg")
134-
Assert.That(ex.InnerException, Is.Null)
135-
Assert.That(ex.StackTrace, Is.Not.Null)
136-
Assert.That(ex.StackTrace, Is.Not.Empty)
114+
Assert.That(ex, Is.InstanceOf<MarshalledException>())
115+
let expected =
116+
sprintf
117+
"System.Exception: msg\n at GWallet.Backend.Tests.ExceptionMarshalling.SerializeRealException() in %s/ExceptionMarshalling.fs:line 38"
118+
MarshallingData.ThisProjPath
119+
Assert.That(ex.FullDescription, Is.EqualTo expected)
137120

138121
[<Test>]
139122
member __.``can serialize inner exceptions``() =
140123
let json = SerializeInnerException ()
141124
Assert.That(json, Is.Not.Null)
142125
Assert.That(json, Is.Not.Empty)
143-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.InnerExceptionExampleInJson false msg)
126+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.InnerExceptionExampleInJson msg)
144127

145128
[<Test>]
146129
member __.``can deserialize inner exceptions``() =
@@ -152,31 +135,24 @@ type ExceptionMarshalling () =
152135
Assert.Inconclusive "Fix the serialization test first"
153136
failwith "unreachable"
154137

155-
let ex: Exception = Marshalling.Deserialize innerExceptionSerialized
138+
let ex: MarshalledException = Marshalling.Deserialize innerExceptionSerialized
156139
Assert.That (ex, Is.Not.Null)
157-
Assert.That (ex, Is.InstanceOf<Exception>())
158-
Assert.That (ex.Message, Is.EqualTo "msg")
159-
Assert.That (ex.StackTrace, Is.Null)
160-
Assert.That (ex.InnerException, Is.Not.Null)
161-
162-
Assert.That (ex.InnerException, Is.InstanceOf<Exception>())
163-
Assert.That (ex.InnerException.Message, Is.EqualTo "innerMsg")
164-
Assert.That (ex.InnerException.StackTrace, Is.Null)
140+
Assert.That (ex, Is.InstanceOf<MarshalledException>())
141+
Assert.That (ex.FullDescription, Is.EqualTo "System.Exception: msg\n ---> System.Exception: innerMsg\n --- End of inner exception stack trace ---")
165142

166143
[<Test>]
167144
member __.``can serialize custom exceptions``() =
168145
let json = SerializeCustomException ()
169146
Assert.That(json, Is.Not.Null)
170147
Assert.That(json, Is.Not.Empty)
171-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomExceptionExampleInJson false msg)
148+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomExceptionExampleInJson msg)
172149

173150
[<Test>]
174-
member __.``serializing custom exception not prepared for binary serialization, throws``() =
175-
let exToSerialize = CustomExceptionWithoutSerializationCtor "msg"
176-
let ex: MarshallingCompatibilityException =
177-
Assert.Throws(fun _ -> Marshalling.Serialize exToSerialize |> ignore<string>)
178-
Assert.That(ex, Is.TypeOf<MarshallingCompatibilityException>())
179-
Assert.That(ex.Message, IsString.WhichContains "GWallet.Backend.Tests.CustomExceptionWithoutSerializationCtor")
151+
member __.``serializing custom exception without inner ex ctor does not crash``() =
152+
let exToSerialize = CustomExceptionWithoutInnerExceptionCtor "msg"
153+
let serializedEx = (Marshalling.Serialize exToSerialize).Trim()
154+
Assert.That(serializedEx, Is.Not.Null)
155+
Assert.That(serializedEx.Length, Is.GreaterThan 0)
180156

181157
[<Test>]
182158
member __.``can deserialize custom exceptions``() =
@@ -188,21 +164,17 @@ type ExceptionMarshalling () =
188164
Assert.Inconclusive "Fix the serialization test first"
189165
failwith "unreachable"
190166

191-
let ex: Exception = Marshalling.Deserialize customExceptionSerialized
167+
let ex: MarshalledException = Marshalling.Deserialize customExceptionSerialized
192168
Assert.That(ex, Is.Not.Null)
193-
Assert.That(ex, Is.InstanceOf<CustomException>())
194-
Assert.That(ex.Message, Is.EqualTo "msg")
195-
Assert.That(ex.InnerException, Is.Null)
196-
Assert.That(ex.StackTrace, Is.Null)
169+
Assert.That(ex, Is.InstanceOf<MarshalledException>())
170+
Assert.That(ex.FullDescription, Is.EqualTo "GWallet.Backend.Tests.CustomException: msg")
197171

198172
[<Test>]
199173
member __.``can serialize F# custom exceptions``() =
200174
let json = SerializeCustomFSharpException ()
201175
Assert.That(json, Is.Not.Null)
202176
Assert.That(json, Is.Not.Empty)
203-
204-
// strangely enough, message would be different between linux_vanilla_dotnet6 and other dotnet6 configs (e.g. Windows, macOS, Linux-github)
205-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomFSharpExceptionExampleInJson true msg)
177+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomFSharpExceptionExampleInJson msg)
206178

207179
[<Test>]
208180
member __.``can deserialize F# custom exceptions``() =
@@ -214,37 +186,18 @@ type ExceptionMarshalling () =
214186
Assert.Inconclusive "Fix the serialization test first"
215187
failwith "unreachable"
216188

217-
let ex: Exception = Marshalling.Deserialize customExceptionSerialized
189+
let ex: MarshalledException = Marshalling.Deserialize customExceptionSerialized
218190
Assert.That(ex, Is.Not.Null)
219-
Assert.That(ex, Is.InstanceOf<CustomFSharpException>())
220-
Assert.That(ex.Message, Is.Not.Null)
221-
Assert.That(ex.Message, Is.Not.Empty)
222-
Assert.That(ex.InnerException, Is.Null)
223-
Assert.That(ex.StackTrace, Is.Null)
224-
225-
// TODO: test marshalling custom exceptions with custom properties/fields, and custom F# exception with subtypes
191+
Assert.That(ex, Is.InstanceOf<MarshalledException>())
192+
Assert.That(ex.FullDescription, Is.EqualTo "GWallet.Backend.Tests.CustomFSharpException: CustomFSharpException")
226193

227194
[<Test>]
228195
member __.``can serialize full exceptions (all previous features combined)``() =
229196
let json = SerializeFullException ()
230197

231198
Assert.That(json, Is.Not.Null)
232199
Assert.That(json, Is.Not.Empty)
233-
234-
#if !LEGACY_FRAMEWORK
235-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionExampleInJson false msg)
236-
#else
237-
if Config.IsWindowsPlatform () then
238-
let serializedExceptionsAreSame =
239-
try
240-
MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionExampleInJson false msg
241-
with
242-
| :? AssertionException ->
243-
MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionWindowsLegacyExampleInJson false legacyMsg
244-
Assert.That serializedExceptionsAreSame
245-
else
246-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionUnixLegacyExampleInJson false legacyMsg)
247-
#endif
200+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionExampleInJson msg)
248201

249202
[<Test>]
250203
member __.``can deserialize full exceptions (all previous features combined)``() =
@@ -256,12 +209,14 @@ type ExceptionMarshalling () =
256209
Assert.Inconclusive "Fix the serialization test first"
257210
failwith "unreachable"
258211

259-
let ex: Exception = Marshalling.Deserialize fullExceptionSerialized
212+
let ex: MarshalledException = Marshalling.Deserialize fullExceptionSerialized
260213
Assert.That(ex, Is.Not.Null)
261-
Assert.That(ex, Is.InstanceOf<CustomException> ())
262-
Assert.That(ex.Message, Is.Not.Null)
263-
Assert.That(ex.Message, Is.Not.Empty)
264-
Assert.That(ex.InnerException, Is.Not.Null)
265-
Assert.That(ex.StackTrace, Is.Not.Null)
266-
Assert.That(ex.StackTrace, Is.Not.Empty)
214+
Assert.That(ex, Is.InstanceOf<MarshalledException> ())
215+
Assert.That(
216+
ex.FullDescription,
217+
Is.EqualTo
218+
<| sprintf
219+
"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 %s/ExceptionMarshalling.fs:line 61"
220+
MarshallingData.ThisProjPath
221+
)
267222

src/GWallet.Backend.Tests/GWallet.Backend.Tests-legacy.fsproj

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,10 @@
7575
<EmbeddedResource Include="data\unsignedAndFormattedEtherTransaction.json" />
7676
<EmbeddedResource Include="data\basicException.json" />
7777
<EmbeddedResource Include="data\realException.json" />
78-
<EmbeddedResource Include="data\realException_unixLegacy.json" />
79-
<EmbeddedResource Include="data\realException_windowsLegacy.json" />
8078
<EmbeddedResource Include="data\innerException.json" />
8179
<EmbeddedResource Include="data\customException.json" />
8280
<EmbeddedResource Include="data\customFSharpException.json" />
83-
<EmbeddedResource Include="data\customFSharpException_legacy.json" />
8481
<EmbeddedResource Include="data\fullException.json" />
85-
<EmbeddedResource Include="data\fullException_unixLegacy.json" />
86-
<EmbeddedResource Include="data\fullException_windowsLegacy.json" />
8782
<Compile Include="ElectrumIntegrationTests.fs" />
8883
<Compile Include="WarpWallet.fs" />
8984
<Compile Include="CompoundBalanceCaching.fs" />

src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,13 @@
3939
<ItemGroup>
4040
<EmbeddedResource Include="data\basicException.json" />
4141
<EmbeddedResource Include="data\customFSharpException.json" />
42-
<EmbeddedResource Include="data\customFSharpException_legacy.json" />
4342
<EmbeddedResource Include="data\realException.json" />
44-
<EmbeddedResource Include="data\realException_unixLegacy.json" />
45-
<EmbeddedResource Include="data\realException_windowsLegacy.json" />
4643
<EmbeddedResource Include="data\signedAndFormattedEtherTransaction.json" />
4744
<EmbeddedResource Include="data\customException.json" />
4845
<EmbeddedResource Include="data\unsignedAndFormattedSaiTransaction.json" />
4946
<EmbeddedResource Include="data\unsignedAndFormattedBtcTransaction.json" />
5047
<EmbeddedResource Include="data\unsignedAndFormattedEtherTransaction.json" />
5148
<EmbeddedResource Include="data\fullException.json" />
52-
<EmbeddedResource Include="data\fullException_unixLegacy.json" />
53-
<EmbeddedResource Include="data\fullException_windowsLegacy.json" />
5449
<EmbeddedResource Include="data\signedAndFormattedBtcTransaction.json" />
5550
<EmbeddedResource Include="data\signedAndFormattedSaiTransaction.json" />
5651
<EmbeddedResource Include="data\innerException.json" />

0 commit comments

Comments
 (0)