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

Context Stop still executing next step #55

Open
VictorTBX opened this issue Dec 18, 2021 · 6 comments
Open

Context Stop still executing next step #55

VictorTBX opened this issue Dec 18, 2021 · 6 comments
Labels
bug Something isn't working hacktoberfest help wanted Extra attention is needed severity-major Item is very important

Comments

@VictorTBX
Copy link
Contributor

VictorTBX commented Dec 18, 2021

Describe the bug
When I use the .stop() function of context, according to the audit, the code still executed the next step.

To Reproduce
Add two steps to an usecase
In the first step use ctx.stop() and return Ok() followed by it
In the second step add anything and return Ok() as well
Log the usecase auditTrail
Check that the second step was executed and both steps have the property "stopped":true

Expected behavior
When ctx.stop() is called, the next step should not be executed and the usecase should finish.

Audit Trail Json Example

"steps":[
         {
            "type":"step",
            "description":"Verify if parameters are valid",
            "return":{
               "Ok":""
            },
            "elapsedTime":"26019"
         },
         {
            "type":"step",
            "description":"Verify if cache exists",
            "return":"",
            "elapsedTime":"23959698",
            "stopped":true
         },
         {
            "type":"step",
            "description":"Get groups by user id",
            "return":{
               "Ok":""
            },
            "elapsedTime":"27913907",
            "stopped":true
         }
      ]
@VictorTBX VictorTBX added the bug Something isn't working label Dec 18, 2021
@VictorTBX
Copy link
Contributor Author

Extra info: the return of the stopped step is empty, it should be an Ok() in this case.

@jhomarolo
Copy link
Contributor

Hi @VictorTBX, do you think it's a bug in stop() or inside the auditTrail?

@VictorTBX
Copy link
Contributor Author

VictorTBX commented Dec 18, 2021

I've tried to reproduce with a POC and the problem didn't happen. I'll need to take more info from the source project in which the bug is occurring.

const { usecase, Ok, step } = require("@herbsjs/herbs")

const useCaseTest = () =>
    usecase("Test Stop", {
        setup: (ctx) => (ctx.di = Object.assign({}, {}, {})),
        "Step1": step(async (ctx) => {
            try {
                console.log("entered step1")

                await new Promise(r => setTimeout(r, 2000));

                if(true) {
                    ctx.ret = "step1"
                    ctx.stop()
                }
                
            } catch (error) {
                console.log("error")
            }

            return Ok()
        }),
        "Step2": step(async (ctx) => {
            console.log("entered step2")
            ctx.ret = "step2"
            return Ok()
        })
    })

const execute = async () => {
    const usecase = useCaseTest()
    await usecase.run()
    console.log(usecase.auditTrail)
}

execute()

@jhomarolo
Copy link
Contributor

@VictorTBX ok, let me know if you need some help into this

@BritoDoug
Copy link
Contributor

@VictorTBX take a look at this test implemented with stop() feature. Your case is different than some way ?
test/step/nestedStep.js

@VictorTBX
Copy link
Contributor Author

I am having difficulties to reproduce this error. The same problem occurred in another usecase. But it's "random", I can't find a pattern.

I was thinking it could be due to some misusage of async, but the implementation on buchu simply iterate through the steps and then do a "break" when the stop is true, so it shouldn't be the problem.

@BritoDoug different it is because the steps have more complex implementations, but the flow is the same.

@jhomarolo jhomarolo added help wanted Extra attention is needed severity-major Item is very important labels Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hacktoberfest help wanted Extra attention is needed severity-major Item is very important
Projects
None yet
Development

No branches or pull requests

4 participants