KomodoPlatform / komodo-defi-framework

This is the official Komodo DeFi Framework repository
https://komodoplatform.com/en/docs/komodo-defi-framework/
102 stars 94 forks source link

Add indication whether recover funds of swap may be called again. #646

Open artemii235 opened 4 years ago

artemii235 commented 4 years ago

https://github.com/KomodoPlatform/atomicDEX-Pro/issues/182#issuecomment-633430481

artemii235 commented 4 years ago

This turns out to be not that easy because errors are Strings everywhere so actually entire RPC server error handling part needs refactoring, postponing for a while.

ArtemGr commented 4 years ago

One can add tags to error strings while retaining the flexibility of joining Strings (e.g. stack traces and different error tags from different layers)

artemii235 commented 4 years ago

I prefer to avoid using "String-only" error approach in the future, it leads to unclean and unobvious error handling code while it can be common match ... statement. Also as some parts of the project will be moved to separate crates/lib it will be nice to follow the official Rust API guidelines: https://rust-lang.github.io/api-guidelines/interoperability.html#c-good-err

Instead, define a meaningful error type specific to your crate or to the individual function. Provide appropriate Error and Display impls. If there is no useful information for the error to carry, it can be implemented as a unit struct.

Also it became quite irritating to put the try_s macro everywhere, that's why ? operator was actually added to replace the std::try macro: https://doc.rust-lang.org/edition-guide/rust-2018/error-handling-and-panics/the-question-mark-operator-for-easier-error-handling.html

I wonder if it will be possible to maintain the stack trace with such common approach, will research later, this task is postponed now because it needs refactoring regardless of the chosen implementation path.

ArtemGr commented 4 years ago

I prefer to avoid using "String-only" error approach in the future, it leads to unclean and unobvious error handling code while it can be common match ... statement

That's quite an expected sentiment. I'd like to offer a few concerns for you to consider in the error handling design: 1) Errors are passed usually through multiple layers, and unpacking a certain error code or a message after it has been wrapped in those layers not always pretty. 2) Pattern matching is tightly coupled (opposite of louse coupling): you need to have the exact version of the Rust library to match the errors properly. Any changes in the error enums need a full major semver bump (that is, a tiniest change in the error enums means full API incompatibility). 3) This in turn leads to the increased risk of employing the https://en.wikipedia.org/wiki/Error_hiding antipatten. 4) HTTP clients or clients in languages and/or programs not having direct access to the Rust libraries defining all the error enums will not benefit from a "common match", error handing for such clients will be anything but common

I wonder if it will be possible to maintain the stack trace with such common approach

It's been brewing for quite some time, cf. rust-lang/rust/issues/53487