-
Notifications
You must be signed in to change notification settings - Fork 361
[RFC] Error handling in Rust runtime #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
A friend (@myrrlyn, feel free to unsubscribe; credit where credit is due) suggested an alternative where the typename is an associated trait ErrorExt {
const TY: &'static str;
// ...
}
impl ErrorExt for Me {
const TY: &'static str = "Me";
// ...
}
fn handle<T, E: ErrorExt>(res: Result<T, E>) {
if let Err(e) = res {
println!("{}", E::TY);
}
} We might need to provide a derive on |
I‘ll try to expand on this later today, but here are some first thoughts and observations:
|
Forgot to say this, but maybe it‘s worth taking a page out of the trait ErrorExt: Display {
fn error_type(&self) -> &‘static str;
}
impl<E> From<E> for HandlerError where E: ErrorExt {
... // store E::error_type into an internal field on HandlerError
}
trait Handler<E, O> {
... // keep the existing HandlerError based definition
} This would allow anyone to use |
Thanks for the detailed feedback!
I suspect that this was an oversight, as I was working on an alternate black-box approach to the Lambda runtime where the event and responses are defined in terms of
I'm guessing that if we provide the impl out of Failure's book as described in this comment, that should address most of the issues you've raised? Also, I'm curious as to how you handle error reporting in srijs/rust-aws-lambda. Maybe we're missing something?
Yeah, that's one of the orphan implementations/type names were some of the biggest blockers we ran into. Not sure how to address them properly other than the situation above (or opening an RFC that introduces an
That's an interesting point. |
Adding a couple of points to @davidbarsky's answer
This is more of a note about the HTTP interface. Your errors can definitely support full UTF-8 but last I checked the runtime interface for lambda expects body in
You alternative solution is what I had been toying with: pub struct HandlerError {
msg: String,
}
impl<E: Display + Send + Sync + Debug> From<E> for HandlerError {
fn from(e: E) -> HandlerError {
HandlerError { msg: format!("{}", e) }
}
}
|
My main concern is:
This stack trace will point to somewhere inside the runtime, not anywhere useful in the user code, where the error originates from. Given this, I don't see how it would help. For getting the type name, we could use the I have my own idea on how the error handling should work, but I never fleshed it out. I'll try it and come back if I have something that seems to work :) |
The idea is to generate it inside the
The typename crate is interesting. However, giving users the ability to override the error type with a custom trait gives us more flexibility.
Please update this issue if you have other suggestions. |
They can do it just as easily by implementing the TypeName trait manually for their types. (That might however not actually be the "type name", so a custom trait can make sense.) |
I tried my idea, but unfortunately it doesn't seem to work, I can't get rid of some trait conflict errors. I suspect specialization would let me do what I want though. |
Yes, the example I provided solves both of these problems.
The decision I made there was basically 1) rely on the Relying on
An RFC that proposes a method like that to It also strikes me that there is some similarity between "error type", and the error kind mechanism that you can find in a few places like For the short-term, not sure, the choices I can see right now are either hard-coding the error type like I did, or introducing an additional trait like I appreciate that you probably have a desire to take advantage of the fact that the AWS Lambda client api supports separating error types from error messages, but it can't help wondering if that's worth the reduction in ergonomics. Lastly, speaking as a co-maintainer of Rusoto, I'm not sure we'd want to introduce a direct dependency on @sapessi: I've expanded a little bit on my example to illustrate how I might go about it (playground), but it feels like generally we're on the same page here about what that could look like 👍 use backtrace::Backtrace;
trait ErrorType: std::error::Error {
fn error_type(&self) -> &'static str;
}
struct HandlerError {
error: Box<ErrorType + Send + Sync>,
backtrace: Backtrace
}
impl<E> From<E> for HandlerError where E: ErrorType + Send + Sync + 'static {
fn from(err: E) -> Self {
HandlerError {
error: Box::new(err),
backtrace: Backtrace::new()
}
}
} |
Closed it by mistake, reopening.
They are meant to be different. Many customers stream logs from CloudWatch logs to an ELK/Splunk stack and rely on error types to generate their custom dashboard. I'd really like to give them the ability to set their custom types. As @Sh4rK discovered, without specialization, I cannot make it easy to implement this for all. If I declare a
Yes, this is the approach I was taking. The errorType is the biggest question left. I really would like to include it. However, as you said, I also want it to be idiomatic and very easy for customers to use. I have nothing against the idea of splitting the error definition into a separate crate so that you can re-use it in rusoto. |
Just as a data point, an advantage of using |
Thank you all for the comments so far. We experimented with the various methodologies on our side. Based on the results, we concluded that the best option is to rely on the
impl<F, E, O, ER> Runtime<F, E, O, ER>
where
F: Handler<E, O, ER>,
E: serde::de::DeserializeOwned,
O: serde::Serialize,
ER: AsFail + LambdaErrorExt + Send + Sync + Debug,
pub trait LambdaErrorExt {
fn error_type(&self) -> &str;
}
With the changes outlined above, developers will have to depend on the basic.rs use failure::{bail, Error as FailError};
...
fn my_handler(e: CustomEvent, c: lambda::Context) -> Result<CustomOutput, FailError> {
if e.first_name == "" {
error!("Empty first name in request {}", c.aws_request_id);
bail!("Empty first name");
}
Ok(CustomOutput {
message: format!("Hello, {}!", e.first_name),
})
} custom.rs use failure::Fail;
...
#[derive(Debug, Fail)]
#[fail(display = "custom error")]
enum CustomError {
#[fail(display = "{}", _0)]
ParseIntError(#[fail(cause)] std::num::ParseIntError),
#[fail(display = "Generic, unknown error")]
Generic,
}
impl LambdaErrorExt for CustomError {
fn error_type(&self) -> &str {
"MyCustomError"
}
}
...
fn my_handler(e: CustomEvent, c: lambda::Context) -> Result<CustomOutput, CustomError> {
if e.first_name == "" {
error!("Empty first name in request {}", c.aws_request_id);
return Err(CustomError::Generic);
}
let _age_num: u8 = e.age.parse().map_err(CustomError::ParseIntError)?;
Ok(CustomOutput {
message: format!("Hello, {}!", e.first_name),
})
} |
Changes are merged and new release is making its way to crates.io |
Currently, the Lambda Rust runtime declares a
HandlerError
type that developers can use to return an error from their handler method. While this approach works, it forces developers to write more verbose code to handle all errors and generate the relevantHandlerError
objects. Developers would like to return errors automatically using the?
operator (#23).For example:
In an error response, the Lambda runtime API expects two parameters:
errorType
anderrorMessage
. Optionally, we can also attach a stack trace to the error.To generate an error message field we need the
Err
variant from the handlerResult
to support theDisplay
trait. This allows us to support anyDisplay
type in the result -Error
types inherently supportsDisplay
too. However, we cannot identify a way to automatically generate the error type field given aDisplay
-compatible object that uses stable features. To address this, we plan to introduce a new trait in the runtime:We'd like to deprecate this trait in the future and rely on the type name intrinsic (which is currently blocked on specialization). For context, see #1428.
The name ErrorExt comes from the extension trait conventions RFC. Based on feedback, we are open to changing this.
The runtime crate itself will provide the implementation of the
ErrorExt
trait for the most common errors in the standard library. Developers will have to implement the trait themselves in their own errors. We may consider adding a procedural macro to the runtime crate to automatically generate the trait implementation.In summary, the proposed changes are:
Display
andErrorExt
in itsErr
variant.Display
trait to extract an error message and use theISO-8859-1
charset.error_type()
function to get the value for theerrorType
field.RUST_BACKTRACE
environment variable is set to1
, the runtime will use the backtrace crate to collect a stack trace as soon as the error is received.The new handler type definition will be:
The text was updated successfully, but these errors were encountered: