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-stepfunctions: state.Retry is not iterable #29274

Closed
abdelnn opened this issue Feb 26, 2024 · 5 comments · Fixed by #29403 · May be fixed by NOUIY/aws-solutions-constructs#98 or NOUIY/aws-solutions-constructs#99
Closed

aws-stepfunctions: state.Retry is not iterable #29274

abdelnn opened this issue Feb 26, 2024 · 5 comments · Fixed by #29403 · May be fixed by NOUIY/aws-solutions-constructs#98 or NOUIY/aws-solutions-constructs#99
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@abdelnn
Copy link
Contributor

abdelnn commented Feb 26, 2024

Describe the bug

When creating a CustomState within a state machine, and specifying the retry behaviour within the inline definition, we get back an error saying state.Retry is not iterable. However, adding the retry behaviour separately with .addRetry seems to fix this issue. I strongly suspects this is linked to the following PR #28793

Expected Behavior

The state machine should create successfully

Current Behavior

Throws

TypeError: state.Retry is not iterable
    at CustomState.toStateJson

Reproduction Steps

Creating a state machine with the following state

const myLambdaState: sfn.CustomState = new sfn.CustomState(this, 'Call Lambda', {
            stateJson: {
                Type: "Task",
                Resource: "arn:aws:states:::lambda:invoke",
                OutputPath: "$.Payload",
                Parameters: {
                    'Payload.$': "$",
                    "FunctionName": "arn:aws:lambda:us-east-1:<<ACCOUNT>>:function:print1:$LATEST"
                },
                Retry: [
                    {
                        "ErrorEquals": [
                            "Lambda.ServiceException",
                            "Lambda.AWSLambdaException",
                            "Lambda.SdkClientException",
                            "Lambda.TooManyRequestsException"
                        ],
                        "IntervalSeconds": 1,
                        "MaxAttempts": 3,
                        "BackoffRate": 2
                    }
                ],
                Catch: [
                    {
                        ErrorEquals: [
                            "States.ALL"
                        ],
                        Next: "fail.id"
                    }
                ]
                ,
            }
        });

However, doing the following works:

      const success: sfn.Succeed = new sfn.Succeed(this, 'Success Name');
        const fail: sfn.Fail = new sfn.Fail(this, 'Fail Name');

        const myLambdaState: sfn.CustomState = new sfn.CustomState(this, 'Call Lambda', {
            stateJson: {
                Type: "Task",
                Resource: "arn:aws:states:::lambda:invoke",
                OutputPath: "$.Payload",
                Parameters: {
                    'Payload.$': "$",
                    "FunctionName": "arn:aws:lambda:us-east-1:<<ACCOUNT-ID>>:function:print1:$LATEST"
                }
            }
        });
        myLambdaState.addRetry({
            maxAttempts: 1,
            maxDelay: cdk.Duration.seconds(5),
            jitterStrategy: sfn.JitterType.FULL,
        });

        myLambdaState.addCatch(fail, {
            errors: ['States.ALL']
        }
        )
        myLambdaState.next(success);

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.128.0

Framework Version

No response

Node.js Version

v21.6.2

OS

Mac 14.3.1

Language

TypeScript

Language Version

No response

Other information

No response

@abdelnn abdelnn added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 26, 2024
@github-actions github-actions bot added the @aws-cdk/aws-stepfunctions Related to AWS StepFunctions label Feb 26, 2024
@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 27, 2024
@jsayres
Copy link

jsayres commented Feb 28, 2024

The problem seems to be here: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-stepfunctions/lib/states/custom-state.ts#L74

renderRetryCatch can yield a value of undefined for Retry if the list of retries is empty. You can see this in the renderList function: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-stepfunctions/lib/states/state.ts#L641

This will cause the state value to have a Retry of undefined from the merge. Then the attempt to iterate fails here: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-stepfunctions/lib/states/custom-state.ts#L79

The solution would be to make the merge a bit smarter and also to validate state.Retry.

@abdelnn
Copy link
Contributor Author

abdelnn commented Feb 29, 2024

I agree - which is probably why we're not getting an error when specifying the error strategy through .addRetry

@mergify mergify bot closed this as completed in #29403 Mar 12, 2024
mergify bot pushed a commit that referenced this issue Mar 12, 2024
…29403)

### Issue # (if applicable)

Closes #29274 

### Reason for this change

CDK users were unable to specify their retry strategy if it was specified inline in their ASL state machine definition

### Description of changes

Checks if the state definition has an inline retry policy defined. If it does, add it to the existing strategy defined using `addRetry` (if there is one defined, this is where it was failing before)

### Description of how you validated changes

Added unit and integration tests

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

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

@abhi-shek01
Copy link

abhi-shek01 commented Mar 22, 2024

In which file do we need to make this change? I am getting a same error with "aws-cdk-lib" : "^2.118.0" version @abdelnn

@wong-a
Copy link
Contributor

wong-a commented Mar 22, 2024

@abhi-shek01 The fix was released in the most recent version of aws-cdk https://github.com/aws/aws-cdk/releases/tag/v2.133.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
5 participants