Skip to content

Commit 55c4cef

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 55c4cef

15 files changed

+79
-300
lines changed

src/GWallet.Backend.Tests/ExceptionMarshalling.fs

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ open NUnit.Framework
88
open GWallet.Backend
99

1010

11-
type CustomExceptionWithoutSerializationCtor =
11+
type CustomExceptionWithoutInnerExceptionCtor =
1212
inherit Exception
1313

1414
new(message) =
@@ -17,8 +17,6 @@ type CustomExceptionWithoutSerializationCtor =
1717
type CustomException =
1818
inherit Exception
1919

20-
new(info: SerializationInfo, context: StreamingContext) =
21-
{ inherit Exception(info, context) }
2220
new(message: string, innerException: CustomException) =
2321
{ inherit Exception(message, innerException) }
2422
new(message) =
@@ -78,7 +76,7 @@ type ExceptionMarshalling () =
7876
let json = SerializeBasicException ()
7977
Assert.That(json, Is.Not.Null)
8078
Assert.That(json, Is.Not.Empty)
81-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.BasicExceptionExampleInJson false msg)
79+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.BasicExceptionExampleInJson msg)
8280

8381
[<Test>]
8482
member __.``can deserialize basic exceptions``() =
@@ -102,20 +100,7 @@ type ExceptionMarshalling () =
102100
let json = SerializeRealException ()
103101
Assert.That(json, Is.Not.Null)
104102
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
103+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.RealExceptionExampleInJson msg)
119104

120105
[<Test>]
121106
member __.``can deserialize real exceptions``() =
@@ -140,7 +125,7 @@ type ExceptionMarshalling () =
140125
let json = SerializeInnerException ()
141126
Assert.That(json, Is.Not.Null)
142127
Assert.That(json, Is.Not.Empty)
143-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.InnerExceptionExampleInJson false msg)
128+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.InnerExceptionExampleInJson msg)
144129

145130
[<Test>]
146131
member __.``can deserialize inner exceptions``() =
@@ -168,15 +153,15 @@ type ExceptionMarshalling () =
168153
let json = SerializeCustomException ()
169154
Assert.That(json, Is.Not.Null)
170155
Assert.That(json, Is.Not.Empty)
171-
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomExceptionExampleInJson false msg)
156+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomExceptionExampleInJson msg)
172157

173158
[<Test>]
174159
member __.``serializing custom exception not prepared for binary serialization, throws``() =
175-
let exToSerialize = CustomExceptionWithoutSerializationCtor "msg"
160+
let exToSerialize = CustomExceptionWithoutInnerExceptionCtor "msg"
176161
let ex: MarshallingCompatibilityException =
177162
Assert.Throws(fun _ -> Marshalling.Serialize exToSerialize |> ignore<string>)
178163
Assert.That(ex, Is.TypeOf<MarshallingCompatibilityException>())
179-
Assert.That(ex.Message, IsString.WhichContains "GWallet.Backend.Tests.CustomExceptionWithoutSerializationCtor")
164+
Assert.That(ex.Message, IsString.WhichContains "GWallet.Backend.Tests.CustomExceptionWithoutInnerExceptionCtor")
180165

181166
[<Test>]
182167
member __.``can deserialize custom exceptions``() =
@@ -200,9 +185,7 @@ type ExceptionMarshalling () =
200185
let json = SerializeCustomFSharpException ()
201186
Assert.That(json, Is.Not.Null)
202187
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)
188+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.CustomFSharpExceptionExampleInJson msg)
206189

207190
[<Test>]
208191
member __.``can deserialize F# custom exceptions``() =
@@ -230,21 +213,7 @@ type ExceptionMarshalling () =
230213

231214
Assert.That(json, Is.Not.Null)
232215
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
216+
Assert.That(MarshallingData.SerializedExceptionsAreSame json MarshallingData.FullExceptionExampleInJson msg)
248217

249218
[<Test>]
250219
member __.``can deserialize full exceptions (all previous features combined)``() =

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" />

src/GWallet.Backend.Tests/MarshallingData.fs

Lines changed: 19 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ module MarshallingData =
1616
let private executingAssembly = Assembly.GetExecutingAssembly()
1717
let private version = VersionHelper.CURRENT_VERSION
1818
let private binPath = executingAssembly.Location |> FileInfo
19-
let private prjPath = Path.Combine(binPath.Directory.FullName, "..") |> DirectoryInfo
19+
let private prjPath = Path.Combine(binPath.Directory.FullName, "..", "..", "..") |> DirectoryInfo
2020

2121
let private RemoveJsonFormatting (jsonContent: string): string =
2222
jsonContent.Replace("\r", String.Empty)
@@ -50,12 +50,6 @@ module MarshallingData =
5050
let RealExceptionExampleInJson =
5151
ReadEmbeddedResource "realException.json"
5252

53-
let RealExceptionUnixLegacyExampleInJson =
54-
ReadEmbeddedResource "realException_unixLegacy.json"
55-
56-
let RealExceptionWindowsLegacyExampleInJson =
57-
ReadEmbeddedResource "realException_windowsLegacy.json"
58-
5953
let InnerExceptionExampleInJson =
6054
ReadEmbeddedResource "innerException.json"
6155

@@ -65,80 +59,35 @@ module MarshallingData =
6559
let CustomFSharpExceptionExampleInJson =
6660
ReadEmbeddedResource "customFSharpException.json"
6761

68-
let CustomFSharpExceptionLegacyExampleInJson =
69-
ReadEmbeddedResource "customFSharpException_legacy.json"
70-
7162
let FullExceptionExampleInJson =
7263
ReadEmbeddedResource "fullException.json"
7364

74-
let FullExceptionUnixLegacyExampleInJson =
75-
ReadEmbeddedResource "fullException_unixLegacy.json"
76-
77-
let FullExceptionWindowsLegacyExampleInJson =
78-
ReadEmbeddedResource "fullException_windowsLegacy.json"
79-
8065
let rec TrimOutsideAndInside(str: string) =
8166
let trimmed = str.Replace(" ", " ").Trim()
8267
if trimmed = str then
8368
trimmed
8469
else
8570
TrimOutsideAndInside trimmed
8671

87-
let SerializedExceptionsAreSame actualJsonString expectedJsonString ignoreExMessage msg =
88-
89-
let actualJsonException = JObject.Parse actualJsonString
90-
let expectedJsonException = JObject.Parse expectedJsonString
91-
92-
let fullBinaryFormPath = "Value.FullBinaryForm"
93-
let tweakStackTraces () =
94-
95-
let fullBinaryFormBeginning = "AAEAAAD/////AQAA"
96-
let stackTracePath = "Value.HumanReadableSummary.StackTrace"
97-
let stackTraceFragment = "ExceptionMarshalling.fs"
98-
99-
let tweakStackTraceAndBinaryForm (jsonEx: JObject) (assertBinaryForm: bool) =
100-
let stackTraceJToken = jsonEx.SelectToken stackTracePath
101-
Assert.That(stackTraceJToken, Is.Not.Null, sprintf "Path %s not found in %s" stackTracePath (jsonEx.ToString()))
102-
let initialStackTraceJToken = stackTraceJToken.ToString()
103-
if initialStackTraceJToken.Length > 0 then
104-
Assert.That(initialStackTraceJToken, IsString.WhichContains stackTraceFragment,
105-
sprintf "comparing actual '%s' with expected '%s'" actualJsonString expectedJsonString)
106-
let endOfStackTrace = initialStackTraceJToken.Substring(initialStackTraceJToken.IndexOf stackTraceFragment)
107-
let tweakedEndOfStackTrace =
108-
endOfStackTrace
109-
.Replace(":line 42", ":41 ")
110-
.Replace(":line 41", ":41 ")
111-
.Replace(":line 65", ":64 ")
112-
.Replace(":line 64", ":64 ")
113-
stackTraceJToken.Replace (tweakedEndOfStackTrace |> JToken.op_Implicit)
114-
115-
let binaryFormToken = jsonEx.SelectToken fullBinaryFormPath
116-
Assert.That(binaryFormToken, Is.Not.Null, sprintf "Path %s not found in %s" fullBinaryFormPath (jsonEx.ToString()))
117-
let initialBinaryFormJToken = binaryFormToken.ToString()
118-
if assertBinaryForm then
119-
Assert.That(initialBinaryFormJToken, IsString.StartingWith fullBinaryFormBeginning)
120-
binaryFormToken.Replace (fullBinaryFormBeginning |> JToken.op_Implicit)
121-
122-
tweakStackTraceAndBinaryForm actualJsonException true
123-
tweakStackTraceAndBinaryForm expectedJsonException false
124-
125-
tweakStackTraces()
126-
127-
// strangely enough, message would be different between linux_vanilla_dotnet6 and other dotnet6 configs (e.g. Windows, macOS, Linux-github)
128-
if ignoreExMessage then
129-
let exMessagePath = "Value.HumanReadableSummary.Message"
130-
let actualMsgToken = actualJsonException.SelectToken exMessagePath
131-
Assert.That(actualMsgToken, Is.Not.Null, sprintf "Path %s not found in %s" exMessagePath (actualJsonException.ToString()))
132-
let expectedMsgToken = expectedJsonException.SelectToken exMessagePath
133-
Assert.That(expectedMsgToken, Is.Not.Null, sprintf "Path %s not found in %s" exMessagePath (expectedJsonException.ToString()))
134-
actualMsgToken.Replace(String.Empty |> JToken.op_Implicit)
135-
expectedMsgToken.Replace(String.Empty |> JToken.op_Implicit)
136-
137-
let actualBinaryForm = (actualJsonException.SelectToken fullBinaryFormPath).ToString()
72+
let SerializedExceptionsAreSame actualJsonString expectedJsonString (msg: string) =
73+
let actualJson = JObject.Parse actualJsonString
74+
Assert.That(actualJson, Is.Not.Null)
75+
let expectedJson = JObject.Parse expectedJsonString
76+
Assert.That(expectedJson, Is.Not.Null)
77+
78+
let fullDescPath = "Value.FullDescription"
79+
let actualJsonToken = actualJson.SelectToken fullDescPath
80+
Assert.That(actualJsonToken, Is.Not.Null)
81+
let expectedJsonToken = expectedJson.SelectToken fullDescPath
82+
Assert.That(expectedJsonToken, Is.Not.Null)
83+
84+
let actualFullDesc = actualJsonToken.ToString()
85+
let expectedFullDesc = expectedJsonToken.ToString()
86+
13887
Assert.That(
139-
TrimOutsideAndInside(actualJsonException.ToString()),
140-
Is.EqualTo (TrimOutsideAndInside(expectedJsonException.ToString())),
141-
msg + actualBinaryForm
88+
TrimOutsideAndInside actualFullDesc,
89+
Is.EqualTo (TrimOutsideAndInside expectedFullDesc),
90+
msg
14291
)
14392

14493
true

src/GWallet.Backend.Tests/data/basicException.json

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,7 @@
22
"Version": "{version}",
33
"TypeName": "GWallet.Backend.MarshalledException",
44
"Value": {
5-
"HumanReadableSummary": {
6-
"ExceptionType": "System.Exception",
7-
"Message": "msg",
8-
"StackTrace": "",
9-
"InnerException": null
10-
},
11-
"FullBinaryForm": "{binaryMarshalledException}"
5+
"DateTimeUtc": "<this value is not compared in tests>",
6+
"FullDescription": "System.Exception: msg"
127
}
138
}

src/GWallet.Backend.Tests/data/customException.json

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,7 @@
22
"Version": "{version}",
33
"TypeName": "GWallet.Backend.MarshalledException",
44
"Value": {
5-
"HumanReadableSummary": {
6-
"ExceptionType": "GWallet.Backend.Tests.CustomException",
7-
"Message": "msg",
8-
"StackTrace": "",
9-
"InnerException": null
10-
},
11-
"FullBinaryForm": "{binaryMarshalledException}"
5+
"DateTimeUtc": "<this value is not compared in tests>",
6+
"FullDescription": "GWallet.Backend.Tests.CustomException: msg"
127
}
138
}

src/GWallet.Backend.Tests/data/customFSharpException.json

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,7 @@
22
"Version": "{version}",
33
"TypeName": "GWallet.Backend.MarshalledException",
44
"Value": {
5-
"HumanReadableSummary": {
6-
"ExceptionType": "GWallet.Backend.Tests.CustomFSharpException",
7-
"Message": "Exception of type 'GWallet.Backend.Tests.CustomFSharpException' was thrown.",
8-
"StackTrace": "",
9-
"InnerException": null
10-
},
11-
"FullBinaryForm": "{binaryMarshalledException}"
5+
"DateTimeUtc": "<this value is not compared in tests>",
6+
"FullDescription": "GWallet.Backend.Tests.CustomFSharpException: CustomFSharpException"
127
}
138
}

src/GWallet.Backend.Tests/data/customFSharpException_legacy.json

Lines changed: 0 additions & 13 deletions
This file was deleted.

src/GWallet.Backend.Tests/data/fullException_unixLegacy.json

Lines changed: 0 additions & 23 deletions
This file was deleted.

src/GWallet.Backend.Tests/data/fullException_windowsLegacy.json

Lines changed: 0 additions & 23 deletions
This file was deleted.

src/GWallet.Backend.Tests/data/innerException.json

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,7 @@
22
"Version": "{version}",
33
"TypeName": "GWallet.Backend.MarshalledException",
44
"Value": {
5-
"HumanReadableSummary": {
6-
"ExceptionType": "System.Exception",
7-
"Message": "msg",
8-
"StackTrace": "",
9-
"InnerException": {
10-
"Case": "Some",
11-
"Fields": [
12-
{
13-
"ExceptionType": "System.Exception",
14-
"Message": "innerMsg",
15-
"StackTrace": "",
16-
"InnerException": null
17-
}
18-
]
19-
}
20-
},
21-
"FullBinaryForm": "{binaryMarshalledException}"
5+
"DateTimeUtc": "<this value is not compared in tests>",
6+
"FullDescription": "GWallet.Backend.Tests.CustomFSharpException: CustomFSharpException"
227
}
238
}

0 commit comments

Comments
 (0)