[UI] Chat workflow better error handling

[soosr]

Fixes #12403
Fixes #12156

Relevant commit from #12391. It got merged when the original PR was merged.

Improves error handling on #12181

This will show the error dialog only the first time an error comes out for a non-interactive step (such as StartConversationStep). For interactive steps (which require user input), these are reset whenever an error occurs within their execution, which means the user will have to re-enter their input in order to retry the action, which is the expected UX:

Interactive: user input -> try action -> error message -> user input -> retry action -> error message -> user input....

Non interactive: try action -> error message -> retry action -> silently fail -> retry action....

[soosr]

Review comments from the other PR:

Review:

• I believe passing the cancel token to ExecuteStepAsync would make sense, and use the cancel token as the condition in the while loop.
• We have to make sure the error dialog can only pop up if the BAB dialog is opened, otherwise, it would be a surprise.
• [VDG] Chat workflow better error handling #12391 (comment)

[soosr]

Review comments from the other PR:

Review:

• I believe passing the cancel token to ExecuteStepAsync would make sense, and use the cancel token as the condition in the while loop.
• We have to make sure the error dialog can only pop up if the BAB dialog is opened, otherwise, it would be a surprise.
• [VDG] Chat workflow better error handling #12391 (comment)

All resolved.
The remaining issue to solve is if an error happens too often in a noninteractive step, it can still freeze the UI.

[soosr]

All resolved. The remaining issue to solve is if an error happens too often in a noninteractive step, it can still freeze the UI.

Fixed.

(Note: the way the canceltoken is passed through should be really simplified, but didn't want to start a refactoring in this PR)

[molnard]

cACK. Did not test it and I won't have time until Monday. Can someone else?

[molnard]

LGTM, did not get the chance to test it yet. If you are sure you can merge.

[soosr]

LGTM, did not get the chance to test it yet. If you are sure you can merge.

Waiting for review and test from a @zkSNACKs/ui-team member.

[ichthus1604]

tACK. Code is also OK, except for this, which I would resolve by deriving from ActivatableViewModel and handling inside OnActivated().

I think we can merge this PR and make this specific change later on.

[molnard]

I put a dummy exception into StartNewConversationAsync method. It is an infinite loop, that ends up in error again and again in the background, and nothing is displayed for the user. Also, it keeps running after I close the BaB so the cancellationtoken is not working.
It is better than before - but we still need to work on this.

[soosr]

I put a dummy exception into StartNewConversationAsync method. It is an infinite loop, that ends up in error again and again in the background, and nothing is displayed for the user.

That is exactly what we need. It keeps trying to execute the step without bothering the user with any error (meanwhile the chat window is busy).

Also, it keeps running after I close the BaB so the cancellationtoken is not working. It is better than before - but we still need to work on this.

It's working, even if the dialog is closed it should still try to execute the step. The cancel token is only called when the order is deleted, or reset. In those cases, the step will be aborted.

[molnard]

Unusual, but let it be like that. It works and we do not have more time before the release.

P.S.
Why unusual? Usually, if the user starts an action that ends up in an error he will get a dialog - hey your network is down, are you sure you paid the internet bill?

[soosr]

Unusual, but let it be like that. It works and we do not have more time before the release.

P.S. Why unusual? Usually, if the user starts an action that ends up in an error he will get a dialog - hey your network is down, are you sure you paid the internet bill?

This PR fixes it #12433