-
Notifications
You must be signed in to change notification settings - Fork 37
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
Showing
17 changed files
with
132 additions
and
369 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.