aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.57k stars 3.88k forks source link

(stepfunctions): Cannot make a loop with Choice and afterwards() in a state machine #16210

Closed nebur395 closed 2 years ago

nebur395 commented 3 years ago

:question: General Issue

The Question

I would want to use a Chain-Choice flow to make a loop within a step function. Besides that, I would like to be able to follow the main chain at root level, not at condition level. I've tried to use afterwards method since I thought this was for implemented following this purpose but it doesn't seem to work or I'm missing some implementation details.

Context/Background

I'm splitting the state machine in several constructs for readability and encapsulation reasons. Simplifying the use case it would look something like this (notice it's written in TS and it's just pseudo-code):

// In a real scenario this would be a list of steps, not just one
const setupStep1Chain = () => Chain.start(new LambdaInvoke('Step 1'));

const waitUntilStep1HasFinished = () => {
  const checkStep1HasFinished = new LambdaInvoke('Check step 1 status');
  const waitAndCheck = new Wait('Wait X').next(checkStep1HasFinished);
  const step2 = new LambdaInvoke('Step 2');
  const step1Choice = new Choice('Has step 1 finished?');

  return Chain.start(checkStep1HasFinished)
    .next(step1Choice.when('No', waitAndCheck).afterwards())
    .next(step2);
};

const setupStateMachine = () =>  
  new StateMachine({
    definition: Chain
      // You can imagine all these methods, as constructs in different files or workspaces or repos 
      // instead a simple function in the same file. This is just for make it simpler
      .start(setupStep1Chain())
      .next(waitUntilStep1HasFinished())
      // ... (a list of more chainable next steps or chains)
      .next(new Succeed('Done'));
  });

This is how I understood the method afterwards() should look like. However if I follow this approach, I just get this error:

Error: State 'Check step 1 status' already has a next state

And if I remove the .afterwards() method, I would get this other error:

Cannot add to chain: last state in chain (Has step 1 finished?) does not allow it

I tried different approaches in the Choice step such as using .otherwise() and intermediate Pass state... and a lot of this, but nothing seems to work. Of course, I can follow the flow in the conditional statement, something like this:

// In a real scenario this would be a list of steps, not just one
const setupStep1Chain = () => Chain.start(new LambdaInvoke('Step 1'));

const waitUntilStep1HasFinished = (nextChain) => {
  const checkStep1HasFinished = new LambdaInvoke('Check step 1 status');
  const waitAndCheck = new Wait('Wait X').next(checkStep1HasFinished);
  const step2 = new LambdaInvoke('Step 2');
  const step1Choice = new Choice('Has step 1 finished?');

  return Chain.start(checkStep1HasFinished)
    .next(step1Choice
      .when('No', waitAndCheck)
      .otherwise(step2.next(nextChain)));
};

const setupStateMachine = () =>  
  new StateMachine({
    definition: Chain
      .start(setupStep1Chain())
      .next(waitUntilStep1HasFinished(
        Chain. // ... (a list of more chainable next steps or chains)
          .next(new Succeed('Done'));
      ))
  });

However, this looks for me a bit weird for some architectural patterns, since you have to make waitUntilStep1HasFinished() method aware of the next chainable logic and as the author (of the issue I'm going to link at the end) said, you really decrease the readability of the code defining the state machine, having to read it by inspecting each Choice.

Is there any workaround to make this? Am I missing something in the afterwards feature?

Please, find just below a visual representation of what I would like to achieve with the pseudo-code I've just posted

Thanks a lot!

State Machine Loop and Afterwards

Environment

Other information

This issue is highly related with this one, it was closed without a response though

nebur395 commented 3 years ago

Hi there, any news so far? It's a bit blocker for us. Thanks a lot!

NGL321 commented 3 years ago

Hey @nebur395,

Thanks for reaching out! I have done some digging into this problem and it seems at its core this is a confusion of afterwards() vs otherwise() in a state machine workflow.

afterwards() is used to represent the then component of a if... else... then... condition. This requires there to be an if AND an else. In your first provided code block you are using afterwards() as the else component when this is actually the job of otherwise(). Your second code block correctly utilizes otherwise and creates the correct state machine flow. Remember, that with otherwise the ASL generated will look something like:

"Step1": {
    "Next": "Choice",
    ...
},
"Choice": {
    "Choices": [{
        "CONDITION": "RESULT",
        "Next": "Step1"
    }],
    "Default": "Step2"
},
"Step2": {
    ...
}

whereas ASL with afterwards() doesnt change the Default state, but rather injects an option after all choices are made. This is considered an extra step and runs regardless of the condition. This is useful for breaking out of complex loops done with conditions that have many possible results but all need to end with the same next step. Chances are, if you dont have an otherwise() already, you will not use an afterwards().

Please let me know if you need more info on this!

😸 😷

nebur395 commented 3 years ago

Good morning!

Thanks for the reply! :) Maybe I didn't make myself clear. I understand .otherwise() would be kinda else statement on a conditional flow. But what I would like to do would be something like

...
if (whatever) {}
else {}
...continue here regardless the if-else clause 

I don't have in this use case an else clause, (which would be the .otherwise() method), that's the reason I didn't include it in the example before, but I don't mind to include it. Then, the pseudo-code would be something like:

 // In a real scenario this would be a list of steps, not just one
const setupStep1Chain = () => Chain.start(new LambdaInvoke('Step 1'));

const waitUntilStep1HasFinished = () => {
  const checkStep1HasFinished = new LambdaInvoke('Check step 1 status');
  const waitAndCheck = new Wait('Wait X').next(checkStep1HasFinished);
  const step2 = new LambdaInvoke('Step 2');
  const step3 = new LambdaInvoke('Step 3');
  const step1Choice = new Choice('Has step 1 finished?');

  return Chain.start(checkStep1HasFinished)
    .next(
      step1Choice.
        when('No', waitAndCheck)
        .otherwise(step2)
    );
};

const setupStateMachine = () =>  
  new StateMachine({
    definition: Chain
      // You can imagine all these methods, as constructs in different files or workspaces or repos 
      // instead a simple function in the same file. This is just for make it simpler
      .start(setupStep1Chain())
      .next(waitUntilStep1HasFinished())
      // ... (a list of more chainable next steps or chains)
      .next(new Succeed('Done'));
  });

This will throw this error: Cannot add to chain: last state in chain (Has step 1 finished?) does not allow it because I'm adding a step (new Succeed('Done')) just after a choice. If I use a combination of otherwise and afterwards that would be something like:

 // In a real scenario this would be a list of steps, not just one
const setupStep1Chain = () => Chain.start(new LambdaInvoke('Step 1'));

const waitUntilStep1HasFinished = () => {
  const checkStep1HasFinished = new LambdaInvoke('Check step 1 status');
  const waitAndCheck = new Wait('Wait X').next(checkStep1HasFinished);
  const step2 = new LambdaInvoke('Step 2');
  const step3 = new LambdaInvoke('Step 3');
  const step1Choice = new Choice('Has step 1 finished?');

  return Chain.start(checkStep1HasFinished)
    .next(
      step1Choice.
        when('No', waitAndCheck)
        .otherwise(step2)
        .afterwards()
    );
};

const setupStateMachine = () =>  
  new StateMachine({
    definition: Chain
      // You can imagine all these methods, as constructs in different files or workspaces or repos 
      // instead a simple function in the same file. This is just for make it simpler
      .start(setupStep1Chain())
      .next(waitUntilStep1HasFinished())
      // ... (a list of more chainable next steps or chains)
      .next(new Succeed('Done'));
  });

This should be what I was looking for I guess. In order to follow the flow out of the conditional statements. However, since one of the flows is a loop, I will get this error: Error: State 'Check step 1 status' already has a next state. Which makes sanes because indeed, step 1 has already a next step, because it's a loop.

So I wonder if there is any possibility to give the flow control back to the main thread, cause I would wanna reach something similar to this:

const setupStep1Chain = () => Chain.start(new LambdaInvoke('Step 1'));

const waitUntilStep1HasFinished = () => {
  const checkStep1HasFinished = new LambdaInvoke('Check step 1 status');
  const waitAndCheck = new Wait('Wait X').next(checkStep1HasFinished);
  const step1Choice = new Choice('Has step 1 finished?');

  return Chain.start(checkStep1HasFinished)
    .next(step1Choice.when('No', waitAndCheck));
};

const setupStateMachine = () =>  
  new StateMachine({
    definition: Chain
      .start(setupStep1Chain())
      .next(waitUntilStep1HasFinished())
      // ... (and I would want to continue the flow here!! :octocat: 
      .next(new Succeed('Done'));
  });

I hope I made myself clearer now :)

m-radzikowski commented 2 years ago

I found this issue because of the same problem.

I understand .afterwards() as a way to flatten the chain definition and not make it nested every time you put Choice in there. But it doesn't work this way, as presented by @nebur395.

What I expected, using the example from above:

Chain
    .start(setupStep1Chain())
    .next(waitUntilStep1HasFinished())
    .next(new Pass('Done'));

what I have to do:

Chain
    .start(setupStep1Chain())
    .next(waitUntilStep1HasFinished(new Chain
      .start(new Pass('Done'))
      .next(...)
    ))

const waitUntilStep1HasFinished = (nextStep) => {
    const wait = new Wait()
    return Chain
        .start(wait)
        .next(
            new Choice()
                .when(Condition.something(), wait)
                .otherwise(nextStep)
        )
}

so each Chain is making a nested structure, while it could be flattened out.