EctorErrorSerializer not available

[thewalkingtoast]

Using either 0.4.0 or HEAD, under test we get ** (UndefinedFunctionError) undefined function: JaSerializer.EctoErrorSerializer.format/1 (module JaSerializer.EctoErrorSerializer is not available) a lot. When ran in dev, it works just fine with error responses.

Mix.exs:

  defp deps do
    [{:phoenix, "~> 1.0.3"},
     {:phoenix_ecto, "~> 1.1"},
     {:postgrex, ">= 0.0.0"},
     {:phoenix_html, "~> 2.1"},
     {:phoenix_live_reload, "~> 1.0", only: :dev},
     {:cowboy, "~> 1.0"},
     {:ja_serializer, "~> 0.4.0"},
     {:dogma, "~> 0.0.10", only: :dev}]
  end

[alanpeabody]

Thanks for the report @thewalkingtoast. I will look into it.

[alanpeabody]

@thewalkingtoast I am having a bit of a tough time reproducing this.

I am wondering if it is a compilation issue, could you give the branch a try and see if it help?
https://github.com/AgilionApps/ja_serializer/tree/fix-ecto-dep-compliation-issues

[thewalkingtoast]

@alanpeabody No change. Here is the exact dump from mix test:

1) test does not update resource and renders errors when data is invalid (InterpreterServer.UserControllerTest)
     test/controllers/user_controller_test.exs:130
     ** (UndefinedFunctionError) undefined function: JaSerializer.EctoErrorSerializer.format/1 (module JaSerializer.EctoErrorSerializer is not available)
     stacktrace:
       JaSerializer.EctoErrorSerializer.format(%{__struct__: Ecto.Changeset, action: :update, changes: %{first_name: "!", start_year: "0"}, conn: %Plug.Conn{adapter: {Plug.Adapters.Test.Conn, :...}, assigns: %{__struct__: Ecto.Changeset, action: :update, changes: %{first_name: "!", start_year: "0"}, constraints: [], errors: [start_year: {"should be %{count} characters", [count: 4]}, first_name: "has invalid format"], filters: %{}, layout: false, model: %InterpreterServer.User{__meta__: #Ecto.Schema.Metadata<:loaded>, address: nil, avatar: nil, deleted_at: nil, first_name: "Arya", id: 1, inserted_at: #Ecto.DateTime<2015-10-27T14:40:19Z>, last_name: "Stark", miles_willing_to_travel: nil, start_month: "04", start_year: "2011", updated_at: #Ecto.DateTime<2015-10-27T14:40:19Z>}, optional: [:avatar, :miles_willing_to_travel, :deleted_at], opts: [source: :changeset], params: %{"first_name" => "!", "last_name" => "Stark", "start_month" => "04", "start_year" => "0"}, repo: InterpreterServer.Repo, required: [:first_name, :last_name, :start_year, :start_month], types: %{address: {:assoc, %Ecto.Association.Has{cardinality: :one, defaults: [], field: :address, on_cast: :changeset, on_delete: :fetch_and_delete, on_replace: :raise, owner: InterpreterServer.User, owner_key: :id, queryable: InterpreterServer.Address, related: InterpreterServer.Address, related_key: :user_id}}, avatar: :string, deleted_at: Ecto.DateTime, first_name: :string, id: :id, inserted_at: Ecto.DateTime, last_name: :string, miles_willing_to_travel: :integer, start_month: :string, start_year: :string, updated_at: Ecto.DateTime}, valid?: false, validations: [start_month: {:length, [is: 2]}, start_year: {:length, [is: 4]}, last_name: {:format, ~r/[^`!<>@#\$%\^&*+_=]+/}, first_name: {:format, ~r/[^`!<>@#\$%\^&*+_=]+/}]}, before_send: [#Function<0.61510215/1 in JaSerializer.ContentTypeNegotiation.set_content_type/2>, #Function<1.25101471/1 in Plug.Logger.call/2>], body_params: %{"data" => %{"attributes" => %{"first_name" => "!", "last_name" => "Stark", "start_month" => "04", "start_year" => "0"}, "id" => "1", "type" => "users"}}, cookies: %Plug.Conn.Unfetched{aspect: :cookies}, halted: false, host: "www.example.com", method: "PATCH", owner: #PID<0.2408.0>, params: %{"data" => %{"attributes" => %{"first_name" => "!", "last_name" => "Stark", "start_month" => "04", "start_year" => "0"}, "relationships" => nil}, "id" => "1"}, path_info: ["api", "v1", "users", "1"], peer: {{127, 0, 0, 1}, 111317}, port: 80, private: %{InterpreterServer.Router => {[], %{}}, :phoenix_action => :update, :phoenix_controller => InterpreterServer.UserController, :phoenix_endpoint => InterpreterServer.Endpoint, :phoenix_format => "json-api", :phoenix_layout => {InterpreterServer.LayoutView, :app}, :phoenix_pipelines => [:api], :phoenix_recycled => true, :phoenix_route => #Function<8.92130501/1 in InterpreterServer.Router.match/4>, :phoenix_router => InterpreterServer.Router, :phoenix_template => "error-object.json", :phoenix_view => InterpreterServer.ChangesetView, :plug_skip_csrf_protection => true}, query_params: %{}, query_string: "", remote_ip: {127, 0, 0, 1}, req_cookies: %Plug.Conn.Unfetched{aspect: :cookies}, req_headers: [{"accept", "application/vnd.api+json"}, {"content-type", "application/vnd.api+json"}], request_path: "/api/v1/users/1", resp_body: nil, resp_cookies: %{}, resp_headers: [{"cache-control", "max-age=0, private, must-revalidate"}, {"x-request-id", "q8hdjseum05p8ldgivlphmf16ev5s5p8"}, {"content-type", "application/vnd.api+json; charset=utf-8"}], scheme: :http, script_name: [], secret_key_base: "4o7<snip>", state: :unset, status: 422}, constraints: [], errors: [start_year: {"should be %{count} characters", [count: 4]}, first_name: "has invalid format"], filters: %{}, model: %InterpreterServer.User{__meta__: #Ecto.Schema.Metadata<:loaded>, address: nil, avatar: nil, deleted_at: nil, first_name: "Arya", id: 1, inserted_at: #Ecto.DateTime<2015-10-27T14:40:19Z>, last_name: "Stark", miles_willing_to_travel: nil, start_month: "04", start_year: "2011", updated_at: #Ecto.DateTime<2015-10-27T14:40:19Z>}, optional: [:avatar, :miles_willing_to_travel, :deleted_at], opts: [source: :changeset], params: %{"first_name" => "!", "last_name" => "Stark", "start_month" => "04", "start_year" => "0"}, repo: InterpreterServer.Repo, required: [:first_name, :last_name, :start_year, :start_month], types: %{address: {:assoc, %Ecto.Association.Has{cardinality: :one, defaults: [], field: :address, on_cast: :changeset, on_delete: :fetch_and_delete, on_replace: :raise, owner: InterpreterServer.User, owner_key: :id, queryable: InterpreterServer.Address, related: InterpreterServer.Address, related_key: :user_id}}, avatar: :string, deleted_at: Ecto.DateTime, first_name: :string, id: :id, inserted_at: Ecto.DateTime, last_name: :string, miles_willing_to_travel: :integer, start_month: :string, start_year: :string, updated_at: Ecto.DateTime}, valid?: false, validations: [start_month: {:length, [is: 2]}, start_year: {:length, [is: 4]}, last_name: {:format, ~r/[^`!<>@#\$%\^&*+_=]+/}, first_name: {:format, ~r/[^`!<>@#\$%\^&*+_=]+/}]})
       (phoenix) lib/phoenix/view.ex:341: Phoenix.View.render_to_iodata/3
       (phoenix) lib/phoenix/controller.ex:628: Phoenix.Controller.do_render/4
       (interpreter_server) web/controllers/user_controller.ex:1: InterpreterServer.UserController.action/2
       (interpreter_server) web/controllers/user_controller.ex:1: InterpreterServer.UserController.phoenix_controller_pipeline/2
       (interpreter_server) lib/phoenix/router.ex:255: InterpreterServer.Router.dispatch/2
       (interpreter_server) web/router.ex:1: InterpreterServer.Router.do_call/2
       (interpreter_server) lib/interpreter_server/endpoint.ex:1: InterpreterServer.Endpoint.phoenix_pipeline/1
       (interpreter_server) lib/phoenix/endpoint/render_errors.ex:34: InterpreterServer.Endpoint.call/2
       (phoenix) lib/phoenix/test/conn_test.ex:193: Phoenix.ConnTest.dispatch/5
       test/controllers/user_controller_test.exs:139

ChangesetView:

defmodule InterpreterServer.ChangesetView do
  @moduledoc """
  Shows Ecto changeset errors in JSON:API compliant fashion.
  """

  use InterpreterServer.Web, :view
  # use JaSerializer.PhoenixView defined in web/web.ex

  def render("error-object.json", changeset) do
    JaSerializer.EctoErrorSerializer.format(changeset)
  end
end

[thewalkingtoast]

I was just able to trigger this locally in dev. No clear answers yet but I feel better knowing it's not just test now.

Interestingly, I had ran rm -rf deps _build, mix deps.get, and mix test and got the error. I then repeated the process and the tests pass. Something to do with how mix is grabbing this repo or how it is being compiled?

[alanpeabody]

Yeah, I am assuming it is some kind of compilation/dependency issue. If Ecto is not compiled it will not define the EctoErrorSerializer module, and you will get this error. I thought my branch change might effect it, but seemingly not.

To be honest I am thinking about opinionating more towards Ecto and making the dependency required, not optional and removing the compilation conditionals.

I am going to play with this a bit more with what you provided above to see if blowing away the deps and build helps me to re-create it.

[alanpeabody]

Thanks I was able to reproduce!

Unfortunately I am a bit of a loss. The Code.ensure_compiled?(Ecto) seemingly has no effect when building the dependencies for the first time.

I will probably push a 0.5 release that adds Ecto as a required (non-optional) dependency this weekend.

As part of that push, I may add a error serializer to the existing view, similar to what you have defined.

[alanpeabody]

@thewalkingtoast I do have a workaround to offer in the meantime:

Add ecto as a dependency of your project directly, not just via phoenix_ecto.

[thewalkingtoast]

@alanpeabody That worked wonderful! Thank you very much for your work! 🎆

[alanpeabody]

@thewalkingtoast I am having trouble convincing myself I should make ecto a required dependency.

Do you think the direct dependency requirement is too much of a pain, especially if called out in the README?

[thewalkingtoast]

@alanpeabody That sounds good to me. Thanks again!

[hunterboerner]

Most people use Ecto. If there's a complaint about Ecto being required then it should be reconsidered.

[bcardarella]

I believe that making Ecto a required dependency was a mistake. There are good cases for not wanting Ecto as it pulls in so many dependencies, but beyond that having a Ecto versioned dep becomes problematic when you are dealing with other libraries. I think Ecto should be removed as a required dep.

[bcardarella]

To better illustrate the issue, I now have to add this to my mix.exs file to avoid conflicts:

{:ecto, "> 0.0.0", override: true}

[alanpeabody]

Thanks for the feedback @bcardarella. I need to spend some time understanding how dependencies + Code.ensure_compiled?/1 works before I can commit to anything.

[nurugger07]

I'm a +1 for removing the Ecto dependency. I would prefer to see example implementations for ecto/phoenix rather that modules in a serializer. I know that phoenix isn't a dep but having specific logic to handle a framework, or external library, seems out of place for a serializer library. I wouldn't mind contributing to the effort.

[alanpeabody]

@nurugger07 you mean encouraging people to write their own render functions in views and something to add to the error and changeset views?

It really isn't much code so I am not opposed to that. Frankly my biggest concern is generating more confusion/issues around "how do I use this with phoenix?" that have to be answered, but maybe forcing explicitness will reduce that?

[nurugger07]

@alanpeabody yes. I think the docs, README, sample apps, or even link to a blog post are all great ways to teach people how to use libraries in specific situations. It also gives them a better understanding of what's happening and how they can alter it for their needs.

[alanpeabody]

The phoenix view and error serializer piece would be easy enough. Do you have suggestions on how some of the other existing pattern matches against Ecto structs could be handled?

In particular these two I don't have an answer too:

• The Ecto.DateTime formatter is important for performance: https://github.com/AgilionApps/ja_serializer/blob/78aae84ef6efc56bf20b1aac41cb33ce5766cab7/lib/ja_serializer/formatter.ex#L18
• The Ecto.Association.NotLoaded gives a user friendly contextual error: https://github.com/AgilionApps/ja_serializer/blob/901d7a50b5efa6328f6d598a392c89305e94cd23/lib/ja_serializer/relationship.ex#L96

Any help, especially with documentation (something I struggle with), is very much appreciated!

[bcardarella]

@alanpeabody I believe you can just pattern match on a map with the expected __struct__ key instead:

  def get_data(struct, name, opts) do
    rel = (opts[:field] || name)
    struct
    |> Map.get(rel)
    |> case do
      %{__struct__: Ecto.Association.NotLoaded} -> raise @error, rel: rel, name: name
      other -> other
    end
  end

I think this will allow you to get away with not having Ecto as a required dependency while still supporting this use-case if someone happens to be using Ecto

[alanpeabody]

That makes sense @bcardarella, thanks!

Any ideas on the protocol implementation? My concern with removing it is that falling back to the Any fallback is awfully slow.

[bcardarella]

Does Elixir throw a compilation error if Ecto is not defined? I would have thought that Elixir would just treat these as atoms internally?

[alanpeabody]

Doesn't appear it does raise an error. I have a branch with ecto moved to a test only dependency.

I am a little unclear if moving a dependency to only: :test actually resolves this issue for the end user. If not it isn't much work to remove it completely.

[totallymike]

Elixir shouldn't have a problem with matching against modules that don't exist. As @bcardarella said, it'll just treat it as an atom. This is the same as when you type a made-up name into the REPL and it just returns it to you.

I think the worst-case scenario is that one will haven to add their own render functions to MyApp.Web/view to catch the common case, which I find acceptable.