Bug: Incorrect error message for IdempotencyAlreadyInProgressError when sortKeyAttr is in use

[mikesnare]

Expected Behavior

When an IdempotencyAlreadyInProgressError is thrown, the error message should include the hashed key information.

Current Behavior

Currently, the error message only includes the partition key, but when a sortKeyAttr has been provided in the config the partition key is a static string. The leads to meaningless errors in the log that all say there's a IdempotencyAlreadyInProgressError error for the same key.

Code snippet

// none

Steps to Reproduce

Set up idempotency and provide a sortKeyAttr in the config, then send duplicate events in quick succession. You will get errors that reference only the partition key, but the partition key is static in this scenario and is therefore not very helpful for debugging.

Possible Solution

Include both the pk/sk, or detect the current scenario better and include the more meaningful one.

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

20.x

Packaging format used

npm

Execution logs

[boring-cyborg]

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #typescript channel on our Powertools for AWS Lambda Discord: Invite link

[dreamorosi]

Hi @mikesnare, thank you for opening the issue.

I have tested the case you described and the error message we emit does not include the sort key, which as you mentioned, makes the error unhelpful.

From a first look, fixing this will require a bit of effort since our abstraction didn't account for keeping track of the sort key at all.

This is to say that we might need some time and the fix might not be ready for the next release (this Friday).

I'll report back here once I have updates.

Also, copying @aws-powertools/lambda-python-core, @aws-powertools/lambda-java-core, and @aws-powertools/lambda-dotnet-core - as far as I can tell this affects all runtimes.

[phipag]

I checked the issue for Java and it looks like the Java runtime is not affected by this. The DynamoDBPersistenceStore checks whether the user specified a sortKeyAttr or not. If the user specified a sort key, it will use this as idempotencyKey for all usages across the code-base including the exception message.

• The idempotencyKey is set here based on whether a sort key is specified: https://github.com/aws-powertools/powertools-lambda-java/blob/main/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/persistence/DynamoDBPersistenceStore.java#L272
• And it is retrieved for the exception message here: https://github.com/aws-powertools/powertools-lambda-java/blob/394ab0ccfc99488fafeea4497e4e6df79cb75047/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyHandler.java#L161-L162

[dreamorosi]

Thank you for reporting @phipag - looks like @aws-powertools/lambda-dotnet-core is doing the same thing, so it might not be affected. I wasn't able to find the same for Python but I haven't tested it.

Regardless, I wanted to ask everyone's opinion on what would be the desired state for the error message.

Current status

There is already an execution in progress with idempotency key: idempotency#idemsk-aamorosi-IdempotencyFunction-onenncmn

As we have established, today in TS (& potentially Python) we include only the primary key which is not helpful.

Option 1 - Java/.NET - SK only

If we were to follow .NET/Java's implementations we would end up with a message like this:

There is already an execution in progress with idempotency key: idemsk-aamorosi-IdempotencyFunction-onenncmn#secAC8juGo8Z7F3rbCs37w==

This is the sort key only. While it's not enough to query the item from DynamoDB by itself, knowing that the primary key is static and equivalent to the static prefix (idempotency by default) + the function name, operators would have enough to go by.

Option 2 - Include both

A third option would be to modify all implementations so they include both primary and sort key, so that the message would be something like:

There is already an execution in progress with idempotency key: idempotency#idemsk-aamorosi-IdempotencyFunction-onenncmn and sk idemsk-aamorosi-IdempotencyFunction-onenncmn#secAC8juGo8Z7F3rbCs37w==

This would be the most unambiguous of the three in my opinion, but as I was alluding to earlier, it might require us revisiting some of the internal abstractions (IdempotencyRecord) and potentially making them less generic.

---

Option 1 is less exhaustive but quick to implement. Option 2 provides all the info needed to unequivocally query an item, but it might require more effort/changes to implement.

What do we think it's the best path forward?

[mikesnare]

First of all, thank you, @dreamorosi for looking into this so quickly. As a user of the package, I don't see a choice but to go with Option 2 even if it means a delay in getting the fix out (this is the first report, so it doesn't look like it's such a serious issue that waiting on the "correct" fix would be a problem). This is an error message, and an error message should be actionable. I don't think users should need to take half the keyset and then try to reverse engineer the other half in order to find the offending records -- even if the pk is static. My 2 cents, but I appreciate whatever fix you all come up with. Thanks again!