upenn-acg/ProcessCache

Better Error Messages

Closed this issue · 2 comments

I have been having fun with error handling :)

In the simplest case you can handle Result::Errs by doing .expect("Error Message"). This has several disadvantages (all issues associated with panicking, etc). We use the anyhow crate for better error handling.

Without going into too much detail, we now get nicer error messages on failures with a pseudo-stacktrace (stack traces are only available for errors on nightly (unless you panic)). Like so:

io_tracker::execution::run_process(): Process Pid(154290) failed. file: src/execution.rs, line: 211.

Caused by:
    0: io_tracker::execution::do_run_process(): Unable to get syscall name for syscall=65535. file: src/execution.rs, line: 255.
    1: io_tracker::system_call_names::get_syscall_name(): System call number 65535 out of bounds. file: src/system_call_names.rs, line: 8.

This should help us track down errors and provides semantic information of why the error happened as well as the context from where it happened.

All we have to do is make sure when handling any errors, e.g. using the ? operator, we use .with_context(|| context!("error message")) like here:

let name = get_syscall_name(event_message as usize).with_context(|| context!(
                    "Unable to get syscall name for syscall={}.",
                    event_message
                ))?;

The .with_context method is provided by anyhow. The context! macro is my own which adds: function name, file name, line number. It also wraps the format! macro. Notice it accepts string literals like

context!("Unable to fetch regs for unspecified syscall")

or formatted strings. Similarly when using anyhow::bail! we can use the context! macro like so:

bail!(context!("Unhandled system call {:?}", name));

This produces nice errors like:

io_tracker::execution::run_process(): Process Pid(158683) failed. file: src/execution.rs, line: 211.

Caused by:
    io_tracker::execution::do_run_process(): Unhandled system call "brk" file: src/execution.rs, line: 253.

This has already been implemented with my latest changes and we just have to be diligent about adding context info!

krs85 commented

This is amazing!

Closing as #28 has been merged.