neo4jrb / neo4j-core

A simple unified API that can access both the server and embedded Neo4j database. Used by the neo4j gem
MIT License
99 stars 80 forks source link

how to rescue a Neo4j Exception within a transaction? #310

Open benjaminbradley opened 6 years ago

benjaminbradley commented 6 years ago

According to the transaction documentation :

you can start a transaction by calling Neo4j::Transaction.new. Be aware that if you do this, it will not close automatically and will not catch errors, so you will need to handle this manually. This is useful if you want to rescue a specific error and have additional logic to be performed in the event of a failure.

NOTE: I'm linking to the older wiki docs because the readthedocs site has no documentation on Transactions, apart from the one-word mention under "Additional features".

However, in the test case below, the transaction appears to be failed automatically anyway, even though the procedural syntax is used to create the transaction. Is this a bug with the code or with the documentation?

Code example

gem 'neo4j', '=7.1.2'
gem 'neo4j-core', '=6.1.6'
require 'neo4j'
require 'neo4j-core'
class TestClass
  include Neo4j::ActiveNode
  property :name, type: String, constraint: :unique
end
begin
  Neo4j::Session.open(:server_db, ENV["NEO4J_URL"])
  neo4j_transaction = Neo4j::Transaction.new
  test = TestClass.create(name: 'foo')
  test2 = TestClass.create(name: 'foo') # throws ConstraintViolationError
rescue Neo4j::Server::CypherResponse::ConstraintViolationError => error
  puts "tx.failed?=#{neo4j_transaction.failed?}" # shows tx.failed?=true
  # further updates within this transaction are not possible, e.g. to resolve the name conflict above:
  test.update_attribute(:name, 'foo.tmp')
  # this line throws the exception: Expected response code 200 Error for request http://DBHOST:7474/db/data/transaction/TXID, 404 (Neo4j::Server::Resource::ServerException)
  # which I am interpreting as saying the transaction has already been rolled back/deleted
  # although the error text certainly could be clearer
ensure
  TestClass.destroy_all # clean up any temp records so we can re-run again
end
cheerfulstoic commented 6 years ago

Hello, sorry for the delayed response!

This is a big, gaping hole in the docs. It's made more complicated by the fact that the way that sessions are handled changed in version 8.0 of the neo4j gem and 7.0 of the neo4j-core gem (which I note that you aren't using, which is fine, though supporting the gem in my spare time makes it harder to diagnose the older versions).

If I had to guess, I would think that the problem is that because the exception comes from Neo4j, that it is probably being raised through some sort of begin / rescue boundary inside of .create. Because Neo4j doesn't truly have nested transactions, a failure in an inner transaction will cause a failure in the outer transaction. In fact, because there's no HTTP API in Neo4j for starting a transaction inside of another transaction, the gem's HTTP code always uses the transaction ID of the outer-most transaction, which has the same behavior (though the failure is probably getting marked on the inner transaction object in Ruby). To make sure, I would also try starting the transaction outside of your begin / rescue block (though I don't see how that would change things, exactly).

I would need to look deeper to find the exact problem / solution, but I imagine it would potentially be to not use a transaction inside of create (if that is the case)

benjaminbradley commented 6 years ago

Thanks, Brian. If you're correct about the issue being inside create, then I would need to patch the gem to resolve the issue, is that right?

Has this issue been resolved in the newer versions of the gems? I don't want to undertake a major upgrade unless I know it will resolve the issue.

cheerfulstoic commented 6 years ago

Hrmmm, since I haven't tracked down the problem I don't know for sure if it's fixed. I definitely change a lot of things WRT how queries are made in the neo4j because the neo4j-core gem was completely refactored to support different adaptors when Bolt came along.

I'm not sure how big your app is, but it would be good to try the upgrade since you may eventually want to switch to Bolt and all new development is happening on that codebase. I made an upgrade guide to try to help the process, but also definitely drop by Gitter if something is puzzling