Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve exception message for Jason.Encoder errors #4534

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

ivanov
Copy link
Contributor

@ivanov ivanov commented Oct 29, 2024

Thanks for all that you do, including continued maintenance of ecto, and now taking the time to consider this small pull request.

I decided it was too much to replicate the full contextual block that was already there just to add one alternative @derive... line, and thought maybe a commented out approach would be both simple and useful.

lib/ecto/json.ex Outdated
Comment on lines 12 to 21
Or choose to not encode the association when converting the struct \
to JSON by explicitly listing the JSON fields in your schema:
to JSON by explicitly listing the JSON fields in your schema, or by \
excluding it (commented out):

defmodule #{inspect(owner)} do
# ...

@derive {Jason.Encoder, only: [:name, :title, ...]}
# @derive {Jason.Encoder, except: [#{inspect(field)}]}
schema ... do
Copy link
Member

@warmwaffles warmwaffles Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One alternative here is to show two separate examples without having a directive to uncomment a line.

    @doc """


      Or choose to not encode the association when converting the struct \
      to JSON by explicitly listing the JSON fields in your schema:

          defmodule #{inspect(owner)} do
            # ...
            @derive {Jason.Encoder, only: [:name, :title, ...]}
            schema ... do

      Or choose to not encode the association by excluding it in your schema:

          defmodule #{inspect(owner)} do
            # ...
            @derive {Jason.Encoder, except: [#{inspect(field)}]}
            schema ... do

 
    """

I know that when I am looking at docs, I usually gravitate to the examples without paying attention to the helpful text above it 💀.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to do convert to that style, if that's preferred (also happy if maintainers want to adjust this in that direction). The only thing to add is that this isn't part of narrative documentation, this is traceback you will get if your code causes this error, so that's why I was trying to lean towards being more terse.

@josevalim
Copy link
Member

I believe except is anti-pattern because it is easy to leak information you don't want but it is ok to mention it. :)

@josevalim josevalim merged commit a54d96f into elixir-ecto:master Oct 30, 2024
6 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants