Rust ergonomics for error handling

[arienmalec]

The examples for using the Rust Lambda runtime require additional calls around error handling that require access to the Context object.

Unfortunately, that breaks Rust ergonomics for error handing, that ordinarily would use ? and map custom errors to HandlerError through an impl From (which doesn't have a reference to the Context struct.

Ideally, I could return my own error types, and either have the runtime take care of supplying context, with perhaps some place to inject logging, etc. into the error handing flow.

In practice, in the short term, I'm breaking out all actions that can return a Result into a separate module, because even trivial activities get complicated quickly if you want to inject error handing with access to Context.

[davidbarsky]

Thanks for the feedback—we really appreciate it! I'm thinking of replacing the current setup with something that leverages (maybe) Failure and (for sure) ErrorKinds, and using something similar to what Sean proposed in tower-rs/tower#131. Would a system like this—behind a breaking change semver bump—be something like you're looking for?

[arienmalec]

I think so. In my head, probably poorly thought out, the handler type would be (U,Context) -> Result<T,<Box<dyn Error>>, and the lambda macro would have an (optional?) second hook for logging/cleanup.

An example of how I'm doing this, loosely based on map_err:

fn map_handler_err(msg: &str, ctx: &Context, err: impl Error) -> HandlerError {
    error!("{} in request {}",msg, ctx.aws_request_id);
    ctx.new_error(&err.to_string())
}

fn my_handler(event: MsgEvent, ctx: Context) -> Result<MsgResponse, HandlerError> {
    match record::Record::parse_base64(&event.msg) {
        Ok(rec) => {
            let s = rec.summary();
            Ok(MsgResponse{id: s.id, sum: s.sum, longest: s.longest})
        },
        Err(e) => Err(map_handler_err("Message parsing error", &ctx, e))
    }
}

Which suggests a non-breaking old-style Rust error handling hack: a trait with a map_err_ctx or some such, so that I could rewrite the handler to:

let r = record::Record::parse_base64(&event.msg).map_err_ctx(my_err_handler)?;
let s = r.summary();
Ok(MsgResponse{...}

[sapessi]

I like the idea. One more change we should couple with this: give you the ability to return a custom error code for the APIs. The Runtime APIs error structure is very simple, we could probably drop the RuntimeApiError trait and reuse the HandlerError as a trait that defines a single error_code() -> &str method for custom error types to implement. The runtime should be smart enough to produce the error struct the APIs expect.

[davidbarsky]

Spoke to @sapessi about this. Long story short, we're considering several options:

• Introduce a Display bound on errors returned by the handler so that we can give the Lambda runtime something useful if an error occurs.
• Use Failure's Fail trait as a bound on returned errors. This is more heavyweight and removal would be a breaking change.

Alternatively, we can define a handler in terms of std::future::Future (when stable, 6-12 weeks from now) which doesn't have an error field, just an associated type called Output that can be a Result<T, E> or just a field. This might be a separate discussion, however.

[sapessi]

The Runtime APIs expect a few properties with an error (listed below). Because these properties are relatively generic, nothing ties us to an std::error::Error trait. In PR #26 we started implementing support for any type that implements std::fmt::Display in order to get the message. we'd like to get your thoughts on this.

• errorType: String in payload
• errorMessage: String in payload
• stackTrace: Vec - Optional
• Lambda-Runtime-Function-Error-Type: String in header (always Unhandled)

The errorType is where we are still debating what to do. At the moment we are sending a static value. We thought we could use the type name of the error but I have no "safe" way to get the actual type. Alternatively, we could define an additional trait that declares the error_code() -> &str method.

The message should represent a description of the error. This is the one parameter I must get from the handler code. For this reason, David an I thought that std::fmt::Display was the only trait we really needed in the Fail branch of the Result.

The stack trace I can collect myself using the backtrace crate. We already do that if the RUST_BACKTRACE env variable is set.

CC @arienmalec @softprops @Sh4rK

[Sh4rK]

What does error "code" mean in this context? I would expect it to be an integer. What is it used for?

[sapessi]

Working on this myself @Sh4rK. From the documentation, I can see that there's an errorType property in the request body which represents what I've been calling code. This is a custom alphanumerical identifier for the specific error thrown. In the OpenAPI specs for the runtime APIs (below), there's also a Lambda-Runtime-Function-Error-Type which I assume is the Handler/Unhandled value. I'll get in touch with the Lambda team to clarify.

...
  /runtime/invocation/{AwsRequestId}/error:
    post:
      summary: >
        Runtime makes this request in order to submit an error response. It can
        be either a function error, or a runtime error. Error will be served in
        response to the invoke.
      parameters:
        - in: path
          name: AwsRequestId
          schema:
            type: string
          required: true
        - in: header
          name: Lambda-Runtime-Function-Error-Type
          schema:
            type: string
...

[sapessi]

Got the clarification I needed and updated the original post

[Sh4rK]

I'm a bit confused.

1. Now we have the errorType field in the JSON result and the Lambda-Runtime-Function-Error-Type header, and in the code (in the PR) they are now both referred to as simply "error type" (err_type, error_type, etc.). It would be useful to disambiguate between them. Some comments are confusing as well.

2. I don't really understand what the Lambda-Runtime-Function-Error-Type header is supposed to be for. It's not mentioned at all in the online docs. In runtime-api.yaml it is stated to be a parameter for the error methods, but not what its values can be, or what it is for. There's a comment at the top about it:

   # Lambda's default behavior is to use Lambda-Runtime-Function-Error-Type header value to construct an error response
   # when error payload is not provided or can not be read.
   

   Does this mean we can leave the header out if we send the body? Or maybe send it, set to the same value as the errorType field in the JSON?

3. Where does the Handled / Unhandled values for the header come from? What do they even mean? If an error is handled, we don't return it, since we handle it ourselves, so I don't see how any returned error can be anything but Unhandled.

4. I'm not sure backtrace generation for Result-style error handling, at least in the state currently in the PR, makes sense: the backtrace is generated far away from where the error actually originates from, so it would not be helpful. Creating errors from the Context object work because the backtrace is generated then and there. How about we forget about backtraces for Result-style error handling (as it is everywhere else in Rust land), and later when we handle panics, we can return stack traces.

[sapessi]

I'm a bit confused.

1. Now we have the errorType field in the JSON result and the Lambda-Runtime-Function-Error-Type header, and in the code (in the PR) they are now both referred to as simply "error type" (err_type, error_type, etc.). It would be useful to disambiguate between them. Some comments are confusing as well.

Agree that it needs disambiguation. The only field that matters as far as handlers are concerned is the errorType in the body. We should find a way to populate that with a code that makes sense when looking at logs.

2. I don't really understand what the Lambda-Runtime-Function-Error-Type header is supposed to be for. It's not mentioned at all in the online docs. In runtime-api.yaml it is stated to be a parameter for the error methods, but not what its values can be, or what it is for. There's a comment at the top about it:

   # Lambda's default behavior is to use Lambda-Runtime-Function-Error-Type header value to construct an error response
   # when error payload is not provided or can not be read.
   

   Does this mean we can leave the header out if we send the body? Or maybe send it, set to the same value as the errorType field in the JSON?

The field is not used at the moment. We should set it to Unhandled by default.

3. Where does the Handled / Unhandled values for the header come from? What do they even mean? If an error is handled, we don't return it, since we handle it ourselves, so I don't see how any returned error can be anything but Unhandled.
4. I'm not sure backtrace generation for Result-style error handling, at least in the state currently in the PR, makes sense: the backtrace is generated far away from where the error actually originates from, so it would not be helpful. Creating errors from the Context object work because the backtrace is generated then and there. How about we forget about backtraces for Result-style error handling (as it is everywhere else in Rust land), and later when we handle panics, we can return stack traces.

Agree. We may be able to solve this as we address the first point (generating the back trace much closer to the error or by standardizing to the failure crate.

The first question we need to answer probably is: for the error result in the handler, do we want to rely on anything that implements Display, an Error from std, or a failure? Or perhaps something else I haven't considered at all. Once we have this answer, we can start planning the rest.

[Sh4rK]

The field is not used at the moment. We should set it to Unhandled by default.

What I wanted to find out is, where does this "We should set it to Unhandled" come from, that is, that Unhandled and Handled are the possible values for the header.

The first question we need to answer probably is: for the error result in the handler, do we want to rely on anything that implements Display, an Error from std, or a failure?

Actually, I think it should be possible to do all of them via some trait trickery, but I haven't tried. That way, if we get something Display, we just use that as the error, if it's a failure::AsFail, we can get a stacktrace to return, for std::error::Error maybe return the Error::source() chain as the stacktrace.

(Small side note: std::error::Error now requires that Display is implemented for the type, and the description() method is deprecated.)

[sapessi]

The field is not used at the moment. We should set it to Unhandled by default.

What I wanted to find out is, where does this "We should set it to Unhandled" come from, that is, that Unhandled and Handled are the possible values for the header.

That's what I've been told by the Lambda team - I have no more background than that.

The first question we need to answer probably is: for the error result in the handler, do we want to rely on anything that implements Display, an Error from std, or a failure?

Actually, I think it should be possible to do all of them via some trait trickery, but I haven't tried. That way, if we get something Display, we just use that as the error, if it's a failure::AsFail, we can get a stacktrace to return, for std::error::Error maybe return the Error::source() chain as the stacktrace.

That's what I've been trying to do (with very little success), as I understand we'd need specialization to make this work. The errors pull request is not the final implementation, more of a public playground to experiment with different ideas. We are talking through this internally now. Once we have some well formed thoughts, we'll update this issue with a full proposal.

(Small side note: std::error::Error now requires that Display is implemented for the type, and the description() method is deprecated.)