[VDG] Chat workflow better error handling #12391
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: molnard Co-authored-by: Szpoti Co-authored-by: soosr Co-authored-by: ichthus1604 Co-authored-by: SuperJMN Co-authored-by: lontivero Co-authored-by: adamPetho Co-authored-by: kiminuo
… into features/bab
Merged
soosr
reviewed
Feb 8, 2024
| { | ||
| OnStepError.SafeInvoke(this, ex); | ||
|
|
||
| // TODO: Roland, are we 100% sure we want to swallow errors and not log them? |
There was a problem hiding this comment.
I think we should store the Exception instead of counting the error. So if the same exception keeps happening then only log it once. Meanwhile if there is a new exception we need to log that too.
|
Review:
|
|
What is the point of merging this PR? The same commits by Fede are in the other PR #12181 and they were even reverted there. |
I didn't merge this intentionally, it got automatically merged by git when I merged the other PR |
|
Changes from this PR are not even visible on master. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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....