Skip to content

Commit f6d5061

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 417b4d1 commit f6d5061

17 files changed

+185
-374
lines changed
Lines changed: 73 additions & 80 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) =
@@ -68,17 +65,17 @@ type ExceptionMarshalling () =
6865
cex
6966
Marshalling.Serialize ex
7067

71-
let msg = "Exceptions didn't match. Full binary form was "
68+
let msg = "Expected Exception.ToString() differs from actual"
7269
#if LEGACY_FRAMEWORK
73-
let legacyMsg = "(Legacy)Exceptions didn't match. Full binary form was "
70+
let legacyIgnoreMsg = "Mono or old .NETFramework might vary slightly; there's no need to really do any regression testing here"
7471
#endif
7572

7673
[<Test>]
7774
member __.``can serialize basic exceptions``() =
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,31 +87,23 @@ 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(
94+
MarshallingData.Sanitize ex.FullDescription,
95+
Is.EqualTo (MarshallingData.Sanitize "System.Exception: msg")
96+
)
9997

10098
[<Test>]
10199
member __.``can serialize real exceptions``() =
102100
let json = SerializeRealException ()
103101
Assert.That(json, Is.Not.Null)
104102
Assert.That(json, Is.Not.Empty)
105103
#if !LEGACY_FRAMEWORK
106-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionExampleInJson false msg)
104+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionExampleInJson msg)
107105
#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)
106+
Assert.Ignore legacyIgnoreMsg
118107
#endif
119108

120109
[<Test>]
@@ -127,20 +116,24 @@ type ExceptionMarshalling () =
127116
Assert.Inconclusive "Fix the serialization test first"
128117
failwith "unreachable"
129118

130-
let ex: Exception = Marshalling.Deserialize realExceptionSerialized
119+
let ex: MarshalledException = Marshalling.Deserialize realExceptionSerialized
131120
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)
121+
Assert.That(ex, Is.InstanceOf<MarshalledException>())
122+
let expected =
123+
sprintf
124+
"System.Exception: msg at GWallet.Backend.Tests.ExceptionMarshalling.SerializeRealException() in %s/ExceptionMarshalling.fs:line 38"
125+
MarshallingData.ThisProjPath
126+
Assert.That(
127+
MarshallingData.Sanitize ex.FullDescription,
128+
Is.EqualTo (MarshallingData.Sanitize expected)
129+
)
137130

138131
[<Test>]
139132
member __.``can serialize inner exceptions``() =
140133
let json = SerializeInnerException ()
141134
Assert.That(json, Is.Not.Null)
142135
Assert.That(json, Is.Not.Empty)
143-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.InnerExceptionExampleInJson false msg)
136+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.InnerExceptionExampleInJson msg)
144137

145138
[<Test>]
146139
member __.``can deserialize inner exceptions``() =
@@ -152,31 +145,27 @@ type ExceptionMarshalling () =
152145
Assert.Inconclusive "Fix the serialization test first"
153146
failwith "unreachable"
154147

155-
let ex: Exception = Marshalling.Deserialize innerExceptionSerialized
148+
let ex: MarshalledException = Marshalling.Deserialize innerExceptionSerialized
156149
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)
150+
Assert.That (ex, Is.InstanceOf<MarshalledException>())
151+
Assert.That (
152+
MarshallingData.Sanitize ex.FullDescription,
153+
Is.EqualTo (MarshallingData.Sanitize "System.Exception: msg ---> System.Exception: innerMsg --- End of inner exception stack trace ---")
154+
)
165155

166156
[<Test>]
167157
member __.``can serialize custom exceptions``() =
168158
let json = SerializeCustomException ()
169159
Assert.That(json, Is.Not.Null)
170160
Assert.That(json, Is.Not.Empty)
171-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomExceptionExampleInJson false msg)
161+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomExceptionExampleInJson msg)
172162

173163
[<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")
164+
member __.``serializing custom exception without inner ex ctor does not crash``() =
165+
let exToSerialize = CustomExceptionWithoutInnerExceptionCtor "msg"
166+
let serializedEx = (Marshalling.Serialize exToSerialize).Trim()
167+
Assert.That(serializedEx, Is.Not.Null)
168+
Assert.That(serializedEx.Length, Is.GreaterThan 0)
180169

181170
[<Test>]
182171
member __.``can deserialize custom exceptions``() =
@@ -188,21 +177,24 @@ type ExceptionMarshalling () =
188177
Assert.Inconclusive "Fix the serialization test first"
189178
failwith "unreachable"
190179

191-
let ex: Exception = Marshalling.Deserialize customExceptionSerialized
180+
let ex: MarshalledException = Marshalling.Deserialize customExceptionSerialized
192181
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)
182+
Assert.That(ex, Is.InstanceOf<MarshalledException>())
183+
Assert.That(
184+
MarshallingData.Sanitize ex.FullDescription,
185+
Is.EqualTo (MarshallingData.Sanitize "GWallet.Backend.Tests.CustomException: msg")
186+
)
197187

198188
[<Test>]
199189
member __.``can serialize F# custom exceptions``() =
200190
let json = SerializeCustomFSharpException ()
201191
Assert.That(json, Is.Not.Null)
202192
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)
193+
#if !LEGACY_FRAMEWORK
194+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomFSharpExceptionExampleInJson msg)
195+
#else
196+
Assert.Ignore legacyIgnoreMsg
197+
#endif
206198

207199
[<Test>]
208200
member __.``can deserialize F# custom exceptions``() =
@@ -214,36 +206,33 @@ type ExceptionMarshalling () =
214206
Assert.Inconclusive "Fix the serialization test first"
215207
failwith "unreachable"
216208

217-
let ex: Exception = Marshalling.Deserialize customExceptionSerialized
209+
let ex: MarshalledException = Marshalling.Deserialize customExceptionSerialized
218210
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)
211+
Assert.That(ex, Is.InstanceOf<MarshalledException>())
212+
213+
if ex.FullDescription.Contains "of type" then
214+
// old version of .NET6? (happens in stockdotnet6 CI lanes)
215+
Assert.That(
216+
MarshallingData.Sanitize ex.FullDescription,
217+
Is.EqualTo (MarshallingData.Sanitize "GWallet.Backend.Tests.CustomFSharpException: Exception of type 'GWallet.Backend.Tests.CustomFSharpException' was thrown.")
218+
)
219+
else
220+
Assert.That(
221+
MarshallingData.Sanitize ex.FullDescription,
222+
Is.EqualTo (MarshallingData.Sanitize "GWallet.Backend.Tests.CustomFSharpException: CustomFSharpException")
223+
)
224224

225-
// TODO: test marshalling custom exceptions with custom properties/fields, and custom F# exception with subtypes
226225

227226
[<Test>]
228227
member __.``can serialize full exceptions (all previous features combined)``() =
229228
let json = SerializeFullException ()
230229

231230
Assert.That(json, Is.Not.Null)
232231
Assert.That(json, Is.Not.Empty)
233-
234232
#if !LEGACY_FRAMEWORK
235-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionExampleInJson false msg)
233+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionExampleInJson msg)
236234
#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)
235+
Assert.Ignore legacyIgnoreMsg
247236
#endif
248237

249238
[<Test>]
@@ -256,12 +245,16 @@ type ExceptionMarshalling () =
256245
Assert.Inconclusive "Fix the serialization test first"
257246
failwith "unreachable"
258247

259-
let ex: Exception = Marshalling.Deserialize fullExceptionSerialized
248+
let ex: MarshalledException = Marshalling.Deserialize fullExceptionSerialized
260249
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)
250+
Assert.That(ex, Is.InstanceOf<MarshalledException> ())
251+
Assert.That(
252+
MarshallingData.Sanitize ex.FullDescription,
253+
Is.EqualTo (
254+
MarshallingData.Sanitize
255+
<| sprintf
256+
"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"
257+
MarshallingData.ThisProjPath
258+
)
259+
)
267260

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)