Repository navigation
[aws_lambda_events] No longer able to construct response types a lambda may return as a response (they are non_exhaustive) #1060
Description
Activity
Hi @dcormier , the intended API now when directly constructing would be to use
..Default::default()to guard against unknown fields. Does that work for you? I do think it would be good to have some examples demonstrating this...These were changed because it was constantly forcing semver bumps every update and was forcing consumers to either update their code or be frozen in time.
Modifying an existing rust example from AWS's docs to this:
async fn function_handler(event: LambdaEvent<SqsEvent>) -> Result<SqsBatchResponse, Error> { let mut batch_item_failures = Vec::new(); for record in event.payload.records { match process_record(&record).await { Ok(_) => (), Err(_) => batch_item_failures.push(BatchItemFailure { item_identifier: record.message_id.unwrap(), ..Default::default() }), } } Ok(SqsBatchResponse { batch_item_failures, ..Default::default() }) }
Results in this:
error[E0639]: cannot create non-exhaustive struct using struct expression --> src/main.rs:18:48 | 18 | Err(_) => batch_item_failures.push(BatchItemFailure { | ________________________________________________^ 19 | | item_identifier: record.message_id.unwrap(), 20 | | ..Default::default() 21 | | }), | |_____________^ error[E0639]: cannot create non-exhaustive struct using struct expression --> src/main.rs:25:8 | 25 | Ok(SqsBatchResponse { | ________^ 26 | | batch_item_failures, 27 | | ..Default::default() 28 | | }) | |_____^ For more information about this error, try `rustc --explain E0639`.Instead, you have to abandon struct literals entirely, which is less than ideal:
async fn function_handler(event: LambdaEvent<SqsEvent>) -> Result<SqsBatchResponse, Error> { let mut batch_item_failures = Vec::new(); for record in event.payload.records { match process_record(&record).await { Ok(_) => (), Err(_) => { let mut item = BatchItemFailure::default(); item.item_identifier = record.message_id.unwrap(); batch_item_failures.push(item) } } } let mut response = SqsBatchResponse::default(); response.batch_item_failures = batch_item_failures; Ok(response) }
Ah, my mistake. This is running afoul of: rust-lang/lang-team#143 , which doesn't seem to have much traction.
It sounds like at minimum we should update the official example to compile properly (@PartiallyUntyped ).
But, agreed that abandoning struct literals is not ideal. This is an awkward situation, since we definitely can't maintain the library with reasonable stability without
#[non_exhaustive]Perhaps some sort of non-default feature flag like
features = ["experimental-non-exhaustive"]that has lesser semver guarantees?Yep, encountered the same issue here. I agree with some of the previous comments, but I think the new way of building responses is not intuitive to use.
Reacted by Luciano MamminoReacted by Luciano MamminoOne of the options we are considering is providing automatically generated builders; the pattern will follow the same practices AWS SDK provides, in that you create a builder, set the fields you need, and then call
build().Do you think that might improve ergonomics?
It might be a step forward, but I suspect the code might still be quite verbose in practice. It would be nice so see some examples for what you have in mind.
Is there an argument against crafting finely tuned utility functions like the ones proposed in #1063?
Based on the PR, the code would look something like this:
Ok( SqsBatchResponse::builder() .batch_item_failures(batch_item_failures) .build() )Thanks @PartiallyUntyped! 🙌🏽
Doesn't sound too bad, but what does building
batch_item_failureslook like?It should look something along the lines of:
for record in event.payload.records {
let message_id = record.message_id.clone().unwrap_or_default();// Try to process the message if let Err(e) = process_record(&record).await { println!("Failed to process message {}: {}", message_id, e); // Add to failures list so it will be retried batch_item_failures.push( SqsBatchItemFailure::builder() .item_identifier(message_id) .build() ); } }Looks pretty good to me! 🎉
Probably a little bit more verbose than what I am suggesting with #1063, but I get that this is probably more idiomatic (and easier to generate rather than having to write all of it manually)
Hey @lmammino , @PartiallyUntyped is currently OOO so the "codegenned automatic builders" will take some time.
Personally, it seems fine to me to also have custom, hand-written, even-more-ergonomic constructors for types where desired, such as #1063. Need to discuss with other maintainers but I'll at least give the PR a read-through in the meantime to reduce future churn.
(EDIT: Yeah, I got everybody's blessing to review #1063 as-is regardless of codegen plans)
Share a use case:
Cannot create
ApiGatewayV2CustomAuthorizerSimpleResponse<MyContext>ifMyContexthas noDefault.use aws_lambda_events::event::apigw::{ ApiGatewayV2CustomAuthorizerSimpleResponse, ApiGatewayV2CustomAuthorizerV2Request, }; use lambda_runtime::LambdaEvent; pub(crate) async fn function_handler( event: LambdaEvent<ApiGatewayV2CustomAuthorizerV2Request>, ) -> Result<ApiGatewayV2CustomAuthorizerSimpleResponse<MyContext>, lambda_runtime::Error> { let mut output: ApiGatewayV2CustomAuthorizerSimpleResponse<MyContext> = ApiGatewayV2CustomAuthorizerSimpleResponse::default(); output.is_authorized = true; output.context = MyContext { some_thing_always_exists: Some(todo!("create a SomeThirdPartyThingWithoutDefaultValue")), }; Ok(output) } /// An authorizer's response context which can be shared with another rust written lambda behind API Gateway. #[derive(Debug, Default, Serialize, Deserialize)] pub struct MyContext { // NOT IDEAL HERE: // // Need to warp `SomeThirdPartyThingWithoutDefaultValue` with `Option` to fulfill `Default` requirement // or else cannot create a `ApiGatewayV2CustomAuthorizerSimpleResponse<MyContext>`. some_thing_always_exists: Option<SomeThirdPartyThingWithoutDefaultValue>, }
Consider this case, I guess force user rely on
Defaultto initialize event has some limitation in type level. Maybe builder pattern can be a rescue? I'm not so sure.Reacted by Jess IzenReacted by Daniel Cormier++ to @ikai104 's use case, clearly we need an escape hatch.
I do think the builder approach COULD address this problem, but it would require a more complicated type state pattern to enforce that any field that does not have a default implementation, is required prior to finishing the builder.
Another option would be to add more ergonomic constructors similar to the ones that recently landed in: #1063
These would let us have constructors that specifically accept eg
contextwithout requiring it to implementDefault, while still also using default for other fields.A third option would be some sort of
unstable_allow_struct_initializationfeature, which we use to apply#[cfg_attr(not(feature="unstable_allow_struct_initialization"), non_exhaustive)]. In other words, we expose a feature that allows freeform struct initialization, and just relax any semver guarantees for that feature to allow breaking changes on struct fields across semver-compatible versions.This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.Reopening since we are a ways off from releasing the new builder patterns (as we are in
-rcterritory still due to the managed instance feature being tested).Also interested in whether the recently added builder pattern addresses your use cases @lmammino @ikai104 @dcormier
(I'm working on getting a new release candidate out so that you can kick the tires...)
Reacted by ManuelReacted by Daniel CormierPerhaps the events library can be released by itself without a new RC?
Reacted by Daniel CormierPerhaps the events library can be released by itself without a new RC?
Ah yeah, good point. Let me get the ball rolling on that.
Reacted by ManuelReacted by Daniel CormierThe builders went out in [email protected].
This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
A number of the types in this crate are responses that a lambda might return. With the recent change to mark structs as
non_exhaustive, these types can no longer be constructed. Some examples:SqsBatchResponse(and its child types) are returned from an SQS lambdaAppSyncLambdaAuthorizerResponse(and its child types) are returned from an AppSync authorizer lambda.ApiGatewayCustomAuthorizerResponse(and its child types) are returned from an API Gateway authorizer lambda.I'm sure there are other response types in this crate that lambdas would construct, but these are the ones I'm currently constructing and am no longer able to after they've been marked
non_exhaustive.