-
Notifications
You must be signed in to change notification settings - Fork 178
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
feat(api): Attach error recovery debug notes to commands #16608
Conversation
A PR has been opened to address analyses snapshot changes. Please review the changes here: #16609 |
…hots (#16609) This PR was requested on the PR #16608 Co-authored-by: SyntaxColoring <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## false_positive_state_update #16608 +/- ##
==============================================================
Coverage ? 92.43%
==============================================================
Files ? 77
Lines ? 1283
Branches ? 0
==============================================================
Hits ? 1186
Misses ? 97
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, I'm glad we're using these
Will merge behind #16556. |
…hots (#16609) This PR was requested on the PR #16608 Co-authored-by: SyntaxColoring <[email protected]>
5850cc3
to
7d5d1c7
Compare
Overview
User interactions for error recovery are already "logged", as the run's
actions
. However, stuff that the robot does automatically is not. This closes that gap in a basic way. When a command fails, we'll append a human-readablenote
to it that mentions how Protocol Engine is basically treating it. (FAIL_RUN
,WAIT_FOR_RECOVERY
,CONTINUE_WITH_ERROR
, orASSUME_FALSE_POSITIVE_AND_CONTINUE
.)In particular, this means that when a user selects Ignore all errors of this type, the run log can show us all the commands that the robot auto-handled in that way.
Closes EXEC-780.
Test Plan and Hands on Testing
Review requests
The existing
CommandNote
shape is just human-readable. So for simplicity, I'm matching that and just providing a human-readable string.Would it be a good idea to also expand
CommandNote
with aMapping[str, Any]
field, like we have for errors? Then we could put stuff likeStateUpdate
s in there without it being a gross monolithic string.Risk assessment
Low.