Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent lambda timeouts or failure from not handled promise due to invalid SSM parameter(s)... Add throwOnFailedCall flag to the SSM middleware #572

Merged
merged 2 commits into from
Nov 2, 2020

Conversation

bokjo
Copy link

@bokjo bokjo commented Oct 31, 2020

What does this implement/fix? Explain your changes.


PR regarding #567

If you provide a non-existent SSM parameter to the @middy/ssm middleware it will cause the function to go to timeout if you are not using @middy/do-not-wait-for-empty-event-loop, or with do-not-wait-for-empty-event-loop in place, it will fail immediately (the error is thrown in the handleInvalidParams() function but it is not handled anywhere!) and not reach the original handler!

Changes:

  • Added optional throwOnFailedCall flag with default value false (Same implementation as Secret Manager middleware)
  • Added catch to the main Promise.all(ssmPromises)
    • If throwOnFailedCall=false it will just console.error the error and continue to the next middleware or main handler
    • If throwOnFailedCall=true it will console.error the error and throw the error thrown from handleInvalidParams()

p.s tested this implementation on AWS lambda with the code below... works as expected!

const middy = require('@middy/core')
const doNotWaitForEmptyEventLoop = require('@middy/do-not-wait-for-empty-event-loop')
const httpEventNormalizer = require('@middy/http-event-normalizer')
const httpSecurityHeaders = require('@middy/http-security-headers')
const ssm = require('@middy/ssm')

const hello = async (event, context) => {
  console.log('EVENT: ', event)
  console.log('CONTEXT: ', context)
  
  const { testSecret } = context
  console.log('testSecret: ', testSecret)

  return {
        statusCode: 200,
        body: JSON.stringify(context)
  }
}

module.exports.default = middy(hello)
  .use(doNotWaitForEmptyEventLoop({ runOnError: true }))
  .use(httpSecurityHeaders())
  .use(httpEventNormalizer())
  .use(
    ssm({
      cache: true,
      cacheExpiryInMillis: 60 * 1000, 
      names: {
        testSecret: process.env.SSM_PARAM
      },
      setToContext: true,
      throwOnFailedCall: false
    })
  )

Does this close any currently open issues?

No

Any relevant logs, error output, etc?

(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)

Any other comments?

@willfarrell let me know if something else is needed! Don't have much time to check the documentation for do-not-wait-for-empty-event-loop

Some additional tests might be needed for the new throwOnFailedCall flag!

Where has this been tested?

Node.js Versions: : Node v12.18.3 (npm v6.14.6)

Middy Versions: : v1.4.0

AWS SDK Versions:: ^2.771.0

Todo list

  • Feature/Fix fully implemented
  • Added tests
  • Add additional tests for throwOnFailedCall if needed
  • Updated relevant documentation
  • Updated relevant examples

…or if there is invalid param. or just prints the error and continues forward to the next middleware or original handler
@willfarrell willfarrell merged commit 9dbedcc into middyjs:master Nov 2, 2020
@willfarrell
Copy link
Member

Looks good. It will be part of v1.5.0. Just waiting on another PR related to ssm first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants