Fixed an issue where a Bolt.Sips.Internals.Error{} would be processed…

[kristofka]

Added tests demonstrating requests timing out. However, those tests require the apoc.util.sleep procedure from apoc. Therefore, they're excluded from the test suite by default. Run them with mix test --include apoc.

Fixed an issue where passing a timeout value to the query options wouldn't work. A recv_timeout value was set to 10s in Bolt.Protocol.Helper, leading to the tcp.recv call timing out.

Now, recv is set to :infinity when running in the calling code. Thus, misbehaving queries will be handled by DbConnection, insuring that disconnect is called and the connection is properly cleaned up. I had to remove the retries from the private execute function, since I couldn't find a sane way to manage timeouts in that context. I don't believe it's a major issue, since DbConnection will try to acquire a new connection with configurable backoff upon disconnect. It also seems to be more in line with the way DbConnection handles code called from the connection (ie connect ...) and code called from the client (ie handle_execute ...).

Updated the documentation accordingly.

Also :

Added a bench tag to the performance tests and excluded them from the test suite by default. The result of those tests is mostly dependant on the environment. It is now possible to run them with mix test --include bench.

Fixed an issue in the performance test where success of the tests would depend on the order they're run.

Fixed an issue where a Bolt.Sips.Internals.Error{} would be processed twice, leading to incorrect error code.

[sourcelevel-bot[bot]]

SourceLevel has finished reviewing this Pull Request and has found:

• 296 possible new issues (including those that may have been commented here).

See more details about this review.

[florinpatrascu]

this is very nice, thank you @kristofka! I have a question, since you removed the retry support, do we still need its dependency? {:retry, "0.9.1"}, respectively?

[kristofka]

@florinpatrascu I don’t think we need the dependency anymore (except for one test that relies on it, namelyretry_backoff_test.exs).
I didn’t want to remove it before you had a chance to look at the commit. But I agree that removing it would make sense.

[florinpatrascu]

that's not needed anymore. Plus, that dependency is old - the newer version wasn't doing exactly what we wanted. The reason for retying the connections is historical. Certain hosted services were very slow in granting a connection, hence the need for retry. Hopefully our users will not experience that type of connectivity?!

[kristofka]

@florinpatrascu I have removed the dependency with that latest commit.

[florinpatrascu]

@kristofka w⦿‿⦿t!

[sascha-wolf]

(for completeness sake to link this PR to the issue)

Fixes #83