Open rdzhou opened 6 years ago
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.
I see. Thank you for your reply. I can understand now. I think a simple tutorial will be quite helpful for new contributors.
I'll leave this issue open. When a consistent error handling method and guideline is ready, we can close this issue then.
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.
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.
https://github.com/mesalock-linux/mesabox/blob/1b32bde66a7bd6b6f2d3898cea6460ee01bfd2eb/src/posix/chmod/mod.rs#L238
head
uses display-msg!
to print out error message.
https://github.com/mesalock-linux/mesabox/blob/1b32bde66a7bd6b6f2d3898cea6460ee01bfd2eb/src/posix/head/mod.rs#L147-L151
But cat
returns CatError and do not print any error messages.
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?
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 &=
)?
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.
Hi, I'm trying to contribute some code but get stuck in error handling.
One example I found for returning error is below. However, the
progname
anderr
are both None, which seems not very helpful.https://github.com/mesalock-linux/mesabox/blob/e90654052215046ed31ffcfe70b37715a1f8f8a3/src/posix/chmod/mod.rs#L161-L165
Another example I found is below. This line is quite difficult to understand for new contributors. https://github.com/mesalock-linux/mesabox/blob/e90654052215046ed31ffcfe70b37715a1f8f8a3/src/networking/ping/mod.rs#L279
Could you give some guildeline for error handling in mesabox?