-
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
fix(api, shared-data, robot-server): stringify all error info in run log #13942
Conversation
…nlog pydantic validation errors
… a state summary fails
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## edge #13942 +/- ##
==========================================
- Coverage 70.66% 70.55% -0.12%
==========================================
Files 2508 1631 -877
Lines 70726 54259 -16467
Branches 8681 3854 -4827
==========================================
- Hits 49977 38281 -11696
+ Misses 18626 15304 -3322
+ Partials 2123 674 -1449
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.
Perfect, thanks! Let's log in that except tho
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.
Awesome, thanks!
Tested on Flexington. Confirmed that the route no longer returns an error, and the app is able to get past the initializing screen. The summary of the run is misleading since it returns a fake empty state summary, we should be sure to follow up on that. |
…log (#13942) * fix(shared-data, api): stringify exception details dict to prevent runlog pydantic validation errors * Add protection for run store to return None if pydantic validation of a state summary fails * Log run store errors when they happen
Overview
The
GET /runs
endpoint was returning a 500 error in some cases as a result of invalid entries in the robot run log. In the specific case of RQA-1872, the cause is a non-string entry in theerrorInfo
dictionary for an error that failed a run. To get more specific, the issue was that the PipetteOverpressureError exception was being created with an info field with a value field that was an instance ofOT3Mount
.Before this PR, our enumerated errors would accept a dictionary with string keys and any value type. This PR changes the classes to force the dictionary values to be strings. The
ErrorOccurrence
constructor was not performing any validation on its inputs because it was using theconstruct
method, and now it doesn't have to worry about that because any exceptions have to use strings.In addition, this PR adds a catch for
ValidationErrors
when getting the State Summary from the Run Store. It will return None, which is already tolerated by the higher calling functions in theGET /runs
chain.Test Plan
Fail a run with an overpressure error on a robot with current
edge
, then push this branch, and then make sureGET /runs
doesn't return a 500 error (there should be an empty state summary for the run that failed).Changelog
Review requests
Risk assessment
Pretty low, touches error handling that was already broken and stringifies a bunch of error messages.