Propagate errors across all results in a transaction #973
Conversation
A new error `ResultFailedError` is introduced. It will be raised when using a `Result` object after the result or another result in the same transaction has failed. User code would only ever run into this situation when catch exceptions and deciding to ignore them. Now, an error will be raised instead of undefined behavior. The undefined behavior before this fix could be (among other things) protocol violations, incomplete summary data, and hard to interpret errors.
| @@ -1946,6 +1948,9 @@ Client-side errors | |||
| :show-inheritance: | |||
| :members: result | |||
|
|
|||
| .. autoexception:: neo4j.exceptions.ResultFailedError() | |||
There was a problem hiding this comment.
Understand if we have settled on a common name for this already. but is this error indicative what is actually wrong? "The result failed" is very vague, would something like: TransactionInvalidStateError be a bit more explanative?
if the detail message could be:
The transaction that contains this result has encountered error, all results in this transaction are therefore invalid. if you could attach the transaction to the error so users can access it that could be useful for debugging
There was a problem hiding this comment.
We have not defined what exact error should be thrown. So this sure is up for debate.
However, there is more detail to this massage:
- https://github.com/neo4j/neo4j-python-driver/pull/973/files#diff-d52cb3ae0b87a7893168acf79ace03a235b5d4de3020d230e45c08fdbea54d79R470-R476
- https://github.com/neo4j/neo4j-python-driver/pull/973/files#diff-9602a662eaafc9db0aa47c1955dafa066ef3edb2c2213439b36f9481e40ac3c3R61-R64
The error name is purposefully vague because it can have multiple reasons why the result would throw it:
- The result in question failed but the user ignored it and kept using the result
- A different result in the same transaction failed but the user ignored it
In either case, the raise ResultFailedError(...) from ... here (specifically the from bit) will make the error say something like "this is a direct cause of <insert original error with stack trace>".
There was a problem hiding this comment.
Thanks, should we consider exposing a property isFailed:boolean on the result? Whether that is in this change or subsequent one.
A new error
ResultFailedErroris introduced. It will be raised when using aResultobject after the result or another result in the same transaction has failed.User code would only ever run into this situation when catch exceptions and deciding to ignore them. Now, an error will be raised instead of undefined behavior. The undefined behavior before this fix could be (among other things) protocol violations, incomplete summary data, and hard to interpret errors.
Depends on: