feat(api,shared-data): error codes in PE #12936
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
On the python side of our code, we want our new enumerated exceptions to be gradually integratable, and we also want to make sure that any errors that we didn't yet get the chance to give error codes end up with error codes. To do this in a programmatic way, we can add some automated methods for wrapping python exceptions. All enumerated errors now get to wrap errors. These are optional sequences of more enumerated errors that are considered to have caused the top-level one - in most cases, this will be because the enumerated error explicitly was instantiated to wrap a python exception, but it could also be if it was raised from one. Since we only wrap other enumerated errors, we need a way to make exceptions enumerated errors. A new exception type (but not code - it's just a GeneralError) called PythonException has this capability; it lets you give it BaseExceptions in addition to other EnumeratedErrors, and it's capable of walking the python memory model internals to try and get the other exceptions in a stack of raise from ... raise from ... calls that are reasonably popular in our code. This is functionality that is promoted out of The Dunder Zone in python 3.11, so I feel pretty good using it (this is what ExceptionGroups are). So now, as in the tests, if you catch an exception and give it to a PythonException you bless it with an error code and save all the exceptions and their stack traces for later inspection. Cool!
This is UNEXPECTED_TIP_ATTACH and occurs when there shouldn't be a tip but there is.
ProtocolEngine is the first place we'll go through and add places that actually use these error codes, since it's in a lovely high-leverage middle spot in our stack. That means we both get to test the upward interface of how these things will be represented in the HTTP API and how they'll be created from lower exceptions. ProtocolEngine already has its own very large and robust set of custom exceptions, which is awesome. We can make them inherit from the enumerated errors pretty easily, but unfortunately we have to add a bunch of stuff to their constructors to pass along things like the ability to wrap other exceptions and so on. Luckily that's just typing. Once we've done that, at the three points we catch all missed exceptions we have to switch over to creating the new style. ProtocolEngineErrors get passed on; uncaught legacy errors get captured as PythonExceptions; and uncaught errors in the normal core do too. Finally, we have to represent this new style of error in the ErrorOccurrence objects. This is the fun part. Previously, we'd added error codes to those objects; this was sort of a big deal because we want them to be required when you make new ErrorOccurrences and when clients look, but we don't want things to break when we deserialize old ones. We can extend that trick pretty easily to add new things. What's not quite as easy is this concept of wrapping errors. Our errors are now essentially trees, and we need tree structure here. Luckily, jsonschema and pydantic are actually pretty good at type-recursive schema and object definitions, so we can plop a list of other error occurrences in there. Now, when we catch one of these errors that's bubbled up from hardware, we give it a name and we capture its entire history in an inspectable way, and I think that's really cool.
sfoster1
requested review from
SyntaxColoring,
TamarZanzouri,
sanni-t and
jbleon95
Codecov Report
@@ Coverage Diff @@
## edge #12936 +/- ##
==========================================
- Coverage 73.03% 72.87% -0.17%
==========================================
Files 1522 2349 +827
Lines 49905 65003 +15098
Branches 3040 7363 +4323
==========================================
+ Hits 36449 47372 +10923
- Misses 12990 15931 +2941
- Partials 466 1700 +1234
Flags with carried forward coverage won't be shown. Click here to find out more.
|
fsinapi
reviewed
Jun 20, 2023
|
|
||
| base_details = { | ||
| k: str(v) | ||
| for k, v in inspect.getmembers(exc, _exc_harvest_predicate) |
TamarZanzouri
approved these changes
Jun 20, 2023
{
"data": [
{
"id": "e6875d13-6411-4233-815e-fbde1645f494",
"status": "completed",
"result": "not-ok",
"pipettes": [
{
"id": "43aa69ef-9fcb-4a8d-a605-eb00b46ea711",
"pipetteName": "p50_multi_gen3",
"mount": "right"
}
],
"labware": [
{
"id": "fixedTrash",
"loadName": "opentrons_1_trash_3200ml_fixed",
"definitionUri": "opentrons/opentrons_1_trash_3200ml_fixed/1",
"location": {
"slotName": "A3"
}
},
{
"id": "052df6d7-e106-4c3c-b008-4d53afc7349e",
"loadName": "opentrons_ot3_96_tiprack_50ul",
"definitionUri": "opentrons/opentrons_ot3_96_tiprack_50ul/1",
"location": {
"slotName": "C1"
}
},
{
"id": "c4d562e7-6dce-492b-9c1d-7bde52e9f7c7",
"loadName": "nest_96_wellplate_100ul_pcr_full_skirt",
"definitionUri": "opentrons/nest_96_wellplate_100ul_pcr_full_skirt/2",
"location": {
"slotName": "D2"
}
}
],
"modules": [],
"commands": [
{
"id": "aa62b242-77aa-41cd-abe4-46aa52e2dd82",
"createdAt": "2023-06-20T18:17:13.106187+00:00",
"commandType": "loadLabware",
"key": "a65f9ba51e49fe483a86eb3577d39ed1",
"status": "succeeded",
"params": {
"location": {
"slotName": "C1"
},
"loadName": "opentrons_ot3_96_tiprack_50ul",
"namespace": "opentrons",
"version": 1
},
"result": {
"labwareId": "052df6d7-e106-4c3c-b008-4d53afc7349e",
"definition": /* elided for readability */
},
"startedAt": "2023-06-20T18:17:13.106956+00:00",
"completedAt": "2023-06-20T18:17:13.191946+00:00"
},
{
"id": "14e18cdd-51a3-471c-81e7-8d4db68b92c6",
"createdAt": "2023-06-20T18:17:13.212726+00:00",
"commandType": "loadPipette",
"key": "4eca259490405003feb426e37ee37b91",
"status": "succeeded",
"params": {
"pipetteName": "p50_multi_gen3",
"mount": "right"
},
"result": {
"pipetteId": "43aa69ef-9fcb-4a8d-a605-eb00b46ea711"
},
"startedAt": "2023-06-20T18:17:13.243933+00:00",
"completedAt": "2023-06-20T18:17:13.248831+00:00"
},
{
"id": "1415d59d-cea3-4296-9354-92897e11a172",
"createdAt": "2023-06-20T18:17:13.258981+00:00",
"commandType": "loadLabware",
"key": "9b3cec49912e93925c019a164e3d4aa6",
"status": "succeeded",
"params": {
"location": {
"slotName": "D2"
},
"loadName": "nest_96_wellplate_100ul_pcr_full_skirt",
"namespace": "opentrons",
"version": 2
},
"result": {
"labwareId": "c4d562e7-6dce-492b-9c1d-7bde52e9f7c7",
"definition": /* elided for readability */
},
"startedAt": "2023-06-20T18:17:13.259928+00:00",
"completedAt": "2023-06-20T18:17:13.366491+00:00"
},
{
"id": "2623e20d-6428-4efd-a855-459919f3bd75",
"createdAt": "2023-06-20T18:17:13.389492+00:00",
"commandType": "pickUpTip",
"key": "0f79a97d73a9955bf3929be4043a70b9",
"status": "succeeded",
"params": {
"labwareId": "052df6d7-e106-4c3c-b008-4d53afc7349e",
"wellName": "A1",
"wellLocation": {
"origin": "top",
"offset": {
"x": 0,
"y": 0,
"z": 0
}
},
"pipetteId": "43aa69ef-9fcb-4a8d-a605-eb00b46ea711"
},
"result": {
"position": {
"x": 14.38,
"y": 181.1,
"z": 99
},
"tipVolume": 50,
"tipLength": 47.4,
"tipDiameter": 5.58
},
"startedAt": "2023-06-20T18:17:13.390351+00:00",
"completedAt": "2023-06-20T18:17:13.397476+00:00"
}
],
"errors": [
{
"id": "8168a21d-1001-4fc3-84ad-d2c0d9294cc2",
"errorType": "ExceptionInProtocolError",
"createdAt": "2023-06-20T18:17:13.594951+00:00",
"detail": "ValidationError [line 22]: 1 validation error for AspirateParams\nvolume\n ensure this value is greater than 0 (type=value_error.number.not_gt; limit_value=0)",
"errorCode": "4000",
"errorInfo": {},
"wrappedErrors": [
{
"id": "8168a21d-1001-4fc3-84ad-d2c0d9294cc2",
"errorType": "PythonException",
"createdAt": "2023-06-20T18:17:13.594951+00:00",
"detail": "pydantic.error_wrappers.ValidationError: 1 validation error for AspirateParams\nvolume\n ensure this value is greater than 0 (type=value_error.number.not_gt; limit_value=0)\n",
"errorCode": "4000",
"errorInfo": {
"args": "([ErrorWrapper(exc=NumberNotGtError(), loc=('volume',))], <class 'opentrons.protocol_engine.commands.aspirate.AspirateParams'>)",
"model": "<class 'opentrons.protocol_engine.commands.aspirate.AspirateParams'>",
"raw_errors": "[ErrorWrapper(exc=NumberNotGtError(), loc=('volume',))]",
"traceback": " File \"/opt/opentrons-robot-server/opentrons/protocols/execution/execute_python.py\", line 60, in run_python\n exec(\"run(__context)\", new_globs)\n\n File \"<string>\", line 1, in <module>\n\n File \"error.py\", line 22, in run\n\n File \"/opt/opentrons-robot-server/opentrons/protocols/api_support/util.py\", line 361, in _check_version_wrapper\n return decorated_obj(*args, **kwargs)\n\n File \"/opt/opentrons-robot-server/opentrons/protocol_api/instrument_context.py\", line 223, in aspirate\n self._core.aspirate(\n\n File \"/opt/opentrons-robot-server/opentrons/protocol_api/core/engine/instrument.py\", line 130, in aspirate\n self._engine_client.aspirate(\n\n File \"/opt/opentrons-robot-server/opentrons/protocol_engine/clients/sync_client.py\", line 254, in aspirate\n params=commands.AspirateParams(\n\n File \"/opt/opentrons-robot-server/pydantic/main.py\", line 406, in __init__\n raise validation_error\n",
"class": "ValidationError"
},
"wrappedErrors": []
}
]
}
],
"liquids": []
}
],
"meta": {
"cursor": 0,
"totalLength": 1
}
} |
|
That's the result of uploading a protocol that does |
|
|
1 task
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.
Draft alert!
This is a draft because I definitely need to get some fixes and further tests in the HTTP API layer and test on a robot and so on and so forth, but I wanted people to be able to see it since this does some medium-dubious jsonschema stuff and pokes into some core PE behavior.
Overview
This PR adds our new enumerated error codes to ProtocolEngine. This is the first place we're using them. We add the error codes to the protocol engine internal custom error hierarchies and add logic to capture python exceptions that are not
EnumeratedErrors at the three places that we catch exceptions - the core engine action handler, the legacy executor, and the main executor.The biggest new functionality is error wrapping. The
EnumeratedErrors can now catch, wrap, and store sequences ofraise fromexception contexts that could previously only really be represented as tracebacks. Now the structure of exception raise sequences is preserved all the way up to theErrorOccurrencemodel and thus to the client to format as they see fit. While some detail isn't really schema controlled (you'll have aDict[str, str]and you'll like it!) the structure actually is, as it's just a tree and pydantic and jsonschema do a pretty good job of supporting type recursion in models.I'm interested to see early reactions from @sanni-t @jbleon95 @TamarZanzouri and @SyntaxColoring ; I think I'm happy with how this turned out, but further testing will tell.
Scope and Followups
This PR is intended to add the enumerated error code handling and transformations to PD and correctly send them over HTTP. There's a followup PR needed to scrape through PE and make sure that everything has a correct error code, and that specific hardware calls that might fail in specific ways and already have specific
try/exceptclauses correctly wrap things in the right error codes and so on. There's also one needed to do that same thing in the other parts of theapisubdirectory, particularlyhardware_control, and hardware; and finally, something similar's going to need to happen in therobot-server, along with some kind of top-level exception catching there.Risks
Well, this modifies serialized data, so a lot, but hopefully in as safe a way as we can do it. It shouldn't introduce new errors, just transform how they're handled.
Testing