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

(aws-sam): DestinationConfig contains onSuccess property even though it's not part of SAM spec #16739

Closed
GaalDornick opened this issue Sep 30, 2021 · 4 comments
Assignees
Labels
@aws-cdk/aws-sam Related to AWS Serverless Application Model bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@GaalDornick
Copy link

The SAM spec specifies a property called OnSuccess here (

). However, SAM documentation doesn't specify an OnSuccess property (https://github.com/aws/serverless-application-model/blob/master/versions/2016-10-31.md#destination-config-object)

Reproduction Steps

test("correctly creates a function invoked from a dynamodb stream", () => {
 const stack = new cdk.Stack();

  new sam.CfnFunction(stack, 'MyFunction', {
    codeUri: {
      bucket: 'my-bucket',
      key: 'my-key',
    },
    runtime: 'nodejs-12.x',
    handler: 'index.handler',
    policies: ['AWSLambdaExecute'],
    events: {
        MyStream: {
          type: "DynamoDB",
          properties: {
            enabled: true,
            startingPosition: "LATEST",
            stream: "arn:aws:dynamodb:us-east-1:123456789012:table/mytable/stream/2021-08-20T02:52:27.890",
            batchSize: 100,
            maximumBatchingWindowInSeconds: 60, // wait 60 seconds for records to reach batchSize
            maximumRetryAttempts: 2,
            maximumRecordAgeInSeconds: -1, // TODO: Figure out if we should set this value
            bisectBatchOnFunctionError: true,
            destinationConfig: {
              onFailure: {
                destination: "arn:aws:sqs:us-east-1:123456789012:myDLQ.fifo",
              },
              // workaround for possible bug in CDK
              // CDK requires this attribute but doesn't do anything with it
              //onSuccess: {
               // destination: procurementConverterDLQ.dlq.queueArn,
              //},
            },
          },
        },
     }
  });

What did you expect to happen?

I am trying to create a FUnction that gets triggerred from DDB stream. THe above code compiled in v1.106.0

What actually happened?

The code doesn't compile in v1.122.0. The error that I get is

test/function.test.ts:52:13 - error TS2322: Type '{ onFailure: { destination: string; }; }' is not assignable to type 'IResolvable | DestinationConfigProperty | undefined'.
  Property 'onSuccess' is missing in type '{ onFailure: { destination: string; }; }' but required in type 'DestinationConfigProperty'.

52             destinationConfig: {
               ~~~~~~~~~~~~~~~~~

  lib/sam.generated.ts:2812:18
    2812         readonly onSuccess: CfnFunction.DestinationProperty | cdk.IResolvable;
                          ~~~~~~~~~
    'onSuccess' is declared here.

Environment

  • CDK CLI Version :
  • Framework Version:
  • Node.js Version:
  • OS :
  • Language (Version):

Other

The DestinationConfig class is in sam.generated.ts which is generated from 000_sam_spec.json.
I'm not sure how the sam spec gets checked in, but it seems the sam_spec is incorrect


This is 🐛 Bug Report

@GaalDornick GaalDornick added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 30, 2021
@github-actions github-actions bot added the @aws-cdk/aws-sam Related to AWS Serverless Application Model label Sep 30, 2021
@GaalDornick
Copy link
Author

I can submit a fix by updating sam_spec, but I'm not sure if that's the right thing to do

@GaalDornick
Copy link
Author

This is a bug in GoFormation. I have submitted an issue and a fix in GoFormation repo (awslabs/goformation#404)

This issue can be closed. I'll close it when the fix is merged into GoFormation

@njlynch
Copy link
Contributor

njlynch commented Oct 7, 2021

Thanks for chasing up and fixing the bug in the upstream, @GaalDornick !

I see that the change was merged in goformation, and has been picked up by our automation (#16820). This will go out with our next release.

@njlynch njlynch closed this as completed Oct 7, 2021
@github-actions
Copy link

github-actions bot commented Oct 7, 2021

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-sam Related to AWS Serverless Application Model bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants