Do we have a guideline for handling error? about mesabox HOT 8 OPEN

There’s no guideline per se at the moment, but generally what you want to do is implement a type that derives Fail (such as ChmodError) and then map errors from the standard library or outside libraries to that type. I think cat is a fairly good example (IIRC). Both of the examples you found should be edge cases.

from mesabox.

I see. Thank you for your reply. I can understand now. I think a simple tutorial will be quite helpful for new contributors.

from mesabox.

I'll leave this issue open. When a consistent error handling method and guideline is ready, we can close this issue then.

from mesabox.

I mean the method is generally pretty consistent (I don’t think it’s possible to make the setup fully general as the utilities have widely different errors) except for a few strange situations (or laziness). I agree that we need some sort of guide though.

from mesabox.

derives Fail (such as ChmodError) and then map errors from the standard library or outside libraries to that type

Using failure is ok for me.

For consistency, I found that cat will return CatResult<T> (type CatResult<T> = ::std::result::Result<T, CatError>) but head will return super::Result<T> (pub type Result<T> = StdResult<T, MesaError>;). Although it can be implicit converted, it still very confusing when first reading the code.

Another example is where to handle (print) error message:

chmod use display_err! to print out error message.

head uses display-msg! to print out error message.

But cat returns CatError and do not print any error messages.

from mesabox.

Everything using display_err!() should switch to display_msg!() as we are getting rid of the error: prefix.

cat also prints errors (on phone so I can’t link). If the utilities didn’t, they would have to fail the moment they encounter an invalid file.

head uses super::Result because it doesn’t customize error messages. What specifically is confusing?

from mesabox.

Ok, understand. I found the display_msg statement in cat. Thank you!

You mentioned about using display_msg!() to display error message. When should I use it? Should I display the error message when encountering it or just return to the caller and let it to handle/print it?

For this one brought up by @rdzhou :

can be simplified, because the exitcode does nothing (the chmod function will return if it has error):

chmoder.chmod(matches.values_of_os("FILES").unwrap())?;

Also, this are more confusing:

Here, this returns error code or err type? DoesOk(1) mean there is an error? Seems that the code mixes traditional C error handling (by returning error code) with Rust's error (by returning Result) handling. In addition, for this specific function, is r always 0 (since you are using &=)?

from mesabox.

It should be |=, so there must have been a typo :/

I’d need to spend some time to review but IIRC chmod returns Err() on fatal errors whereas Ok(1) indicates there was an issue but it was able to keep going.

I’m still not entirely decided about what to do with the last error that gets returned from execute() (which is automatically printed). On one hand, it’s a little weird that it is treated differently from other errors (e.g. the per-file errors that get reported in cat). On the other hand, it makes writing the utilities more convenient as it eliminates boilerplate. At the moment, I would say if a utility takes multiple inputs and can keep going if one fails, then it should print out errors itself for each of the inputs (and then maybe keep track of the number of errors like in cat). If it takes one input or will fail if any of the inputs fails, then it should just return an error and let the framework machinery take care of printing.

from mesabox.