Bug: IdempotencyItemAlreadyExistsError loses referential identity when IdempotencyConfig is imported via external pkg

[mrerichoffman]

Expected Behavior

When using the Idempotency utility with ddb persistence, the utility should return the result of a previous operation from cache provided that the request is handled by the same execution environment.

Current Behavior

When using ddb persistence the utility doesn't return the correct value from the cache, but instead causes a runtime error. If I revert from v2.1.0 to v1.17.0 the bug goes away.

Code snippet

import { IdempotencyConfig } from "@aws-lambda-powertools/idempotency";
import { DynamoDBPersistenceLayer } from "@aws-lambda-powertools/idempotency/dynamodb";
import { makeHandlerIdempotent } from "@aws-lambda-powertools/idempotency/middleware";
import middy from "@middy/core";

export interface ProcessResult {
  processResult: "processed" | "rejected";
}

const getDefaultIdempotencyConfigs = (eventKeyJmesPath = '["correlationId"]') => {
  const persistenceStore = new DynamoDBPersistenceLayer({
    tableName: process.env.IDEMPOTENCY_TABLE_NAME || "IdempotencyTable",
    sortKeyAttr: "sortKey"
  });
  const idempotencyConfig = new IdempotencyConfig({
    throwOnNoIdempotencyKey: true,
    eventKeyJmesPath
  });
  return {
    persistenceStore,
    idempotencyConfig
  };
};

const { persistenceStore, idempotencyConfig } = getDefaultIdempotencyConfigs();

const lambdaHandler = async (event: Record<"string", unknown>): Promise<ProcessResult> => {
  console.info("event", event);
  const processResult: ProcessResult = {
    processResult: "processed"
  };

  return processResult;
};

export const processLambda = middy(lambdaHandler).use(
  makeHandlerIdempotent({
    persistenceStore,
    config: idempotencyConfig
  })
);

Steps to Reproduce

Then run the function with the same event payload, { "correlationId": "abc-123-def-456" }, twice and observe the error (shown below) being thrown at the second execution.

Possible Solution

No response

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

20.x

Packaging format used

npm

Execution logs

2024-05-10T22:01:01.282Z	8034ee17-fd14-4244-8603-79500af0f7cb	ERROR	{
    "_logLevel": "error",
    "msg": "Failed to save in progress record to idempotency store. This error was caused by: Failed to put record for already existing idempotency key: svc-quick-list-name-dev-callListenerNameRules#7Hd4wd4mD/EQSj89aUna6A==.",
    "stack": "Error: Failed to save in progress record to idempotency store. This error was caused by: Failed to put record for already existing idempotency key: svc-quick-list-name-dev-callListenerNameRules#7Hd4wd4mD/EQSj89aUna6A==.\n    at IdempotencyHandler.<anonymous> (/node_modules/@aws-lambda-powertools/idempotency/lib/esm/IdempotencyHandler.js:92:27)\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\n    at IdempotencyHandler.handleMiddyBefore (/node_modules/@aws-lambda-powertools/idempotency/lib/esm/IdempotencyHandler.js:242:32)\n    at runMiddlewares (/node_modules/@middy/core/index.js:160:21)\n    at runRequest (/node_modules/@middy/core/index.js:114:9)",
    "_tags": []
}

[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 @mrerichoffman - thank you for opening the issue & providing the example.

I'm going to try to reproduce the issue on my end and will be back to you!

[dreamorosi]

Hi, I have been looking at reproducing this and I was not able to do so.

Running your exact code on a function backed by a DynamoDB table using composite primary key I was able to execute the function more than once using the same payload and observe that subsequent requests are being considered idempotent (see screenshot below):

In the sentence above I specifically called out that the DynamoDB table uses composite primary key, meaning the configuration provided to the DynamoDBPersistenceLayer class expects a partition key of type string and with name id (default), and a sort key also of type string but with name sortKey (as specified in your code using the sortKeyAttr option).

This is a non standard setup, and if you have deployed a Table using the configuration suggested in our docs, then the key schema would be with a partition key named id only. If I run your code using the default configuration I get a different runtime error Failed to update success record to idempotency store. This error was caused by: The provided key element does not match the schema, which is still the not the one you're seeing.

---

With this in mind, is there any other info that you think could be relevant in helping me reproduce the error consistently?

Alternatively, I also want to call out that this error is expected when two concurrent identical in-flight requests are being handled. As the error states, if a request arrives while a previously received identical one is still in progress, any subsequent one is rejected. This flow is detailed in this diagram in the docs and is also explained here.

If your first request failed to process and the idempotency record remained in an "in progress" state, subsequent requests made within what we call "in progress expiration" time are also rejected. This time is set to the remaining time in milliseconds before the invocation will timeout. In other words, if a request starts being processed while the function has 100 seconds remaining, the corresponding idempotency record will be placed as "in progress" with an in_progress_expiration equal to that. Any identical request that comes in that time frame while the item is still marked as "in progress" will be considered as "in flight".

This locking mechanism is necessary due to the distributed nature of Lambda, since subsequent identical requests have no way of knowing whether the first original request is still being processed (i.e. long running process) or has failed/timed out. After that in_progress_expiration has passed however, we can safely assume that if the item is still marked as "in progress", then it must have failed since by that time the Lambda function that was processing would have definitely timed out.

After that, if a new identical request comes it will be accepted and processed accordingly.

[mrerichoffman]

Sorry, I must have tested the provided sample code before I ripped out additional code to simplify testing. As it turns out the bug is reproducable when getDefaultIdempotencyConfigs is externalized into a separate npm package which is bundled to commonjs. To fix I removed the idempotency utility from the npm package and exported json configurations instead.

[dreamorosi]

Thanks for letting me know.

Just to make sure I understand, could you share an example of how to reproduce the issue?

I'd like to see if there's anything that we can either improve in the code or docs to avoid others experiencing the same problem.

Thanks!

[mrerichoffman]

Sure, I externalized the idempotency configs into an example npm package that can be installed from the attached tarball.

Updated lambda handler:

import { makeHandlerIdempotent } from "@aws-lambda-powertools/idempotency/middleware";
import middy from "@middy/core";
import { getDefaultIdempotencyConfigs } from "example-config-package";

export interface ProcessResult {
  processResult: "processed" | "rejected";
}

const { persistenceStore, idempotencyConfig } = getDefaultIdempotencyConfigs();

const lambdaHandler = async (event: Record<"string", unknown>): Promise<ProcessResult> => {
  console.info(`event`, { event });
  const processResult: ProcessResult = {
    processResult: "processed"
  };

  return processResult;
};

export const testingPowertool = middy(lambdaHandler).use(
  makeHandlerIdempotent({
    persistenceStore,
    config: idempotencyConfig
  })
);

npm package example-config-package referenced in import { getDefaultIdempotencyConfigs } from "example-config-package";

import { IdempotencyConfig } from "@aws-lambda-powertools/idempotency";
import { DynamoDBPersistenceLayer } from "@aws-lambda-powertools/idempotency/dynamodb";

export const getDefaultIdempotencyConfigs = (eventKeyJmesPath = '["correlationId"]') => {
  const persistenceStore = new DynamoDBPersistenceLayer({
    tableName: process.env.IDEMPOTENCY_TABLE_NAME || "IdempotencyTable",
    sortKeyAttr: "sortKey"
  });
  const idempotencyConfig = new IdempotencyConfig({
    throwOnNoIdempotencyKey: true,
    eventKeyJmesPath
  });
  return {
    persistenceStore,
    idempotencyConfig
  };
};

Place the tgz packed library in the root of the project.
example-config-package-v1.0.0.tgz

Reference the tzg file in package.json dependencies like this - "example-config-package": "file:./example-config-package-v1.0.0.tgz"

[dreamorosi]

Hi, I have been looking into this and I am able to reproduce the issue when I use the tarball you provided but not when I bundle/build the same utility on my end.

When deploying my function using your example-config-package and bundle it with esbuild I noticed that the Idempotency package gets included twice, once because it's pulled in by the handler, and once more because it's pulled in by the config package. This is kind of strange.

Despite this, I haven't been able to pinpoint the issue yet and due to the fact that it appears only under these specific conditions I am not sure how to troubleshoot it properly.

I'll still give it a try and get back to you.

[dreamorosi]

I'm back from PTO but this week I will be attending the AWS Summit in Madrid (Spain). I'll start looking at this as soon as I can but just to manage expectations - I might need a few more days.

[dreamorosi]

Hi @mrerichoffman, thank you for your patience.

I have been looking at this on & off for the past couple days, built an isolated version of the config package, and then finally resorted to debug the esbuild output and I finally think I have found the root cause.

The error we were both observing was consistently happening in subsequent invocations - aka any request that should've been considered idempotent. This led me to narrow down the root cause to this try/catch block here.

After stepping through the built code, I noticed that the error being caught in that block was failing the e instanceof IdempotencyItemAlreadyExistsError check and instead throwing a generic error that also caused the utility to delete the record as if the invocation had been considered failed altogether.

To explain why this instanceof check was failing, we need to look into how the bundled output looks like.

With the current setup, the "import tree" of the dependency looks something like this:

• index.js (aka the function)
  • @aws-lambda-powertools/idempotency
  • example-config-package
    • @aws-lambda-powertools/idempotency
  • @middy/core
  • ...

Due to this, once bundled, some of the objects from the Idempotency utility lose their referential identity. Specifically, observing the final bundle we can see that the errors from here, as well as other objects are present twice in the bundle.

We know that this is not a desirable outcome because we have authored both the index.js file (entry point) as well as the example-config-package, as well as having set the shared module as peer dependency - however esbuild doesn't know this and is not in a position to make the assumption that these are indeed the same package.

This is good overall, even though in this specific instance it leads to some extra code being imported. There are ways to go around this, but they require using esbuild plugins and by definition this is a space where Powertools for AWS Lambda should not have an opinion, so I'll leave this as an exercise to the reader - you can find more info here, where the author of the bundler also explains more in details why the tool behaves the way it does.

As a proof of this, I also marked the @aws-lambda-powertools/* modules as external when bundling my index.js entry point, and then deployed it while also attaching the Powertools for AWS Lambda layer to the function.

In this case esbuild was explicitly instructed to skip the Idempotency utility (as well as others), and at runtime both the index.js file and the example-config-package were importing @aws-lambda-powertools/idempotency from the Layer, and thus using the same copy of the utility. When doing this, the function ran just fine and the error we were seeing did not occur.

---

In terms of solutions, as I mentioned there are esbuild plugins that customers such as you and with your setup could look into, however I think there's something that we can also do to at least partially mitigate the issue.

For example, I'm inclined to update the error classes of the utility to have a more specific name property, for example this error would become like this:

/**
 * Item attempting to be inserted into persistence store already exists and is not expired
 */
class IdempotencyItemAlreadyExistsError extends Error {
  public existingRecord?: IdempotencyRecord;

  public constructor(message?: string, existingRecord?: IdempotencyRecord) {
    super(message);
+   this.name = 'IdempotencyItemAlreadyExistsError';
    this.existingRecord = existingRecord;
  }
}

so that we could then refactor the identity check within the try/catch to something like this:

try {
  await this.#persistenceStore.saveInProgress(
    this.#functionPayloadToBeHashed,
    this.#idempotencyConfig.lambdaContext?.getRemainingTimeInMillis()
  );

  return returnValue;
} catch (e) {
-  if (e instanceof IdempotencyItemAlreadyExistsError) {
+  if (e.name === 'IdempotencyItemAlreadyExistsError') {

With this change, even though we would still not preventing esbuild to include duplicate copies of some of the objects (something we can't really do anyway), we would be able to unblock use cases like yours since the check would no longer depend on referential identity but rather on the value of a property within the object.

I have tried this change with a custom build of the Idempotency utility and I was able to confirm that it solves the issue with your setup.

I will work on a PR tomorrow and I'm positive that we should be able to include it in the next release.

[dreamorosi]

As a side note to the above, another way to avoid duplicating some of the objects and guide esbuild towards recognizing they are the same, would be to align the authoring imports to the ones in the bundled package.

As of now, looking at the tarball you have provided you are authoring the package using import statements, but then building it (using esbuild) as a commonjs module, which converts the imports to require calls.

In the index.js entry point of the function, we are once again using import, which led the bundler to import some of the objects as ESM, even though the bundling output was set to CJS.

By changing the index.js file to this:

- import { makeHandlerIdempotent } from "@aws-lambda-powertools/idempotency/middleware";
+ const {
+  makeHandlerIdempotent,
+ } = require('@aws-lambda-powertools/idempotency/middleware');
import middy from "@middy/core";
import { getDefaultIdempotencyConfigs } from "example-config-package";

export interface ProcessResult {
  processResult: "processed" | "rejected";
}

const { persistenceStore, idempotencyConfig } = getDefaultIdempotencyConfigs();

const lambdaHandler = async (event: Record<"string", unknown>): Promise<ProcessResult> => {
  console.info(`event`, { event });
  const processResult: ProcessResult = {
    processResult: "processed"
  };

  return processResult;
};

export const testingPowertool = middy(lambdaHandler).use(
  makeHandlerIdempotent({
    persistenceStore,
    config: idempotencyConfig
  })
);

as well as changing the import in the example-config-package package to this (which requires a change on Powertools side):

- import { IdempotencyConfig } from "@aws-lambda-powertools/idempotency";
+ import { IdempotencyConfig } from "@aws-lambda-powertools/idempotency/config";
import { DynamoDBPersistenceLayer } from "@aws-lambda-powertools/idempotency/dynamodb";

export const getDefaultIdempotencyConfigs = (eventKeyJmesPath = '["correlationId"]') => {
  const persistenceStore = new DynamoDBPersistenceLayer({
    tableName: process.env.IDEMPOTENCY_TABLE_NAME || "IdempotencyTable",
    sortKeyAttr: "sortKey"
  });
  const idempotencyConfig = new IdempotencyConfig({
    throwOnNoIdempotencyKey: true,
    eventKeyJmesPath
  });
  return {
    persistenceStore,
    idempotencyConfig
  };
};

the bundled output now includes only one copy of the error classes, which means the instanceof check is now successful:

While I haven't tested this, I suspect that changing the build script of the example-config-package to output ESM, and using ESM for the main function as well, would yield a similar result.

Despite this, which is ultimately a case of dual package hazard, I'm still going to make the change to use branded errors (aka using error.name) since when authoring in TypeScript the widely adopted convention is to just use import statements throughout, and regardless of the target output.

[github-actions]

⚠️ COMMENT VISIBILITY WARNING ⚠️

This issue is now closed. Please be mindful that future comments 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.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

[github-actions]

This is now released under v2.4.0 version!