Rethinking the error management system

[kamyuentse]

Currently, the CSwitch uses the basic handwritten Error type to manage the error, I admit that's the most flexible way, but it would make us spent more time to write the code here if we want to trace the source of the error, or we got a useless error.

For example, in the crypto, I notice we use () as the error in many places. But when the program runs into trouble, how to find the where is the exact location? If we leave this implementation, the caller must handle this error once the error happened, and assign a proper Error variant for it, this work should be done for each caller.

Another example is the Channeler, in the Channeler, we have some submodule, each of them may run into trouble. If we want to write a recoverable system, I think we should divide the error into:

• fatal ── we should close the whole system.
• recoverable ── which means that module is already broken, we should find a way restart the module, if we can't restart this module, it would be a fatal error, we should close the whole system.
• ignorable ── an error happened, but it does not harm the whole module. For example, in futures 0.1, when polling a stream and it returns an Error, it doesn't mean that the stream terminated, check Consider having polling an error represent the final Stream value rust-lang/futures-rs#206 for more details.

The partition above might not accurate, just my idea.

---

If this is reasonable, we may discuss it, and add more things here.

[realcr]

@kamyuentse :
Thanks for trying to put some order into the code. I think that I understand some of the idea now, but I'm not fully sure. I just read rust-lang/futures-rs#206. I understand it correctly, they talk about returning an error from a Stream, and aborting the Stream early.

For example, in the crypto, I notice we use () as the error in many places

This is because I was a clown, it was some very early code that I wrote. You may change all of this to a reasonable error type.

If we want to write a recoverable system, I think we should divide the error into:

Some questions:

1. Do you propose to make this change systemwise, or just as part of the Channeler?
2. How do you propose to make this change? Is this some kind of a trait over the relevant Error type, or something else?
3. Could you provide a code example from the Channeler where you think that this proposal will make life much easier?

Another thing: I know of some efforts in the Rust community to streamline the handling of errors. I know of error-chain, but I also heard about some other alternatives.
I checked some code that uses those crates. To me it seems like exceptions reinvented. It feels like we may lose some of the control we have on the error types if we use this. Of course, I never actually tried to use error-chain, so I might have a very wrong idea of what it is like.

What is your opinion of this direction? Is it related to your suggestion?

[A4Vision]

Hey @kamyuentse ,

What is the correct way to handle errors in the following function ?
https://github.com/realcr/cswitch/blob/real/feat/messenger-balance-state/src/networker/messenger/token_channel.rs#L306

Don't bother reading the code and understanding its functionality, just take a look at its structure.
We call a function, and check for None value to handle an error. Maybe the underlying functions, such as pending_request_option could return a Result instead of Option and the caller could use ? to translate it elegantly.

request = ...pending_request_option.map_err(| | ...)?;

(Notice the question mark above)

Assaf

[kamyuentse]

@A4Vision

No problem there.

Some advice:

1. If you want to translate Option<T> to a Result, use optional_value.ok_or(Err). Take a look at the Option::ok_or(..) might be helpful.
2. If you want to translate a Result to a Result, the map_err is an explicit way. Or you can implement impl From<ErrorA> for ErrorB, which is an implicit way.