search

getmoto / moto

A library that allows you to easily mock out tests based on AWS infrastructure.
http://docs.getmoto.org/en/latest/
Apache License 2.0
7.49k stars 1.99k forks source link

Step function creation does not work with Retry conditions specifying MaxAttempts equal 0 #7823

Open mik3h0 opened 3 days ago

mik3h0 commented 3 days ago

Why this is an issue

Whilst it might seem like a zero retry statement is redundant, it has some niche use cases. Take the following example:

import boto3
from moto import mock_aws

STATE_MACHINE_DEFINITION = """
{
  "Comment": "A description of my state machine",
  "StartAt": "DynamoDB DeleteItem",
  "States": {
    "DynamoDB DeleteItem": {
      "Type": "Task",
      "Resource": "arn:aws:states:::dynamodb:deleteItem",
      "Parameters": {
        "TableName": "ThisTableDoesNotExist",
        "Key": {
          "Column": {
            "S": "MyEntry"
          }
        }
      },
      "End": true,
      "Retry": [
        {
          "ErrorEquals": [
            "DynamoDB.ConditionalCheckFailedException"
          ],
          "BackoffRate": 2,
          "IntervalSeconds": 1,
          "MaxAttempts": 0
        },
        {
          "ErrorEquals": [
            "DynamoDB.AmazonDynamoDBException"
          ],
          "BackoffRate": 2,
          "IntervalSeconds": 1,
          "MaxAttempts": 3
        }
      ]
    }
  }
}
"""

def test_recreate_zero_issue():
    with mock_aws(config={"stepfunctions": {"execute_state_machine": True}}):
        client = boto3.client("stepfunctions", region_name="us-east-1")
        response = client.create_state_machine(
            name="test",
            definition=STATE_MACHINE_DEFINITION,
            roleArn="arn:aws:iam::123456789012:role/service-role/StatesExecutionRole-us-east-1",
        )

In this case we use a zero retry to say "retry all DynamoDB error states, except DynamoDB.ConditionalCheckFailedException" (because we have a conditional check on the DynamoDb operation, which would send us off to another part of the workflow)

Recreating the issue

With the above code example, try to run the test. you will see this in the stack trace (full trace at the bottom of this post for readability)

self = (MaxAttemptsDecl| {}, attempts = 0

    def __init__(self, attempts: int = DEFAULT_ATTEMPTS):
        if not (1 <= attempts <= MaxAttemptsDecl.MAX_VALUE):
>           raise ValueError(
                f"MaxAttempts value MUST be a positive integer between "
                f"1 and {MaxAttemptsDecl.MAX_VALUE}, got '{attempts}'."
            )
E           ValueError: MaxAttempts value MUST be a positive integer between 1 and 99999999, got '0'.

Note: The AWS console specifies retry values must be "zero or greater", it's what I used to create this example.

setting retries to 1 or more creates the step function as expected.

Expected behaviour

The step function should be created if the definition contains retries with a MaxAttempts value of zero

Actual behaviour

The step function is not created, an exception is raised

Moto Version

5.0.10

Full stack trace

tests/test_recreate_zero_issue.py:43 (test_recreate_zero_issue)
definition = '\n{\n  "Comment": "A description of my state machine",\n  "StartAt": "DynamoDB DeleteItem",\n  "States": {\n    "Dyna...   "BackoffRate": 2,\n          "IntervalSeconds": 1,\n          "MaxAttempts": 3\n        }\n      ]\n    }\n  }\n}\n'

    @staticmethod
    def _validate_definition(definition: str):
        # Validate
        # TODO: pass through static analyser.
        try:
>           AmazonStateLanguageParser.parse(definition)

../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/models.py:68: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/asl_parser.py:69: in parse
    program = preprocessor.visit(tree)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:34: in visit
    return tree.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:7819: in accept
    return visitor.visitState_machine(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/preprocessor.py:937: in visitState_machine
    return self.visit(ctx.program_decl())
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:34: in visit
    return tree.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:7880: in accept
    return visitor.visitProgram_decl(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/preprocessor.py:914: in visitProgram_decl
    cmp: Optional[Component] = self.visit(child)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:34: in visit
    return tree.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:7953: in accept
    return visitor.visitTop_layer_stmt(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParserVisitor.py:23: in visitTop_layer_stmt
    return self.visitChildren(ctx)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:44: in visitChildren
    childResult = c.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:8528: in accept
    return visitor.visitStates_decl(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/preprocessor.py:271: in visitStates_decl
    cmp: Optional[Component] = self.visit(child)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:34: in visit
    return tree.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:8643: in accept
    return visitor.visitState_decl(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/preprocessor.py:376: in visitState_decl
    state_props: StateProps = self.visit(ctx.state_decl_body())
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:34: in visit
    return tree.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:8706: in accept
    return visitor.visitState_decl_body(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/preprocessor.py:370: in visitState_decl_body
    cmp: Optional[Component] = self.visit(child)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:34: in visit
    return tree.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:8290: in accept
    return visitor.visitState_stmt(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParserVisitor.py:39: in visitState_stmt
    return self.visitChildren(ctx)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:44: in visitChildren
    childResult = c.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:13664: in accept
    return visitor.visitRetry_decl(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/preprocessor.py:723: in visitRetry_decl
    cmp: Optional[Component] = self.visit(child)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:34: in visit
    return tree.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:13748: in accept
    return visitor.visitRetrier_decl(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/preprocessor.py:731: in visitRetrier_decl
    cmp: Optional[Component] = self.visit(child)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:34: in visit
    return tree.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:13827: in accept
    return visitor.visitRetrier_stmt(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/preprocessor.py:736: in visitRetrier_stmt
    return self.visit(ctx.children[0])
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/antlr4/tree/Tree.py:34: in visit
    return tree.accept(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/antlr/runtime/ASLParser.py:14055: in accept
    return visitor.visitMax_attempts_decl(self)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/parse/preprocessor.py:779: in visitMax_attempts_decl
    return MaxAttemptsDecl(attempts=int(ctx.INT().getText()))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = (MaxAttemptsDecl| {}, attempts = 0

    def __init__(self, attempts: int = DEFAULT_ATTEMPTS):
        if not (1 <= attempts <= MaxAttemptsDecl.MAX_VALUE):
>           raise ValueError(
                f"MaxAttempts value MUST be a positive integer between "
                f"1 and {MaxAttemptsDecl.MAX_VALUE}, got '{attempts}'."
            )
E           ValueError: MaxAttempts value MUST be a positive integer between 1 and 99999999, got '0'.

../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/asl/component/common/retry/max_attempts_decl.py:26: ValueError

During handling of the above exception, another exception occurred:

    def test_recreate_zero_issue():
        with mock_aws(config={"stepfunctions": {"execute_state_machine": True}}):
            client = boto3.client("stepfunctions", region_name="us-east-1")
>           response = client.create_state_machine(
                name="test",
                definition=STATE_MACHINE_DEFINITION,
                roleArn="arn:aws:iam::123456789012:role/service-role/StatesExecutionRole-us-east-1",
            )

test_recreate_zero_issue.py:47: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/client.py:565: in _api_call
    return self._make_api_call(operation_name, kwargs)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/client.py:1001: in _make_api_call
    http, parsed_response = self._make_request(
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/client.py:1027: in _make_request
    return self._endpoint.make_request(operation_model, request_dict)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/endpoint.py:119: in make_request
    return self._send_request(request_dict, operation_model)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/endpoint.py:202: in _send_request
    while self._needs_retry(
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/endpoint.py:354: in _needs_retry
    responses = self._event_emitter.emit(
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/hooks.py:412: in emit
    return self._emitter.emit(aliased_event_name, **kwargs)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/hooks.py:256: in emit
    return self._emit(event_name, kwargs)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/hooks.py:239: in _emit
    response = handler(**kwargs)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/retryhandler.py:207: in __call__
    if self._checker(**checker_kwargs):
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/retryhandler.py:284: in __call__
    should_retry = self._should_retry(
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/retryhandler.py:307: in _should_retry
    return self._checker(
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/retryhandler.py:363: in __call__
    checker_response = checker(
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/retryhandler.py:247: in __call__
    return self._check_caught_exception(
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/retryhandler.py:416: in _check_caught_exception
    raise caught_exception
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/endpoint.py:278: in _do_get_response
    responses = self._event_emitter.emit(event_name, request=request)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/hooks.py:412: in emit
    return self._emitter.emit(aliased_event_name, **kwargs)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/hooks.py:256: in emit
    return self._emit(event_name, kwargs)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/botocore/hooks.py:239: in _emit
    response = handler(**kwargs)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/core/botocore_stubber.py:37: in __call__
    response = self.process_request(request)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/core/botocore_stubber.py:84: in process_request
    status, headers, body = method_to_execute(
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/core/responses.py:290: in dispatch
    return cls()._dispatch(*args, **kwargs)
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/core/responses.py:501: in _dispatch
    return self.call_action()
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/core/responses.py:587: in call_action
    response = method()
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/responses.py:31: in create_state_machine
    state_machine = self.stepfunction_backend.create_state_machine(
../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/models.py:87: in create_state_machine
    StepFunctionsParserBackend._validate_definition(definition=definition)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

definition = '\n{\n  "Comment": "A description of my state machine",\n  "StartAt": "DynamoDB DeleteItem",\n  "States": {\n    "Dyna...   "BackoffRate": 2,\n          "IntervalSeconds": 1,\n          "MaxAttempts": 3\n        }\n      ]\n    }\n  }\n}\n'

    @staticmethod
    def _validate_definition(definition: str):
        # Validate
        # TODO: pass through static analyser.
        try:
            AmazonStateLanguageParser.parse(definition)
        except ASLParserException as asl_parser_exception:
            invalid_definition = InvalidDefinition()
            invalid_definition.message = repr(asl_parser_exception)
            raise invalid_definition
        except Exception as exception:
            exception_name = exception.__class__.__name__
            exception_args = list(exception.args)
>           invalid_definition = InvalidDefinition()
E           TypeError: AWSError.__init__() missing 1 required positional argument: 'message'

../../../../../../Library/Caches/pypoetry/virtualenvs/centralised-lock-Q7F2D_P9-py3.11/lib/python3.11/site-packages/moto/stepfunctions/parser/models.py:76: TypeError
mik3h0 commented 3 days ago

PR incoming for this issue btw :)