Conversation
Error responses of `lambda_runtime` are now customizable. One use case is to avoid using `std::any::type_name` for error types, which is not reliable for determining subsequent program behavior. `F::Error` of `Runtime::run` and `run` is required to implement `Into<Diagnostic<'a>>` instead of `Display`. `EventErrorRequest` accepts a value implementing `Into<Diagnostic<'a>>` instead of the error type and message. `Diagnostic` becomes public because users may implement their own conversions. Its fields are wrapped in `Cow` so that they can carry both borrowed and owned strings. `Diagnostic` is implemented for every type that implements `Display` as a fallback, which also ensures backward compatibility.
The comments on `Diagnositc` shows how to customize error responses with an example.
calavera
left a comment
There was a problem hiding this comment.
This is really nice, thank you!
| use lambda_runtime_api_client::{body::Body, BoxError, Client}; | ||
| use serde::{Deserialize, Serialize}; | ||
| use std::{ | ||
| borrow::Cow, |
There was a problem hiding this comment.
@evbo I introduced Cow to overcome a lifetime issue rather than performance concern. Since the fallback conversion from Error (Display) into Diagnostic had to dynamically format error_message, I was not able to simply return &str: https://github.com/awslabs/aws-lambda-rust-runtime/blob/e0e95e61cba0a8a695d4766d9215cbe839caf3a7/lambda-runtime/src/types.rs#L67-L77
However, regarding error_type, it could have been &str. I did not notice that until you pointed it out.
There was a problem hiding this comment.
error_type can't always be &str in the case where you're borrowing the variant name (such as with strum::AsRefStr), which is helpful for auto-converting Variants to error types.
I'm not sure why both can't just be String? Just seems like a lot of extra typing using Cow, sorry to nitpick
There was a problem hiding this comment.
I think some might say the opposite "why String? they are &'static str most of the time."
Btw, you may avoid explicit use of Cow as long as you are dealing with &str or String:
let msg = "message".to_string();
Diagnostic {
error_type: "SomeError".into(), // &str → Cow::Borrowed
error_message: msg.into(), // String → Cow::Owned
};There was a problem hiding this comment.
Thanks for the into - that's much cleaner. I hate to burn up so much time, but why would someone say: "why String"? If we're creating millions of exceptions, then I understand the cost of heap allocation would eventually cost noticeably, but maybe at most 1 exception gets generated once in a blue moon, so isn't it fair using String?
|
@evbo Have you tried something like the following? #[derive(thiserror::Error, Debug)]
pub enum MyError {
#[error("some error: {0}")]
SomeError(String),
// ... other variants
}
impl<'a> Into<Diagnostic<'a>> for MyError {
fn into(self) -> Diagnostic<'a> {
match self {
Self::SomeError(s) => Diagnostic {
error_type: Cow::Borrowed("SomeError"),
error_message: Cow::Owned(s),
},
// ... other variants
}
}
}The above will cause conflict between A workaround in this case may be to define a wrapper struct for your error type and implement struct ErrorResponse(MyError);
impl<'a> Into<Diagnostic<'a>> for ErrorResponse {
fn into(self) -> Diagnostic<'a> {
match self.0 {
ErrorResponse::SomeError(s) => Diagnostic {
error_type: Cow::Borrowed("SomeError"),
error_message: Cow::Owned(s),
},
// ... other variants
}
}
}
Obviously, I should refine the documentation. |
|
@kikuomax I take back my earlier messages. I think you're right - this can be solved with better docs. I think the best ergonomics are achieved when |
|
@evbo I am planning a PR to improve the documentation. How about having the following in the API documentation? Does it make better sense?
Example: use lambda_runtime::{service_fn, Diagnostic, Error, LambdaEvent};
#[derive(Debug)]
struct ErrorResponse(Error);
impl<'a> Into<Diagnostic<'a>> for ErrorResponse {
fn into(self) -> Diagnostic<'a> {
Diagnostic {
error_type: "MyError".into(),
error_message: self.0.to_string().into(),
}
}
}
async fn function_handler(_event: LambdaEvent<()>) -> Result<(), Error> {
// ... do something
Ok(())
}
#[tokio::main]
async fn main() -> Result<(), Error> {
lambda_runtime::run(service_fn(|event| async {
function_handler(event).await.map_err(ErrorResponse)
})).await
}The container ⓘ compile_fail use lambda_runtime::Diagnostic;
#[derive(Debug)]
struct MyError(String);
impl std::fmt::Display for MyError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "MyError: {}", self.0)
}
}
impl std::error::Error for MyError {}
// ERROR! conflicting implementations of trait `Into<Diagnostic<'_>>` for type `MyError`
impl<'a> Into<Diagnostic<'a>> for MyError {
fn into(self) -> Diagnostic<'a> {
Diagnostic {
error_type: "MyError".into(),
error_message: self.0.into(),
}
}
} |
Issue #, if available:
Description of changes:
F::ErrorofRuntime::runandrunis required to implementInto<Diagnostic<'a>>instead ofstd::fmt::DisplayInto<Diagnostic<'a>>is implemented for any types that implementstd::fmt::Displayas a fallback and for backward compatibilityDiagnosticand its fields become public so that users can customize error responseserror_typeanderror_messageofDiagnosticare wrapped instd::borrow::Cowso that they can hold both borrowed and owned strings. An example of the situation where an owned string is necessary is in the fallback implementation ofFrom<Display>forDiagnostic, which generateserror_messageinsidefrommethod and it has to outlive the method call.By submitting this pull request