error handling

[colin-kiegel]

It would be great, if we find a common way here. That will make everything else a lot easier!

Ok, here is our starting point :-)

twig-rust

• return err!(ParserErrorCode::Logic, "Unexpected end of token stream"), where filename:line:column and module-path is collected by the err!-macro automatically. It also supports formatting err!(..., "Hello {name}", name = "world"). This is something I find quite useful.
• and I use try!( .... ) a lot, where try! will convert different error-types for me, because my error-types implement a couple of Into-traits, e.g. a SyntaxError implements Into, via this macro: impl_convert_exception!(SyntaxErrorCode, LexerErrorCode, LexerErrorCode::SyntaxError);

Definitions are in: src/error/* - but I am sure, it has room for improvement. It definitely needs some cleanup, due to boiler-plate when defining new error definitions.

twig-rs

I am keeping all errors in enums/structs and then converting them to string on actual display. There are 3 groups: template for lexer and parser errors, engine for loading/caching, and runtime which is a bit in flux. On top of that, there is master twig Error which can contain any of these children, and in case of engine/runtime, the chain of error causes. Of course, all chilren are implementing Into trait to be converted to "master" error.

I have found one idea useful: separating location from actual error. There is this At struct which wraps actual error inside. To convert to it, there is method on error named at, which requires location in file. So I do this:

TemplateError::ExpectedTokenTypeButReceived(
    (expected.into(), Received::EndOfStream)
).at(line)

Note that I only have line numbers, but I was planning to expand this to line/col later.

[colin-kiegel]

Reopening (see https://github.com/rust-web/twig/pull/12#issuecomment-162331981 + following 2 comments).

Look at alternatives to provide

• composable structured errors
• with traces
• implementing std::error::Error
• in a rusty way

/// Runtime error with stack trace.
#[derive(Clone, Debug)]
pub struct Traced<T: Clone, Debug> {
    pub error: T,
    /// Trace entries which may have location and/or another error. Maybe another struct.
    pub trace: Vec<(Location, Option<T>)>,
}

[colin-kiegel]

I did it quite similar to your suggestion - but instead of Vec<(Location, Option<T>)> the trace only is Vec<Location>.

We can use the inner error for rust-style error-composition MyError::IO(std::io::Error). Of course we loose information about when exactly such nesting of errors occurs along the backtrace, but I think this is totally acceptable. Presentation will be a lot easier, because we can separate inner error and trace presentation. The first thing the user will see is the inner error.

What do you think?

[Nercury]

I am willing to try it :)

[colin-kiegel]

Ok. :-)

Btw: I was not sure about one detail - should Traced<T: Error> (a) deref to T: Error (current choice) (b) implement Error by delegating to T: Error (c) neither one (d) don't care

I think Deref is usually reserved for smart pointers and Newtype-Tuple-Structs like struct Inches(i32);. On the other hand implementing Error would allow Traced<Traced<T: Error>> - which is not necessarily something bad, but it could happen by accident.

[Nercury]

I would do no magic until really needed. So I think (c) until proven otherwise :)

[colin-kiegel]

Ok, I will remove (a).

I had another thought about it. I think what we want is composable errors and giving the caller the flexibility to convert our traced errors into opaque trait objects. If a consumer of Twig wants to convert our Traced<TwigError> into Box<Error> with the following code, he will fail:

fn foo() -> Result<(), Box<Error>> {
    let _ = try!(twig::Engine::fail());
    // ...
}

The try macro will try to call <Box<Error> as From>::<Traced<TwigError>>::from()

• the caller can't implement this trait
• we can not implement this trait either (because From and Box are foreign to our crate)
• But we can either implement Error for Traced<TwigError> or Traced<T: Error>. If we do that, than try will be able to call <Box<Error> as From>::<T: Error>::from() which is implemented by the std-lib.

So actually I think we should do (b).

[Nercury]

Ok.