fix: handle throw error inside retryTo promise

[Horsty80]

Motivation/Description of the PR

• When a throw error occurs inside promise, that stale the process, by rewrite with good reject and done call, we prevent this to happen

• fix(cli): stale process #4367

• maybe this pr can help this issue [Asking for help] Process are blocking when throw error in scenario with Retry enabled #4358

Applicable plugins:

• retryTo

Type of change

• 🐛 Bug fix

Checklist:

• Tests have been added
• Local tests are passed (Run npm test)

[Horsty80]

@kobenguyent with a friend, we have work on this, you can find a reproducible unit test that lead to a stale process without our change on retryTo.js

It's seems not terminating promises properly can lead to a stale process.
So maybe a clue to resolve issue #4358

Let me know if this needs some reworks

Thanks

[kobenguyent]

Hey @Horsty80 thanks for the investigation. May you hell check the UTs? Failed tests there. 🤔

[Horsty80]

Hey @Horsty80 thanks for the investigation. May you hell check the UTs? Failed tests there. 🤔

I check the why it's failing, on my laptop the npm run test is working and correct that

[Horsty80]

@kobenguyent TU are fix now :)

[kobenguyent]

Could you do me a favor please?

Trying your fix with scenario listed here. #4197

If that the case then I don't think my fix is needed or perhaps an extra layer to be sure.

[Horsty80]

Could you do me a favor please?

Trying your fix with scenario listed here. #4197

If that the case then I don't think my fix is needed or perhaps an extra layer to be sure.

I've check the scenario

Scenario('test issue', async ({ I }) => {
    I.amOnPage('http://example.org')
    I.waitForVisible('.nothing', 1); // should fail here but it won't terminate
    await retryTo( (tryNum) => {
        I.see(".doesNotMatter");
    }, 10);
});

And I think is another issue, not on the retryTo
I'm working on fix this to so I will fix it in another PR

I've tested this scenario and the issue (without your fix) is still present.
Sequence I.waitForVisible that failed and after a retryTo lead to stale the process

[kobenguyent]

Could you do me a favor please?
Trying your fix with scenario listed here. #4197
If that the case then I don't think my fix is needed or perhaps an extra layer to be sure.

I've check the scenario

Scenario('test issue', async ({ I }) => {
    I.amOnPage('http://example.org')
    I.waitForVisible('.nothing', 1); // should fail here but it won't terminate
    await retryTo( (tryNum) => {
        I.see(".doesNotMatter");
    }, 10);
});

And I think is another issue, not on the retryTo I'm working on fix this to so I will fix it in another PR

I've tested this scenario and the issue (without your fix) is still present. Sequence I.waitForVisible that failed and after a retryTo lead to stale the process

Thanks @Horsty80. I guess that aforementioned covered by #4367

[Horsty80]

Could you do me a favor please?
Trying your fix with scenario listed here. #4197
If that the case then I don't think my fix is needed or perhaps an extra layer to be sure.

I've check the scenario

Scenario('test issue', async ({ I }) => {
    I.amOnPage('http://example.org')
    I.waitForVisible('.nothing', 1); // should fail here but it won't terminate
    await retryTo( (tryNum) => {
        I.see(".doesNotMatter");
    }, 10);
});

And I think is another issue, not on the retryTo I'm working on fix this to so I will fix it in another PR
I've tested this scenario and the issue (without your fix) is still present. Sequence I.waitForVisible that failed and after a retryTo lead to stale the process

Thanks @Horsty80. I guess that aforementioned covered by #4367

Indeed maybe your fix cover this case.

[kobenguyent]

as this also addresses the stale process, so I think we would combine them into a branch. Then provide a beta version to test it out.