db_connection icon indicating copy to clipboard operation
db_connection copied to clipboard

Alter functions to pass error tuple instead of raising errors

Open jbavari opened this issue 2 years ago • 1 comments

As per #260 , we wanted a way to prevent raising errors and instead return error tuples.

I may be missing something - seeking some additional guidance to figure out what's needed to make it return error tuple instead of raising.

  • Configuration - right now no clear pattern exists in db_connection. Either the configuration would have to be passed down to prevent raising, or have a simple Application.get_env, but keep sane defaults to raise. We do not want to disrupt all consumers of the db_connection library.
  • All the consumers of db_connection would appear to need to adhere to raise vs error tuple, if indeed we did get this change in place.
  • Tests around DBConnection (where the raise'ing happens) is very limited. My changes are quite fragile at the moment.
  • Concerns around how to set up mocks/tests to get the raise/error tuples to trigger.
  • By passing the tuples up, the call stack would receive unintended error tuples.
  • An example Repo may have: UserApplication.Repo -> Ecto.Repo -> Ecto.Adapters.Postgres -> Ecto.Repo.Schema -> Ecto.Adaptesr.SQL -> Ecto.Adapters.SQL.Connection -> Ecto.Adapters.Postgres.Connection -> DBConnection.execute

Would love any feedback or suggestions the team would have. From where I sit, I would even think just overriding the userspace Repo.insert call to try/catch may be an easier fit over all. Happy to explore with the team!

jbavari avatar Jun 21 '22 00:06 jbavari

Using the application environment is a no-no but you can pass this as an option. However, as much as it pains me to say this, this will definitely have further implications in transactions and similar, so the best is to most likely to rescue the exception. :(

josevalim avatar Jun 21 '22 07:06 josevalim