search

elixir-ecto / db_connection

Database connection behaviour
http://hexdocs.pm/db_connection/DBConnection.html
309 stars 112 forks source link

Handle exceptions in Query/Connection callbacks in client process #51

Closed fishcakez closed 8 years ago

fishcakez commented 8 years ago

This adds some major changes in error handling so requires tests plus review. I plan to add a commit which does more error handling in prepare_execute (closes prepared query if describe or encode raises).

fishcakez commented 8 years ago

Note for future: this will require handling result of {kind, reason, stack} in Ecto.LogEntry

josevalim commented 8 years ago

It looks good to me but we need to be careful with not breaking current Ecto versions as soon as we release this.

fishcakez commented 8 years ago

@josevalim: Shall we solve this by rewriting {kind, class, stack} as {:error, %DBConnection.ConnectionError{}} when logging for now?

I have added the close on raising in describe/encode - we need this for https://github.com/elixir-ecto/postgrex/issues/219.

josevalim commented 8 years ago

@fishcakez that would probably be better, yes.

fishcakez commented 8 years ago

@josevalim ok tests have been added.

With prepare_execute we now close the query on describe exception. Should we do the same for prepare? I think we have to for consistency and because the query did not finish preparing . This means a slower checkin if just doing a prepare but that situation is rare - usually inside a transaction.

josevalim commented 8 years ago

With prepare_execute we now close the query on describe exception. Should we do the same for prepare? I think we have to for consistency and because the query did not finish preparing . This means a slower checkin if just doing a prepare but that situation is rare - usually inside a transaction.

Sounds like the way to go indeed.