fuelen / ecto_dev_logger

An alternative logger for Ecto queries
Apache License 2.0
147 stars 14 forks source link

Crashing on a struct that gets persisted as JSON #17

Closed bhuntpenn closed 1 year ago

bhuntpenn commented 1 year ago

Hi there, I add Ecto.DevLogger.install(Brybags.Repo) to my Application.start/2 callback, and start the application.

The following crash is logged to the terminal.

10:46:32.388 [error] Handler [:ecto_dev_logger, :brybags, :repo] has failed and has been detached. Class=:error
Reason=%Protocol.UndefinedError{
  protocol: Ecto.DevLogger.PrintableParameter,
  value: %Cheese.Assignments{
    members: [%Cheese.Member{node: :"brybags@catah", can_provide: []}],
    assignments: [
      %Cheese.Assignment{
        service: Ingest.Providers.GoatsSupervisor,
        shard_id: 0,
        owning_node: nil
      },
      %Cheese.Assignment{
        service: Cheese.Registration.ButterSupervisor,
        shard_id: 1,
        owning_node: nil
      }
    ]
  },
  description: ""
}
Stacktrace=[
  {Ecto.DevLogger.PrintableParameter, :impl_for!, 1,
   [file: 'lib/ecto/dev_logger/printable_parameter.ex', line: 1]},
  {Ecto.DevLogger.PrintableParameter, :to_expression, 1,
   [file: 'lib/ecto/dev_logger/printable_parameter.ex', line: 31]},
  {Ecto.DevLogger, :"-inline_params/4-fun-1-", 3,
   [file: 'lib/ecto/dev_logger.ex', line: 109]},
  {Regex, :apply_list, 5, [file: 'lib/regex.ex', line: 756]},
  {Regex, :apply_list, 5, [file: 'lib/regex.ex', line: 751]},
  {Regex, :replace, 4, [file: 'lib/regex.ex', line: 686]},
  {Ecto.DevLogger, :"-telemetry_handler/4-fun-0-", 6,
   [file: 'lib/ecto/dev_logger.ex', line: 82]},
  {Logger, :__do_log__, 4, [file: 'lib/logger.ex', line: 884]},
  {:telemetry, :"-execute/3-fun-0-", 4,
   [
     file: '/Users/Bryan.Hunt/repos/brybags/devlogger_experiment/deps/telemetry/src/telemetry.erl',
     line: 135
   ]},
  {:lists, :foreach, 2, [file: 'lists.erl', line: 1342]},
  {Ecto.Adapters.SQL, :log, 4, [file: 'lib/ecto/adapters/sql.ex', line: 1078]},
  {DBConnection, :log, 5, [file: 'lib/db_connection.ex', line: 1559]},
  {Postgrex, :query_prepare_execute, 4, [file: 'lib/postgrex.ex', line: 361]},
  {Ecto.Adapters.SQL, :query!, 4, [file: 'lib/ecto/adapters/sql.ex', line: 438]},
  {Cheese.Adapters.Postgres.Server, :handle_call, 3,
   [file: 'lib/cheese/adapters/postgres/server.ex', line: 107]},
  {:gen_server, :try_handle_call, 4, [file: 'gen_server.erl', line: 721]},
  {:gen_server, :handle_msg, 6, [file: 'gen_server.erl', line: 750]},
  {:proc_lib, :init_p_do_apply, 3, [file: 'proc_lib.erl', line: 226]}
]

I then install the handler manually, Ecto.DevLogger.install(Brybags.Repo) and everything works fine.

I was thinking, perhaps I should implement a PrintableParameter like such, but doesn't seem to have taken effect (I did edit the dep source code directly, but in any event, I'm not sure what approach I should take).

  defimpl Ecto.DevLogger.PrintableParameter, for: Any do # or maybe 
    # test
    def to_expression(x) when is_map(x) , do: to_string_literal( Map.from_struct(x))
    def to_string_literal(x), do: Jason.encode!(Map.from_struct(x))
  end

Or would I have to do something like this?

  defimpl Ecto.DevLogger.PrintableParameter, for: Cheese.Assignments.. 
  defimpl Ecto.DevLogger.PrintableParameter, for: Cheese.Assignment..
  defimpl Ecto.DevLogger.PrintableParameter, for: Cheese.Member.. 

I'm asking because I'm wondering if I can add something to improve first-time user experience, it works fine by manually running Ecto.DevLogger.install(Brybags.Repo) - but I'm thinking of the next person.

fuelen commented 1 year ago

Hey @bhuntpenn

I then install the handler manually, Ecto.DevLogger.install(Brybags.Repo) and everything works fine.

It will work to the next invocation of the query that causes the error :)

I'd go with this:

defimpl Ecto.DevLogger.PrintableParameter, for: [Cheese.Assignments, Cheese.Assignment, Cheese.Member] do
  def to_expression(x) , do: to_string_literal(x)
  def to_string_literal(x), do: Jason.encode!(Map.from_struct(x))
end
bhuntpenn commented 1 year ago

Thanks for the help @fuelen !

fuelen commented 1 year ago

@bhuntpenn I just want to add, that there was one small bug with map representation, the end result has to be wrapped in quotes https://github.com/fuelen/ecto_dev_logger/commit/486c2bba192524049f09b9a4794c04be224bbde9 also, the changes you're doing are a result of updating from Ecto 3.9.0 to 3.9.1. Ideally, you wouldn't have to manually convert your structs to JSON. Here is a corresponding issue I raised today https://github.com/elixir-ecto/ecto_sql/issues/463

fuelen commented 1 year ago

Hey @bhuntpenn The fix is already in the main ecto_sql branch https://github.com/elixir-ecto/ecto_sql/pull/464