Add file/line information to main error type with macros

[svix-gabriel]

This closes #574, by adding diagnostic information to the main error type along with some helpful macros.

Motivation

For internal server errors, we want to visibility into both the underlying error, in addition to where the error originated from. e.g. for code like this

fn foo() -> crate::error::Result<()> {
   let app = Application.fetch(...)?;
   app.bar = 12;
   app.save()?;
   Ok(())
}

Just looking at the logs, it can be hard to diagnose whether the underlying db error came from Application.fetch(...)? or app.save()?.

We want to be able to quickly correlate an error to a specific line, while still keeping error handling relatively boilerplate free.

Solution

First, we extend the error::Error type with some diagnostic information - literally just the line number and file name.

To avoid really boilerplate heavy code, most of the error::Error initialization logic is now handled by macros. queue_err!, generic_err!, etc. This makes it pretty easy to add the file/line information without making error initialization excessively verbose (in some cases, we're actually able to make error initialization easier than before).

One point of friction is that we can't easily attach location information when propagating errors using ? - which we lean on pretty heavily when converting DbErr, RedisError, bb8::RunError, etc. Adding the location information to an error should be easy, but it should also be something that is difficult to forget to do.

To circumvent this problem, I dropped the From<...> for Error implementations for DbErr, RedisError, etc. These were replaced with a very similar trait to just transforms std::result::Result<T, E> into crate::error::Result<T> for DbErr, RedisError, etc. There's then a ctx! macro that handles the conversion, and adds the location information. Now error handling looks closer too

fn foo() -> crate::error::Result<()> {
   let app = ctx!(Application.fetch(...))?;
   app.bar = 12;
   ctx!(app.save())?;
   Ok(())
}

So, marginally more boilerplate. But forgetting to include ctx! now becomes a compiler error, forcing developers to add the diagnostic information using ctx!.

In addition, ctx! adds tracing information if the error is already a crate::result::Result.

What does this end up looking like in practice? If you have code like this

fn fallible_a() -> std::result::Result<(), DbErr> {
    Err(DbErr::Custom("db borked!".into()))
}

fn fallible_b() -> Result<()> {
    ctx!(fallible_a()) // line 182 in application.s
}

// . . .

ctx!(fallible_b())?; // line 190 in application.rs

when the error is transformed into a response from the axum handlers, we get logs like

2022-09-14T14:13:04.156518Z ERROR HTTP request{...}: svix_server::error: type: Database("Custom Error: db borked!"), location: ["svix-server/src/v1/endpoints/application.rs:182", "svix-server/src/v1/endpoints/application.rs:190"]

[svix-daniel]

Overall, these changes are pretty good. It's a shame that the syntax can't be nicer, but I understand it'd require a lot more effort to get it to look more natural.

One question:

In addition, ctx! adds tracing information if the error is already a crate::result::Result.
Is there any way to enforce adding a ctx! invocation easily? It'd be useful to get the traceback as bulky as possible (within reason), so I'd want to get it to as close to the top as possible in its use.

I have one really hacky idea for that, but it'd be terribly hard to do, so I'll leave the topic be if there's no obvious solution.

[svix-gabriel]

Is there any way to enforce adding a ctx! invocation easily?

Yeah, I definitely agree having some kind of compile time enforcement for calling ctx! would be nice. I can't think of any reasonable approach though