florinpatrascu / bolt_sips

Neo4j driver for Elixir
Apache License 2.0
258 stars 49 forks source link

[bolt v3] Bolt.Sips.transaction crash with invalid query #69

Closed dominique-vassard closed 5 years ago

dominique-vassard commented 5 years ago

Considering the awaited return, which happened in bolt version 1 and 2 (Neo4j < 3.4)

iex(5)> Bolt.Sips.transaction(Bolt.Sips.conn(), fn conn ->
...(5)> Bolt.Sips.query(conn, "RETRN 1 AS num")           
...(5)> end)
{:ok,
 {:error,
  [
    code: "Neo.ClientError.Statement.SyntaxError",
    message: "Invalid input 'R': expected 'u/U' (line 1, column 4 (offset: 3))\n\"RETRN 1 AS num\"\n    ^"
  ]}}

The same code in a bolt V3 Neo4j server:

iex(7)> Bolt.Sips.transaction(Bolt.Sips.conn(), fn conn ->  
...(7)> Bolt.Sips.query(conn, "RETRN 1 AS num")             
...(7)> end)
** (MatchError) no match of right hand side value: {:error, %Bolt.Sips.Internals.Error{code: "Neo.ClientError.Request.Invalid", connection_id: 10928, function: :commit, message: "Message 'COMMIT' cannot be handled by a session in the READY state.", type: :cypher_error}}
    (bolt_sips) lib/bolt_sips/protocol.ex:117: Bolt.Sips.Protocol.handle_commit/2
    (db_connection) lib/db_connection/holder.ex:270: DBConnection.Holder.holder_apply/4
    (db_connection) lib/db_connection.ex:1468: DBConnection.run_commit/3
    (db_connection) lib/db_connection.ex:1006: DBConnection.checkin/4
    (db_connection) lib/db_connection.ex:1450: DBConnection.commit/3
    (db_connection) lib/db_connection.ex:1377: DBConnection.run_transaction/4

This is due to how DBConnection handles the transaction:

But in Bolt v3, a rollback is automatically performed at server-level if query is invalid. All is required is a RESET message...

dominique-vassard commented 5 years ago

Investigation leads to discover some weird stuff. When server sends back a FAILURE message:

I opened a topic on Neo4j's forum in order to know if it's the expected behaviour or not.

florinpatrascu commented 5 years ago

a rollback initiated by the server sounds awkward, from the server' design perspective. I'm following your thread on on their forums and await for replies.

sashman commented 3 years ago

Are there any updates/breadcrumbs on this? I am facing a similar issue where I have a large number of queries failing in a transaction with:

** (Protocol.UndefinedError) protocol String.Chars not implemented for %MatchError{term: {:error, %Bolt.Sips.Internals.Error{code: "Neo.ClientError.Request.Invalid", connection_id: 352, function: :commit, message: "Message 'COMMIT' cannot be handled by a session in the READY state.", type: :cypher_error}}} of type MatchError (a struct). This protocol is implemented for the following type(s): Phoenix.LiveComponent.CID, Integer, Date, DateTime, Version, Version.Requirement, Float, NaiveDateTime, Atom, List, URI, BitString, Time
Anders2303 commented 3 years ago

Irregardless of whether the rollback should be initiated by the server, could we atleast make it so the error message contains the reason the query crashes (ala the error message we got in v.2)? As it stands, I need to manually print out the error reason to see what went wrong:


Bolt.Sips.transaction(Bolt.Sips.conn. fn conn -> 
 case Bolt.Sips.query(conn, cypher, attrs) do
   {:error, reason} ->
     IO.inspect(reason)
     throw(reason)
   res ->
     res
   end
end)
sukidhar commented 2 years ago

Considering the awaited return, which happened in bolt version 1 and 2 (Neo4j < 3.4)

iex(5)> Bolt.Sips.transaction(Bolt.Sips.conn(), fn conn ->
...(5)> Bolt.Sips.query(conn, "RETRN 1 AS num")           
...(5)> end)
{:ok,
 {:error,
  [
    code: "Neo.ClientError.Statement.SyntaxError",
    message: "Invalid input 'R': expected 'u/U' (line 1, column 4 (offset: 3))\n\"RETRN 1 AS num\"\n    ^"
  ]}}

The same code in a bolt V3 Neo4j server:

iex(7)> Bolt.Sips.transaction(Bolt.Sips.conn(), fn conn ->  
...(7)> Bolt.Sips.query(conn, "RETRN 1 AS num")             
...(7)> end)
** (MatchError) no match of right hand side value: {:error, %Bolt.Sips.Internals.Error{code: "Neo.ClientError.Request.Invalid", connection_id: 10928, function: :commit, message: "Message 'COMMIT' cannot be handled by a session in the READY state.", type: :cypher_error}}
    (bolt_sips) lib/bolt_sips/protocol.ex:117: Bolt.Sips.Protocol.handle_commit/2
    (db_connection) lib/db_connection/holder.ex:270: DBConnection.Holder.holder_apply/4
    (db_connection) lib/db_connection.ex:1468: DBConnection.run_commit/3
    (db_connection) lib/db_connection.ex:1006: DBConnection.checkin/4
    (db_connection) lib/db_connection.ex:1450: DBConnection.commit/3
    (db_connection) lib/db_connection.ex:1377: DBConnection.run_transaction/4

This is due to how DBConnection handles the transaction:

  • execute the query
  • rollback if there is an error

But in Bolt v3, a rollback is automatically performed at server-level if query is invalid. All is required is a RESET message...

I am experiencing similar crash, when query fails. anything to resolve this ?

sukidhar commented 2 years ago

Are there any updates/breadcrumbs on this? I am facing a similar issue where I have a large number of queries failing in a transaction with:

** (Protocol.UndefinedError) protocol String.Chars not implemented for %MatchError{term: {:error, %Bolt.Sips.Internals.Error{code: "Neo.ClientError.Request.Invalid", connection_id: 352, function: :commit, message: "Message 'COMMIT' cannot be handled by a session in the READY state.", type: :cypher_error}}} of type MatchError (a struct). This protocol is implemented for the following type(s): Phoenix.LiveComponent.CID, Integer, Date, DateTime, Version, Version.Requirement, Float, NaiveDateTime, Atom, List, URI, BitString, Time

Did you find any solution ?

florinpatrascu commented 2 years ago

No. PRs are very welcome.