aws_stepfunctions: addPrefix fails when added to map iterator

[grzegorzewald]

Describe the bug

When using StpeFunctionFragment construct with map, prefix_states() method does not workg properly with Map construct used in fragment.

This feels like add_prefix would add prefix only to CDK constructs but not to the step function tasks inside MAP iterator elements. Issue does not exist outside of the map.

Note: worth to check if Parallel is not affected as well.

Expected Behavior

Create Map element iterator items with proper prefix.

Current Behavior

Prefix feels not to be added at proper time instant, erroring with duplicated states. The error for code example below is: jsii.errors.JSIIError: State with name 'The Pass' occurs in both Map The Map Iterator (Fragment 1: The Pass) and Map The Map Iterator (Fragment 2: The Pass). All states must have unique names.

Reproduction Steps

Minimum viable code to reproduce error:

from aws_cdk import (
    aws_stepfunctions as sfn,
    Stack,
)
from constructs import Construct

class HasMapIssueFragment(sfn.StateMachineFragment):
    def __init__(self, parent, fragment_id: str) -> None:
        super().__init__(parent, fragment_id)

        map = sfn.Map(
            self,
            "The Map",
        ).iterator(
            sfn.Pass(self, "The Pass")
        )

        self._start_state = map.start_state
        self._end_states = map.end_states
        self.prefix_states(f'{fragment_id}: ')

    @property
    def start_state(self) -> sfn.State:
        return self._start_state

    @property
    def end_states(self) -> list[sfn.INextable]:
        return self._end_states

class HasMapIssue(Stack):

    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        machine = HasMapIssueFragment(self, "Fragment 1").next(HasMapIssueFragment(self, "Fragment 2"))

        self.step_function = sfn.StateMachine(
            self, "The Machine",
            state_machine_type=sfn.StateMachineType.STANDARD,
            definition=machine,
        )

app = cdk.App()
HasMapIssue(app, "HasMapIssueStack",)
app.synth()

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.54.0

Framework Version

No response

Node.js Version

v18.12.1

OS

OSX

Language

Python

Language Version

Python 3.9.6

Other information

No response

[peterwoodworth]

Thanks for reporting this, I've confirmed it fails with Parallel and Fail as well

I think this is happening because when you add a state to an iterator, a new graph is created using the iterator.startState value https://github.com/aws/aws-cdk/blob/9f41881be1cca31129078b7839dded480220f060/packages/%40aws-cdk/aws-stepfunctions/lib/states/map.ts#L161

The prefix isn't yet applied when this is ran. Refactoring the code like this will work successfully:

    pass = Pass(self, `Pass`)
    self.prefix_states()
    map = Map(self, 'iterator').iterator(pass)
    self.prefix_states()

[grzegorzewald]

Hi @peterwoodworth,

Thanks for confirming. Your fix works only partially - it causes to add double prefixes (technically every call for prefix_states() adds prefixes to already instantiated constructs.

[peterwoodworth]

Ah that would make sense, I was just checking for successful synth. I suppose you can make an individual call for each additionally created state then, but that could get inconvenient in a larger project

    pass = Pass(self, `Pass`)
    self.prefix_states()
    map = Map(self, 'iterator').iterator(pass)
    map.add_prefix()

[jrieko]

The above workaround, calling prefixStates within the fragment constructor, works only if you want the default prefix or are ok passing the prefix string into the constructor. I was looking to call .prefix_states(string) on the fragment object, after construction:

new SomeFragment(...).prefixStates('[context] ');

Here's my rather hacky workaround, so that prefixStates just works as advertised:

import { IChainable, Map as sfnMap, StateGraph } from 'aws-cdk-lib';

export class Map extends sfnMap {
    private _iterator?: IChainable;

    public iterator(iterator: IChainable): Map {
        // fix for https://github.com/aws/aws-cdk/issues/23320
        // defer putting the iterator into the StateGraph until `bindToGraph` is called, after any `addPrefix` calls are done
        this._iterator = iterator;
        return this;
    }

    public bindToGraph(graph: StateGraph): void {
        if (this._iterator != null) {
            super.iterator(this._iterator);
            // unset the pending iterator, so this is called only once
            this._iterator = undefined;
        }
        super.bindToGraph(graph);
    }
}

• I don't know if there could be any side effect of deferring assigning the iterator graph, but I haven't encountered such yet.
• If you don't want to change your imports to this class, one could modify cdk Map's prototype. Even more hacky, but then once this issue is resolved, one need only delete the above code.

I'm surprised to see less activity on this issue. I'd think this issue is a pretty common case, for anyone building reusable fragments, which often contain the affected States. I wonder if most folks are explicitly prefixing the ids, rather than using prefixStates.