Ecto silently crashes when given insufficient config

[spacebat]

Elixir version

1.16.2

Database and Version

PostgreSQL 14

Ecto Versions

3.11.1

Database Adapter and Versions (postgrex, myxql, etc)

Postgrex 0.17.5

Current behavior

Find your favourite phoenix app with an ecto repo as one of the direct children of the application supervisor
Delete the :database key from that repo's config
Start the application with iex -S mix phx.server
The application crashes without giving a reason

Expected behavior

Ecto should log when there is an error in the configuration, so that when it crashes due to this there is some indication where to start debugging the problem.

This isn't a big deal but it could save people debugging time in future.

[v0idpwn]

DbConnection.Connection crashes, because Postgrex.Protocol expects the :database key to be present. If OTP/SASL logs are disabled, there are zero logs about what went wrong. Supervisors quickly die due to restart intensity.

[v0idpwn]

There are different alternatives to fix this. We can add some validation in Postgrex.Protocol, returning a proper error that is handled by DbConnection and logged (like it's done for invalid credentials). Another option is to perform some validation earlier, so that we don't even attempt to start up connections if the config wasn't supplied as expected.

Do you have any opinion on this? I can send a PR @josevalim @greg-rychlewski
I haven't looked thoroughly, but I think the same issue happens for some other misconfigurations, and not only for missing :database.

[spacebat]

I like the idea of expanding the invalid credentials approach to report these misconfigurations too.

[josevalim]

We can log in more cases for sure but I don’t see the need to stop it from crashing. :) PRs definitely welcome!

[v0idpwn]

Here's a potential fix: elixir-ecto/postgrex#671
It makes the function return an error tuple instead of raising an exception.

As I mentioned, I think a similar situation may happen for other errors.

@josevalim Maybe instead of this, DbConnection should could handle exceptions better and log about them? Seems like it's more guaranteed to cover all cases. But adding it now may produce duplicate/unnecessary logs.

[v0idpwn]

Closing in favor of the fix in elixir-ecto/postgrex#671
I'm going to invest some time into other keys and other connection adapters