aws-powertools / powertools-lambda-typescript

Powertools is a developer toolkit to implement Serverless best practices and increase developer velocity.
https://docs.powertools.aws.dev/lambda/typescript/latest/
MIT No Attribution
1.56k stars 136 forks source link

Bug: AppConfig value retrieval fails when there is more than 24 hours between calls #1798

Closed mareksnincak closed 8 months ago

mareksnincak commented 10 months ago

Expected Behaviour

AppConfigProvider get method shouldn't fail, when there is more than 24 hours between calls.

Current Behaviour

AppConfigProvider get method fails, when there is more than 24 hours between calls.

Cause:

  1. token is set in https://github.com/aws-powertools/powertools-lambda-typescript/blob/v1.16.0/packages/parameters/src/appconfig/AppConfigProvider.ts#L316
  2. token is reused in https://github.com/aws-powertools/powertools-lambda-typescript/blob/v1.16.0/packages/parameters/src/appconfig/AppConfigProvider.ts#L289 even when it already expired - according to https://docs.aws.amazon.com/appconfig/latest/userguide/appconfig-retrieving-the-configuration.html tokens are only valid for 24 hours

Code snippet

const appConfigProvider = new AppConfigProvider({
  application: "my-app",
  environment: "my-env",
  clientConfig: { region: "us-east-1" },
});

await appConfigProvider.get("my-config");

// Wait more than 24 hours...

await appConfigProvider.get("my-config");

Steps to Reproduce

  1. Initiate app config provider
  2. call get method to retrieve value - this will cache the token from GetLatestConfiguration command
  3. wait more than 24 hours - without killing the environment and calling get method again
  4. call get method to retrieve value - this will reuse expired token from previous call

Possible Solution

Set cache to expire after e.g. 23 hours - cache retrieval in https://github.com/aws-powertools/powertools-lambda-typescript/blob/v1.16.0/packages/parameters/src/appconfig/AppConfigProvider.ts#L289 should be updated to not return expired token.

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

18.x

Packaging format used

npm

Execution logs

No response

boring-cyborg[bot] commented 10 months ago

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 commented 10 months ago

Hi @mareksnincak thank you for opening the issue.

One question: could you please expound on how do you achieve point number 3 and keep the Lambda execution environment active for all these hours?

mareksnincak commented 10 months ago

Hi, in this case I've been using it in non-lambda environment, but this are the scenarios with lambda, where I think it may happen:

  1. you use lambda with provisioned concurrency
  2. lambda is called periodically, so it doesn't terminate and you have this appconfig retrieval in some conditional logic that is not normally used. After >24 hours, you trigger this logic and it fails
  3. some anomaly like it's mentioned in here in the results section:

    "Incidentally, there was a period during the experiment week in which instances in eu-west-1 were not terminated every two hours. This resulted in three instances that ran for 48 hours. These exceptionally long lifetimes were omitted in calculating the averages and standard deviations, as such a thing didn’t happen during the other experiments, and also not in the other two regions."

dreamorosi commented 10 months ago

I see, makes sense.

Thanks for proving the info, we'll look into it and try to reproduce the issue.

We'll report back here as soon as we have more info to share. The team is attending re:Invent next week so we might take longer than normal to triage the issue.

dreamorosi commented 8 months ago

Hi @mareksnincak, apologies for the long wait. Between re:Invent and annual leaves I was just now able to get back to this.

After looking into this, while I didn't directly reproduce the issue, I was able to confirm that due to the token lasting only up to 24 hrs whenever the next call to AppSync happens later than that, it is expected to result in a bad request.

To guard against this occurrence I am going to open a pull request that changes the implementation to store the expiration timestamp together with the token. We will use the expiration timestamp to check if the token is still valid before retrieving a configuration, and if the token has expired, we'll start a new session and get a new token.

github-actions[bot] commented 8 months ago

⚠️ COMMENT VISIBILITY WARNING ⚠️

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. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] commented 8 months ago

This is now released under v1.18.0 version!