DB related exceptions within a transaction cause the transaction to fail

[aanastasiou]

The problem

This is a sneaky bug because the traceback is all due to the Neo4j Python driver when the issue is more likely due to the way neomodel TransactionProxy handles the presence of additional exceptions.

Reproducing it

# Set up a very simple model for the tests
class PersonalRelationship(neomodel.StructuredRel):
    """
    A very simple relationship between two basePersons that simply records 
    the date at which an acquaintance was established.
    This relationship should be carried over to anything that inherits from 
    basePerson without any further effort.
    """
    on_date = neomodel.DateProperty(default_now = True)

class BasePerson(neomodel.StructuredNode):
    """
    Base class for defining some basic sort of an actor.
    """
    name = neomodel.StringProperty(required = True, unique_index = True)
    friends_with = neomodel.RelationshipTo("BasePerson", "FRIENDS_WITH", model = PersonalRelationship)

class TechnicalPerson(BasePerson):
    """
    A Technical person specialises BasePerson by adding their expertise
    """
    expertise = neomodel.StringProperty(required = True)

This simple case works:

with neomodel.db.transaction:
        A = TechnicalPerson(name = "Grumpy", expertise = "Grumpiness").save()

This however fails:

A = TechnicalPerson(name = "Grumpy", expertise = "Grumpiness").save()

with neomodel.db.transaction:
    try:
        A = TechnicalPerson(name = "Grumpy", expertise = "Grumpiness").save()
    except neomodel.UniqueProperty:
        A = TechnicalPerson.nodes.get(name = "Grumpy", expertise = "Grumpiness")

The latter snippet fails with a trace that concludes with:

. . .
 raise CypherError.hydrate(**metadata)
neo4j.exceptions.ClientError: Transaction rolled back even if marked as successful

Reason & Potential Resolution

util.Database defines transaction as a property. Upon calling with neomodel.db.transaction:, this transaction property getter returns a util.TransactionProxy object whose __enter__ kickstarts the transaction.

If an exception occurs within the with block, the util.Database.TransactionProxy.__exit__ is called with the exception type, value and traceback.

Neomodel catches the generic neo4j.v1.CypherError and specifically the ConstraintValidationFailed to raise its own exception.UniqueProperty.

This is exactly what happens right here but, an additional exception seems to be fired as well which bubbles unhandled all the way up into the with block causing it to fail.

[robinedwards]

Very nice spot whats the class of the additional exception, would love to have a patch for this

[aanastasiou]

Did a little bit more digging with this:

The exception is fired by the Neo4J driver within util.TransactionProxy.__exit__ when everything seems to have gone alright (if not exc_value: self.db.commit()).

That commit receives exception neo4j.exceptions.ClientError with .message "Transaction rolled back even if marked as succesful".

This however happens ONLY when an exception has taken place previously.

I am guessing that even the presence of a ConstraintValidation exception invalidates the transaction. Therefore, TransactionProxy needs a way of raising its own exception (here UniqueProperty) and then resetting itself. That is, roll back the transaction (to reset the "driver's state") and somehow restart it. This does not sound valid though. If the transaction is reset, that's final, which would exclude the ability to perform iteration within a with block. I will keep digging.

[aanastasiou]

Without modifying util.TransactionProxy.__exit__ and simply ignoring this specific exception if it is raised on the commit branch, then, despite the "complaint", the transaction is going ahead and data gets written to the db.

Hence the clarifying question here.

The "fix" in user land is:

A = TechnicalPerson(name = "Grumpy", expertise = "Grumpiness").save()
try:
    with neomodel.db.transaction:
        try:
            A = TechnicalPerson(name = "Grumpy", expertise = "Grumpiness").save()
        except neomodel.UniqueProperty:
            A = TechnicalPerson.nodes.get(name = "Grumpy", expertise = "Grumpiness")  
except neo4j.exceptions.ClientError:
    # Ignore
   pass

Which simply catches the exception.

In terms of a fix: According to this, which neomodel does anyway in util.Database.begin(), commit does not need to be called explicitly. It will probably be called in the __exit__ of the with of the Neo4j driver (?). This might be causing some of this confusion, but I am not sure.

Therefore, the "fix" might be as simple as removing the if not exc_value: self.db.commit() branch (?).

Update:

If the with contains more than one successfully handled exceptions, there is a block condition which leads to "Transaction in progress" raised by neomodel this time. I am referring to the "user-land" fix above. If, despite the exterior try / except, the with contains code that raises more than one exceptions, it leads to a deadlock which terminates with a "Transaction in progress" error.

The alternative is to turn certain try / except to get_or_create but that only covers one specific use of transactions. There are still conditions where we might have to handle more than one exceptions within a transaction the combined result of which ending up valid. :/