aws-samples / serverless-rust-demo

Sample serverless application written in Rust
MIT No Attribution
267 stars 21 forks source link

'Missing Id' in DynamoDBEvent parsing - eventVersion 1.1 instead of eventVersion1.0? #26

Closed mostalive closed 2 years ago

mostalive commented 2 years ago

Thank you for putting this project together - I'm learning Rust (I have some background with Haskell and Purescript), and love tests and hexagons. I have no experience with DynamoDBEvents, nor investigating issues with it, so I may be seeing this wrong, any suggestions appreciated.

Steps to reproduce:

Expected result:

Actual result:

From a CloudWatch log:

{"timestamp":"Nov 16 10:03:52.779","level":"INFO","fields":{"message":"Transform events"},"target":"products::entrypoints::lambda::dynamodb","span":{"name":"parse_events"},"spans":[{"name":"parse_events"}]}
49
2021-11-16T10:03:52.779+00:00
{"timestamp":"Nov 16 10:03:52.779","level":"ERROR","fields":{"message":"InternalError: Missing id"},"target":"lambda_runtime"}

Possible fix: Also (or only?) allow lowercased field names (price, name and id), and parse ApproximateCreationDateTime as a float instead of an int.

I changed src/entrypoints/lambda/dynamodb/mod.rs to print the incoming JSON value (I had to change the type to Value, because parsing changes the casing to be snake_case. not complaining, but makes extracting a unit test harder) and then copied that into a test in that mod.rs .

The existing unit tests in src/entrypoints/lambda/dynamodb/model.rs have "eventVersion": "1.0", the incoming JSON had "eventVersion": "1.1". I have searched, but not been able to find documentation for the various versions.

I don't understand why the casing of the filenames changes, as they are part of the dynamodb model. Full messy branch here, this now works for me and sends messages to EventBridge. It is not ready for a pull request, I still don't understand what is going on here. Just curious about your ideas.

nmoutschen commented 2 years ago

Hey @mostalive ! Thanks for spotting this. I was able to reproduce on my side. It looks like I'm trying to extract field names with a capital letter instead of all-lowercase ("Price" versus "price"). Running tests now to see if it fixes the issue and will push a change.

As for the eventVersion, I'm unsure what's the difference there. I think version 1.1 has been going on for a long time (this test event for the Go runtime dates back to 2018), and I just happened to use 1.0 by accident.

nmoutschen commented 2 years ago

Hey @mostalive ! I've merged the fix into the main branch. Could you tell me if it's working for you?

mostalive commented 2 years ago

Hi @nmoutschen, deployed a fresh stack to eu-central-1 and it works (events dispatched to EventBridge, no errors in the log), thank you!

FYI I wonder if the integration tests could stretch to EventBridge (just wondering, I'm probably going to take inspiration from the sample code and send a mail with SES instead, don't need more at the moment).

nmoutschen commented 2 years ago

Thanks a lot! Yep, ideally I'd like to add an integration test to EventBridge, but there are a few things that makes it more complicated. EventBridge doesn't expose a listener API, so I would need to create a test harness, such as an SQS queue listening to events from this sample, and then check if the events are sent in the queue.