[aws_lambda_events] No longer able to construct response types a lambda may return as a response (they are `non_exhaustive`)

[dcormier]

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 lambda
• AppSyncLambdaAuthorizerResponse (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.

[jlizen]

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.

[dcormier]

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)
}

[jlizen]

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?

We provide examples with the latest behaviour here:

• basic-s3-thumbnail
• basic-error-handling
• advanced-sqs-partial-batch-failures
• basic-s3-object-lambda-thumbnail

I will be updating the aws docs shortly as well.

[lmammino]

I’ve just run into the same issue and also found the current suggestion a bit unergonomic.

I’ve opened a proposal specifically for SqsBatchResponse in #1063 that might (hopefully) offer a more ergonomic alternative.

[jeastham1993]

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.

One 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?

[lmammino]

One 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()
)

[lmammino]

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_failures look 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()
            );
        }
    }

[lmammino]

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)

[lmammino]

I know it's re:Invent season and everyone is probably super busy with it but I am curious to know if there was any progress with the discussion here or any consideration given to #1063.

Thanks

[jlizen]

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)

I feel I need to address the changes and what we are trying to achieve.

First, we want to retain ergonomics, and second we want to lower the barrier for using the rust runtime.

To achieve those, we need to balance a) cost of maintaining the library — the [code] debt we own — b) cost of keeping the library up-to-date from user side, c) cost of updating the library from dev side, and d) cost of extending with more features.

To achieve that, we added Default and non-exhaustive. This came at a detriment to ergonomics. So to address that, and retain the same patterns used throughout AWS SDK, we will be adding builders.

Personally, I am a big fan of PRs like #1063 — I've done a few of those myself over the years in other projects — at the same time the cost of doing that for all structs, in all combinations that might be useful or interesting is high.

Now, that doesn't mean we are not open to those PRs. Rejecting contributions like this goes against our commitments to our users and our community.

So in keeping up with this spirit, I am looking for a process to facilitate this kind of contributions, and I will keep you posted :)