Simplified error handling

[miere]

I'd like to suggest a different error handling mechanism for this lambda runtime crate. As the majority of the Rust crates embrace recoverable Errors, we should as well provide an enum that allows developers to predictably deal with errors.

This would be extremely beneficial to the project itself, especially for lambda functions that handle HTTP requests. As a project grows bigger, we might take advantage of Rust's Recoverable Errors pattern to standardise error handling and generate uniform error responses - following OData's widely adopted best practices.

Also, it is a bit exhaustive to copy and paste type Error = Box<dyn std::error::Error + Send + Sync + 'static> in several standalone projects and not being able to take full advantage of the useful thiserror unless using a few hacks.

I'm aiming to have the handler code as simple as this:

use aws_lambda_events::event::sqs::SqsEvent;
use netlify_lambda::{handler_fn, Context, LambdaResult, LambdaRuntimeResult};

#[tokio::main]
async fn main() -> LambdaRuntimeResult {
  let func = handler_fn(handle_sqs_messages);
  netlify_lambda::run(func).await
}

async fn handle_sqs_messages(sqs_events: SqsEvent, _ctx: Context) -> LambdaResult<()> {
  println!("Received {} events", sqs_events.records.len());
  Ok(())
}

[calavera]

it is a bit exhaustive to copy and paste type Error = Box<dyn std::error::Error + Send + Sync + 'static>

I feel this so much!

I like your approach, and I agree that error handling can be much better. Feel free to open a PR with your suggested changes.

[miere]

Awesome. I'll send a proposal this weekend.

On Tue, Jan 5, 2021 at 7:21 AM David Calavera notifications@github.com wrote:

it is a bit exhaustive to copy and paste type Error = Box<dyn std::error::Error + Send + Sync + 'static>

I feel this so much!

I like your approach, and I agree that error handling can be much better. Feel free to open a PR with your suggested changes.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/netlify/aws-lambda-rust-runtime/issues/10#issuecomment-754196336, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD7NUFTILZ4Z4T2Z4EOK7DSYIPKTANCNFSM4VMYYW6A .

-- / Miere L. Teixeira /

[calavera]

@miere I was wondering if you've made any progress on this. It ok if not, I was just curious.

[miere]

@calavera I did have some progress indeed. I had to make a pause due to my annual leave.

From what I've seen in the source and the modification required to accomplish what I was aiming for, here are the steps that are required to be made:

• [x] Modify lambda crate, expanding the domain with the Error struct.
• [ ] Adjust the lambda-http crate to make use of the just-introduced Error struct.
• [ ] Update the rust documentation to match the newly created contract.

Although I mentioned creating an enum for the first item, after putting more thoughts on that, I turned it into a single struct. My reasoning lies at the fact we're not expected to handle those errors (which would be a good case for enums), but to comply with that Error contract. You can see the modified piece of code here.

Regarding the needed modifications in the lambda-http, I might have to spare extra time on that to make sure I'll introduce minimum breaking changes (if that's even possible). I shall put another push on that by the end of the week.

[calavera]

That's very awesome progress @miere, thanks for the update!

I'm not concerned about breaking changes because we're still in a 0.x release, and those should be expected. It should be ok as long as we have clear examples.

Let me know if you need any help.

[miere]

I'm not concerned about breaking changes because we're still in a 0.x release, and those should be expected. It should be ok as long as we have clear examples.

Glad to hear that, mate. Will keep you posted through the weekend.

[miere]

Hi @calavera @softprops,

During the implementation of this enhancement, I've found something really disconcerting: there's an API leakage of http::Request on its contract signature.

When I look into frameworks and abstractions I often avoid those which exposes its internal dependencies, as it might introduce major impacts in my codebase in the mid/long term. For instance, if the library evolves and does not use http crate anymore I might force me to rewrite my code to comply with new imposed design.

I wanna hear your thoughts regarding a possible change on this signature. Perhaps we could simplify the lambda-http crate even further, by enforcing a contract that hides internal complexities from developers that consumes the API.

[miere]

I've also noticed that Body has been moved to aws_lambda_events. As we're now relying on this crate, I'd like to also propose that we move away from relying on aws_lambda_events::encodings::Body, allowing the HTTP payload to be dynamically inferred from the compile time. In this case, developers would literally do makes direct use of the generated events. Something like this:

use aws_lambda_events::event::alb:: AlbTargetGroupRequest as Request;
use netlify_lambda::{handler_fn, Context, LambdaResult, LambdaRuntimeResult};

#[tokio::main]
async fn main() -> lambda::RuntimeResult {
  netlify_lambda_http::run(|req: Request, ctx: Context| do_something_with(req)).await
}

I really believe this is an important move towards better maintainability in this runtime. ALB and APGW have different signatures, often being independently updated. Even though the whole wrapper about Request makes it behave like a single facade for both AWS services, it doesn't match the behaviour that happens on AWS SDKs from other languages.

It also doesn't provide a fully compatible API unless we put lots of effort into this. For instance, APGW is tightly coupled to AWS Cognito, something we can easily see in the ApiGatewayV2httpRequestContextAuthorizerIamDescription whereas ALB makes no direct referent to it.

Also, developers will eventually get familiar with aws_lambda_events for non-http events. It will more intuitive to make them rely only on a single crate for events instead of forcing them to memorise two different syntaxes.

[miere]

Implementing one or both of the abovementioned suggestions also implies on significantly reducing the codebase of this crate - hence reducing the compilation time. ;)

Let me know your thoughts so I can move further with this implementation.

[softprops]

There's an API leakage of http::Request on its contract signature.

some context here might be useful. This is actually the documented intent. The http crate is was extracted from hyper because libraries and frameworks built on to of it needed a stable interface that was generic enough to represent the needs of the majority of http centric applications and provided an standard interface for the rust http ecosystem. The choice to expose it was to embrace that design goal. This is not an internal implementation detail it is the interface :)

it might also be worth noting for context that lambda is not a framework as you might think of more traditional http frameworks in various ecosystems. It is the request handler of what would be a hosted generalization of what you commonly need to implement yourself, routing, metrics, request logging, auth, rate limiting are all provided by api gateway. Lambda ends up just be the component that can’t be generalized, the behavior that makes your app unique.

From a framework perspective you should only need to think about it’s event inputs and outputs. Since rust has a generalized stable and common http interface, the http crate, the lambda http crate just proves on layer of convenience, adapting the suite of http like events to a common rust http interface

[miere]

The choice to expose it was to embrace that design goal. This is not an internal implementation detail it is the interface :)

@softprops Thehttp crate is an abstraction at the protocol level, whereas AWS Lambda is an application-level abstraction. This is a subtle but important distinction that we shall keep in mind when designing abstractions. Our users must rely on the contract provided by AWS itself like it happens to be in the other SDKs.

It is the request handler of what would be a hosted generalization of what you commonly need to implement yourself, routing, metrics, request logging, auth, rate limiting are all provided by api gateway.

Bear in mind that AWS provides at least 2 ways of handing HTTP requests: Application Load Balancer and ApiGateway. Both comes with its own trade-offs and idiosyncrasies. When developers chose one over another, and decide to use a pure AWS Lambda runtime (which should be the case for this project), they want to take control of how things will work on their codebase. Otherwise, they would've chosen a more opinionated implementation for the job (like Rocket and Actix).

it might also be worth noting for context that lambda is not a framework as you might think of more traditional http frameworks in various ecosystems.

It still is a library (or a framework if you will) in a sense that it abstracts away how AWS Lambda internally works. In practice, runtimes shall hide internal representations allowing developers to focus on the business side. On the other hand, it should be transparent for them on how they interact with its desired components (e.g. listening to HTTP coming from ApiGateway or ALB).

And here lies the foundation of my reasoning: despite its similarities, ALB and APGW have different contracts, makes use of different features and have different life cycles. Developers looking for an official AWS SDK for Lambda shall be able to interact with both ALB and APGW (or any other similar service that AWS might include in the future) and take full advantage of their different features. And, if we choose the road of maintaining an abstraction layer for that, full advantage implies keep updating the software when either ALB or APGW is updated. In theory, we should update this crate when AWS Lambda is updated.

As I mentioned in other thread, I'm not against creating such abstraction - quite the opposite, I'd love to use one. I just don't think that the official AWS Lambda runtime should provide such a high-level abstraction. But, there's something really gets the eye in the lambda_http that, I believe, worth its existence: it automatically handles Errors. It's something I did by myself last year and I loved that we were on the same page on that. That's why, I believe, we could revisit the way we listen to Http requests, removing duplicated code base created for that and focusing mostly on the behaviours that would be common between Http Events: error and response handling.

We can't avoid libraries to be opinionated, but it should stick to its own existence responsibility and allow seamlessly to interoperate with different AWS components.

[softprops]

Our users must rely on the contract provided by AWS itself like it happens to be in the other SDKs.

I gotcha in that case you likely want to just use the default lambda crate which just accepts trigger events as they come whether provided by a service trigger or user defined.

The lambda-http crate was designed specifically for the case of rust http crate ecosystem. Which provides a common way to represent what would be a generalization over the two flavors of api gateway trigger events as well as alb.

As far as other sdks go, this is typically a compatibility layer for the more familiar standard http interfaces that exist within those language communities. For rust, that'd be the http crate.