Reading a Dialyzer Error

Questions / Help
1

Hi Folks,

I am using dialyzer to do some static analysis on my code. So far, everything has been going great, but I am running into a warning that I don’t know how to parse. The warning is this:

posts_controller.ex:1: The call 'Elixir.CRM.PostController':authenticate(_@5::#{},[]) will never return since the success typing is (#{},#{}) -> #{} and the contract is ('Elixir.Plug.Conn':t(),#{}) -> 'Elixir.Plug.Conn':t()

Someonne mentioned that this is likely due to an dialyzer overspec setting being on. However, the documentation for dialyxir indicates that it no longer sets any of those flags (and I’m not sure that it ever set the overspec flag).

I tried changing the spec to something less strict, such as a map, but I still got the warning. Then, I tried making the function more strict (by making it pattern match on a %Plug.Conn{} struct, but that also gave the warning.

Anyway, would like to know what you folks think is the problem, and the potential solution.

Here is my code:

defmodule CRM.PostController do
  @moduledoc false

  use CRM.Web, :controller
  alias CRM.Authentication

  plug :authenticate when action in [:index]

  def index(conn, _params) do
    render(conn, "index.html")
  end

  @spec authenticate(Plug.Conn.t, map) :: Plug.Conn.t
  def authenticate(conn, params) do
    Authentication.require_authenticated(conn, params)
  end
end

Finally, here is the definition of authentication:

defmodule CRM.Authentication do
  @moduledoc ~S"""
  Defines a set of helper plugs for authentication.
  """

  import Phoenix.Controller, only: [put_flash: 3, redirect: 2]
  import Plug.Conn, only: [get_session: 2, assign: 3, halt: 1]

  @doc ~S"""
  Ensures that a user is authenticated, or halts the connection.
  """
  @spec require_authenticated(Plug.Conn.t, map) :: Plug.Conn.t
  def require_authenticated(%{assigns: %{current_user: nil}} = conn, _params) do
    conn
    |> put_flash(:error, "You must be logged in to access that page.")
    |> redirect(to: "/")
    |> halt
  end

  def require_authenticated(%{assigns: %{current_user: _user}} = conn, _params) do
    conn
  end
end
1 Like
2

I think your problem might be in the second argument (params) which you typespecced as a map, but in fact an empty list will be sent. That’s why dialyzer says:

The call ‘Elixir.CRM.PostController’:authenticate(_@5::#{},) will never return

Notice that the second argument here is an empty list, not a map.

2 Likes
3

@sasajuric

Thank you. I changed the spec to:

@spec authenticate(Plug.Conn.t, any) :: Plug.Conn.t

and

@spec require_authenticated(Plug.Conn.t, any) :: Plug.Conn.t

I’m satisfied with that fix since I ignore the second argument anyway, but I’m confused as to what code tried to pass an empty list in.

1 Like
4

Trying to answer my own question here.

The function CRM.PostController.authenticate/2 used the _, which indicates that the second argument can be anything at all. Meanwhile, the spec indicated that only a map can be the second argument.

Basically, dialyzer was warning me of this discrepancy. If I’m reading this right, the dialyzer message is confusing, because it leads me to believe that PostController.authenticate/2 with an empty list as the second argument will blow up - i.e., never return - when in fact it wont’.

1 Like
5

The contract of a plug says, that the second argument has to be of type Plug.opts, which is specified as opts :: tuple | atom | integer | float | [opts] (Plug-behaviour).

As you can see, by requiring it to be a map on your end is breaking a given contract and as such dialyzer complains. Correct way of handling that error were to actually exactly fulfill the contracts expectation or a subset thereof and not to use a superset which might break at another stage of development (perhaps you will use that parameter in future).

1 Like
6

If I read Phoenix code correctly, the plug macro is defined here, and uses the default params value of [].

1 Like
8

@sasajuric / @NobbZ -

Thank you both for pointing me to the docs and helping me answer the question.

The reason I was confused is because I had the incorrect idea that actions in Phoenix Controllers are valid plugs, but that can’t be the case since they do not have the correct type signature.

Thank you!

1 Like
9

FWIW, I personally think of actions as plugs, and this is what I usually say in my talks on Phoenix. That said, you do raise a good point that according to typespec, opts/params can’t be a map, which means that an action functions is strictly speaking not a plug.

I wonder what’s the reason for such typespec, i.e. why is the set of plug opts types restricted (cc @josevalim @chrismccord)?

1 Like
10

I believe that line was written before we had maps. :slight_smile: Great catch, I have fixed it in Plug master.

6 Likes
11

@josevalim Thank you for the update.

Given the new type for Plug.opts, is it still the case that passing a map with binary keys would break the contract? If so, then controller actions are not still not valid plugs right, since often we’d pass in something like %{"id" => 1}? (Just making sure that that was the intended result).

2 Likes
12

Good catch, binaries are also fine.

3 Likes
13

Awesome! Thank you all for your help!

2 Likes
14

really they are valid plugs in that they abide by the plug contract. For dispatch, the controller calls the action/2 plug as the last plug in the pipeline which simply calls the function:

def action(conn, opts) do
  apply(__MODULE__, action_name(conn), [conn, conn.params])
end
3 Likes
15

Thanks Chris!

1 Like