Hi all. Why do function docs not seem to list all possible error tuples? For something like Phoenix.Token.Verify (https://hexdocs.pm/phoenix/Phoenix.Token.html#summary) which returns error tuples, when I look at the documentation it states as an example that {:error, :expired}, {:error, :invalid} may be returned, but I’m left wondering if there are more. The other functions in Phoenix.Token don’t state anything about possible errors being returned. Are there none? I’m trying to understand why this is ok? Are developers expected to go source code diving to find this?
Usually I’d expect such information to be available in the functions @spec, if though there is none, I usually treat values opaque and try to not make any assumptions.
Sadly sometimes one has to make assumptions, like in that case. I try to limit my assumptions to what is documented in examples and free form text.
The doc or spec is also where I expect it to be. My main problem for Phoenix.Token.Verify is that the possible errors were listed as examples. An example of what errors can be returned isn’t the same as a specific section for all functions stating here is exactly what will be returned. I feel @doc should fail and throw document generation errors if this proposed returns section isn’t filled out for every function.
Spacs are in general optional.
You can file a bug and ask for adding specs or to add a list of possible errors to the documentation.
Alternatively you can just do what I’d do and assume that the given error values from the example cover everything, or alternatively you could do a code dive and check if there are more possible error reasons.
1 Like
There’s also the reason of continuous development. The function does currently returns a set of errors, but spec’ing or documenting them might mean you have a breaking change at your hand if another error case is added. Leaving some things opaque leaves the door open to not have new major version all the time.
1 Like
Can you explain a bit why a new :error result would cause a breaking change? This is only in the case of the error results being specified in spec only right? Not if they were just documented in doc.
If some docs were to explicitly say, for example, that function do_stuff/0 can return either :ok, or {:error, :nope}, and nothing else (a spec like @spec do_stuff() :: :ok | {:error, :nope}), I could write some code like this:
case do_stuff() do
:ok ->
IO.puts("done :)")
{:error, :nope} ->
IO.puts("oh, come on!")
end
The problem is that, later, the project might need to add another error, say {:error, :already_did}, which would break my code because of a failed pattern match (I have no clause for {:error, :already_did} in my code above). Strictly speaking, this will have to be considered a breaking change for the project.
If instead the documentation says that do_stuff/0 can return :ok or {:error, cause} where cause is not a specific value, and adds that {:error, :nope} is one of the possible errors (a spec like @spec do_stuff() :: :ok | {:error, term}), that signals that I should better expect other possible errors, so I can write my code like this:
case do_stuff() do
:ok ->
IO.puts("done :)")
{:error, :nope} ->
IO.puts("oh, come on!")
{:error, _} ->
IO.puts("Stuff were not done because of some other poor excuse")
end
In this case, I am explicitly leaving it open for other kind of errors, so the introduction of {:error, :already_did} won’t be a breaking change.
1 Like
Ok got it. I think the general case for error results will always be to expect the developer to handle {:error, _}, leaving it open for new error results to be returned without causing breaking changes. For this scenario it would still be nice to see a very clearly defined Returns section stating all the current possible return errors. There should be a note or asterisk somewhere in there that states to the developer to handle {:error, _} for the functions still undergoing breaking changes.
1 Like
Looking at the source code of Phoenix.Token.verify\4, which calls Plug.Crypto.verify/4, it looks like the possible errors are {:error, :invalid}, {:error, :expired}, and {:error, :missing}, but the latter is only if nil is passed as the token. So the documentation is rather exhaustive, but I’d still account for other possible error patterns.
Documenting possible errors is definitely good practice. In this specific case, a @spec is missing too. I am quite sure that PRs to improve docs or specs are welcome 
1 Like
How is the documentation exhaustive when the {:error, :missing} case is not listed? Not even in the examples, which to me are just that. examples, not clear cut all possible return values list. Neither Phoenix.Token.Verify or Plug.Crypto.Verify list the missing case as possible returns. Again I don’t blame examples here, I blame a missing returns section not showing all possible error results and the caveat to handle future ones to devs if verify/4 is subject to breaking changes.
