Elixir in Action Application structure is wrong

Background

I have a project that I have recently converted to run as an OTP application. Upon doing this I came back to the “Elixir in Action” book and replay the same steps - but I found something is wrong in the book.

Code

This is how @sasajuric defines an Application (I have added typespec for calrity):

defmodule Todo.Application do
  use Application
  
  @impl Application
  @spec start(any, any) :: Supervisor.on_start
  def start(_, _) do
    Todo.System.start_link()
  end
end

Simple enough. Todo.System is a simple supervisor:

defmodule Todo.System do

  @spec start_link() :: Supervisor.on_start
  def start_link do
    Supervisor.start_link([...children...], strategy: :one_for_one)
  end
end

Problem

At first glance it seems like everything is as it should be. But dialyzer pointed me a problem:

The return type ‘ignore’ in the specification of start/2 is not a subtype of {‘error’,} | {‘ok’,pid()} | {‘ok’,pid(),}, which is the expected return type for the callback of the ‘Elixir.Application’ behaviour(undefined)

This all means that what Todo.System.start_link returns (Supervisor.on_start which expands to something similar to{:ok, pid} | :ignore | {:error, any}) is not compatible with what Todo.Application.start should return: :ok | {:error, term}.

Now, I am not sure what consequences this can have in the long term. The apps launch for now, but this may change on latter updates.

Am I missing something?

But maybe I am missing something? Maybe dialyzer is not smart enough to see some hidden compatibility going on and I am just crying wolf for nothing. What do you think? Is my analysis correct?

On the other hand, if this is in fact wrong, it would be nice if the author could issue a fix (perhaps in the 3rd edition :stuck_out_tongue: )

https://hexdocs.pm/elixir/Application.html#start/2
is not the same as
https://hexdocs.pm/elixir/Application.html#c:start/2

So the expected return type of Todo.Application.start/2 should be {:ok, pid()} | {:ok, pid(), state()} | {:error, reason :: term()}, but you’re correct that :ignore is not allowed as return value for the application.

2 Likes

This is not how I defined it, the typespec is your own addition :slight_smile:

You are :slight_smile: The error message states that ignore return type of the specification is not the subtype of the expected return type for the callback of the application behaviour.

Wher does ignore come from? It’s a part of Supervisor.on_start type.

The Application.start spec in contrast states that valid return values are {:ok, pid()} | {:ok, pid(), state()} | {:error, reason :: term()} (notice the lack of :ignore).

So basically the typespec you provided for Todo.Application.start is incorrect. You could use something like @spec start(Application.start_type, term) :: {:ok, pid} instead.

4 Likes

Ahhh, good point !! My bad!

Ahh, yes, that is correct. I am merely trying to build on the shoulders of titans :stuck_out_tongue:

This is where I have trouble understanding the issue. If Todo.System.start_link returns the type Supervisor.on_start, then your Todo.Application.start, which returns whatever Todo.System.start_link returns, should return the same.

The error should still be there, as Todo.System.start_link can in theory still return :ignore, thus causing the signature to be different. Am I miss interpreting something?

Sure, go ahead, just don’t credit me with your own work :stuck_out_tongue:

The return value of the start callback is a subset of possible return values of Supervisor.start_link. As long as Supervisor.start_link returns that subset, everything is fine. Otherwise, the app will fail to start with an error returned a bad value: :ignore.

Keep in mind that dialyzer can’t detect all possible errors. In fact, in my informal estimate, I feel that dialyzer mostly doesn’t catch errors. In this particular example, I’m pretty sure it would miss the case where :ignore is returned, since that situation is triggered by another process (the supervisor process, if our supervisor callback returns :ignore from it’s init).

If everything was running in the same process, the dialyzer would maybe catch a situation where the called function returns a value which is not allowed by the spec.

1 Like

Edited the original post for the sake of clarity and to not shame you with my horrible work :stuck_out_tongue:

Wouldn’t something like this be more resilient then?

@impl Application
@spec start(any, any) :: {:ok, pid} | {:error, :ignore | any}
def start(_type, _args) do
  case System.start_link() do
    {:ok, pid}  ->  {:ok, pid}
    :ignore     -> {:error, :ignore}
    error       -> error
  end
end

Or doesn’t it make sense at all?

I personally wouldn’t bother. Returning :ignore from the top-level supervisor is a bug, and so it’s not something I wouldn’t deal with in the code, since either way the app won’t start, and the original error message is IMO clear enough.

1 Like

As a reader and a fan of your work, and as someone who bought your book to learn Elixir, I believe it would be nice to mention this detail.

Yes, it may be completely useless for someone who already knows Supervisors and GenServers and the interactions with init. It may even be useless for someone who simply copy/pastes your code and runs it.

But for someone like me, who genuinely tries to understand the things that are going on and how to piece everything together using dialyzer in order to make (ever so slightly) more robust apps, such a paragraph could help.

It also prevent idiots like me from creating posts like this :stuck_out_tongue:


PS: This is just my opinion, you don’t have to agree :stuck_out_tongue:

Your post asks a perfectly valid question, no need to be hard on yourself.

I’ll keep it in mind for the next edition, thanks!

As the author, I feel I should point out that EiA isn’t a reference book, and so it doesn’t treat every nuance of the language. Even the aspects it talks about aren’t covered in full depth. There’s simply no room for that, but more importantly, I feel that I would lose most of the readers if I went to fine-print detail of every single topic I’m discussing. Thus, some nuances will inevitably be left out, so don’t be surprised if you will have further questions.

2 Likes

On a personal level, the main reason I recommend your book to people around me in work (besides liking it obviously) is the fact I you are so active in this community.

If one of my co workers has a question about your book that I can’t answer, they can always ask the creator for help. And that is priceless.

3 Likes

Happy to hear that!

I think what’s even more important is that we have a nice community here, so different people can chime in with different points of view, like e.g. @LostKobrakai did earlier in this thread by spotting the difference between Application.start and the start callback. This is why I usually advise people to ask their questions on this forum.

3 Likes