hammox icon indicating copy to clipboard operation
hammox copied to clipboard

Better error formatting

Open dkuku opened this issue 3 years ago • 5 comments

This library is very usefull but I find the errors always so hard to read when it's inside a large struct because there are no newlines and when it's wrapped on 100 lines it looks like this (it's a simple example - this can be multiple screens long when refactoring legacy code):

     ** (Hammox.TypeMatchError) 
    Returned value {:ok, %Customers.Schemas.Customer{__meta__: #Ecto.Schema.Metadata<:built, "custom
ers">, first_name: "John", last_name: "Doe"}, []} does not match type {:ok, Customers.Schemas.Custom
er.t(), [integer()]} | {:ok, :no_customers} | {:error, :not_found}.
       Value {:ok, %Customers.Schemas.Customer{__meta__: #Ecto.Schema.Metadata<:built, "customers">,
 first_name: "John", last_name: "Doe"}, []} does not match type {:ok, Customers.Schemas.Customer.t()
, [integer()]} | {:ok, :no_customers} | {:error, :not_found}.
         2nd tuple element %Customers.Schemas.Customer{__meta__: #Ecto.Schema.Metadata<:built, "cust
omers">, first_name: "John", last_name: "Doe"} does not match 2nd element type Customers.Schemas.Cus
tomer.t().
           Map value "48 22 333 33 33" for key :contact_number does not match map value type EctoPho
neNumber.t() | nil.
             Value "48 22 333 33 33" does not match type EctoPhoneNumber.t() | nil.

I added some newlines and pretty printed the inspected structures - if it's a problem the inspect params can be set also in config. Another useful feature would be different colors for data and messages.

     ** (Hammox.TypeMatchError) 
     Returned value {:ok,
      %Customers.Schemas.Customer{
        __meta__: #Ecto.Schema.Metadata<:built, "customers">,
        first_name: "John",
        last_name: "Doe"
      }, []} does not match type 
     {:ok, Customers.Schemas.Customer.t(), [integer()]}
      | {:ok, :no_customers}
      | {:error, :not_found}
     
       Value {:ok,
        %Customers.Schemas.Customer{
          __meta__: #Ecto.Schema.Metadata<:built, "customers">,
          first_name: "John",
          last_name: "Doe"
        }, []} does not match type 
       {:ok, Customers.Schemas.Customer.t(), [integer()]}
        | {:ok, :no_customers}
        | {:error, :not_found}
     
         2nd tuple element %Customers.Schemas.Customer{
          __meta__: #Ecto.Schema.Metadata<:built, "customers">,
          first_name: "John",
          last_name: "Doe"
         } does not match 2nd element type 
         Customers.Schemas.Customer.t().
     
           Map value "48 22 333 33 33" for key :contact_number does not match map value type 
           EctoPhoneNumber.t()
            | nil.
     
             Value "48 22 333 33 33" does not match type 
             EctoPhoneNumber.t()
              | nil.

dkuku avatar Nov 29 '22 19:11 dkuku

human_reason returning multi-line strings
original left_pad
and without the need of putting padding into process memory

I had this implemented but decided to not to modify the original messages for now. I don't think the padding is needed at all when the message is properly formatted with new lines and some dividers - it may be useful for simple cases and short messages but with nested structs it does not work, even some of existing messges are too long to fit in one line and break the layout.

dkuku avatar Nov 30 '22 06:11 dkuku

Thanks for the PR!

The current Hammox messages are definitely quite unreadable and they do need work. Your proposal at a glance seems like a big readability improvement!

However I'm thinking about what you mentioned:

(it's a simple example - this can be multiple screens long when refactoring legacy code)

I work in a big codebase and I definitely experience this. There are not only very long stack traces, but also many tests in the suite failing and printing them. In this situation, I'm not sure we'd want to enforce pretty printing, as it would make the output so much longer.

Additionally, I think there is a deeper problem of displaying too much information in the first place. Currently the logic is pretty basic and we print whole structs for comparison. A better way to tackle the readability problem could be to be smarter about what is printed from the struct and decrease the total output. From this angle, pretty printing could potentially make the problem worse because it's easier to choose structs you want to dig into when they are on one line compared to them taking big chunks of screen space.

if it's a problem the inspect params can be set also in config.

I think this could be a reasonable way forward. A pretty: true config option for Hammox could allow us to experiment with and develop a pretty mode for those that prefer it while not changing the current default. What do you think?

msz avatar Dec 27 '22 18:12 msz

Config option may be the way to go, it should not be hard to implement. I can look into it.

dkuku avatar Dec 27 '22 18:12 dkuku

I added config and some description

dkuku avatar Dec 27 '22 20:12 dkuku

Hey what's the status for this @msz? Could this PR be merged?

boonious avatar Mar 30 '23 08:03 boonious