Should `Base.decodeXX` functions return more than just `:error`?

To me this feels like you try to change something just because it’s inconvenient with with. That’s imho not a reason to change anything. You’ll have the same issues with not just the Base module’s functions, but with many other functions.

I feel like changing the return types of the Base module might be justified if the additional second error atom would provide more details about the error, but in this case there’s only one error case: Can’t be decoded.

You can “isolate” Base.decodeXX errors by using case instead of with:

case Base.decode64(binary) do
  {:ok, decoded} ->
    case do_something_with_decoded(decoded) do
      {:ok, result} = success -> success
      failure -> failure
    end

  :error ->
    {:error, :bad_encoding}
end

As for your suggestion, what tuples other than {:error, :bad_encoding} do you have in mind for failed base.decodeXX?

I understand that the function is not “wrong” in its current form. That is not my intention here. Nor is it to make it more “convenient” with with. My intention is to help our code bases from the point of view of introspection.

Even if there is currently only one possible return, consider these 2 forms:

• : error
• {:error, :bad_encoding}

Tell me which gives you more information about where to search for the cause of this? For another example, think of an action_fallback in Phoenix with this def:

  def call(conn, :error) do
    conn
    |> put_status(:bad_request)
    |> json(%{"error" => "Some :error"})
  end

How many “things” can fall into that clause?

Without seeing the stacktrace, you would need to “know” that decoding functions in the Base module might return just :error. The difference is that when you see {:error, :bad_encoding} you’d probably grep your source code for decoding functions. IMHO the second option is better.

Also, if we “know” before hand that the core elixir libraries “always” return tagged tuples, then it would be least surprising where to look for that.

I think if you return a tuple is because you’re returning more information about the error. However, {:error, :bad_encoding} is not giving you more information than {:error}. If I would need this for a with, I would create a simple wrapper:

defmodule MyBase do
  def decode64(bin) do
    case Base.decode(bin) do
      :error -> {:error, :bad_encoding}
      any -> any
    end
  end
end

The more information, in this case, is that this is an error from a decoding function. We could go even further and provide {:error, :bad_base64_encoding} and so on.

Of course you can wrap the function and that is valid even for functions that DO return error tuples, but my point here is about how you instrospect errors. If you “know” beforehand this return is possible from decode functions, then sure, you can wrap it. But what if you don’t know this and has this log on your production system “Some error”?

So: :error is not as obvious as {:error, :bad_encoding} to my understanding at least. What I was hoping to achieve here is to provide just what you people are saying: being obvious about what happened in an error other than just saying “an error occured”.

In any case, it seems like I am alone in this quest hehehe that’s fine too. I thought this would turn out the other way around but it is not like there is a wrong or right here.

Just to add to this, I can see where you’re coming from and I also remember a couple of situations in which I wished some function returned something more than just :error.

But I think in this case the return value is correct since the only type of error is “can’t be decoded”. {:error, :bad_encoding} would then be needlessly specific just as Map.fetch/2 #=> {:error, :key_not_found} would also be needlessly specific.

However it seems that Elixir is not entirely consistent in this either For example Date.new/4 returns {:ok, date} or {:error, :invalid_date} (and that’s the only type of error):

https://github.com/elixir-lang/elixir/blob/master/lib/elixir/lib/calendar/date.ex#L219-L225

Ultimately I don’t think it’s the job of the function that being called to provide a return value that still has meaning 3-4 levels deep. But rather I would say it is the responsibility of the thing that calls that function to decide whether it is okay pass along the return value (:error), or not.

So your first example I would have written like this:

def some_input_handling_fun(binary) do
  with {:ok, decoded} <- Base.decode64(binary),
       {:ok, result} <- do_something_with_decoded(decoded) do
    {:ok, result}
  else
    # Be as specific as you want here
    :error -> {:error, :bad_encoding}
    # Assuming do_something_with_decoded already returns a tagged tuple      
    {:error, reason} -> {:error, reason}
  end
end

In your example there can be hidden things happening

If, for instance, the function do_something_with_decoded/1 also returns only :error because “there is only one error condition possible” then you might be surprised by the output. That is because it would also match on the else but you translated that to {:error, :bad_encoding} when the “only possible error” could have been something entirely different.

I know this is not broke “per se” but I still think that being obvious is better than assuming something. Being explicit here means we communicate better (and that is the reason why we’ve wrapped the error to something else on the with in the first place).

Ultimately I don’t think it’s the job of the function that being called to provide a return value that still has meaning 3-4 levels deep.

Totally agree with that! But you don’t even need nesting for this use case to happen. Think this:

def some_fun(arg) do
  with {:ok, result1} <- function1(arg),
       {:ok, result2} <- funciton2(result1) do
     IO.puts "happy path"
  else
    :error -> {:error, :which_function_returned_this?}
    _ -> raise "boom!"
  end
end

It is a subtle thing but it seems I am the only one bothered with it hehehe

Thanks for your research on the Elixir library! I am even more convinced this would be good

I think that because we have the freedom to return whatever we are allowing that to cloud our judgement a bit. In a statically typed language there wouldn’t even be an argument about this because the same type would be returned everywhere. There can most certainly be more information returned even though the actual issue is always the same. Consider this:

case Base.decode64(text) do
  {:ok, data} -> {:ok, data}
  {:error, {:bad_encoding, byte_offset}} -> {:error, "Encountered error decoding after the #{byte_offset}th byte\n#{text}"}
end

Both what went wrong and specifics about the error are accessible in native elixir terms and we can then choose to create a more friendly or debuggable error (or even take different action depending on the error metadata if we so choose).

Supposed we had a type to represent this as part of the standard library:

defmodule Result do
  @type success :: {:ok, any()}
  @type error :: {:error, {atom(), any()}}
  @type t :: success | error
end

This is so common in Erlang and Elixir. Why is there no type for it?

Now would the question really be what should we return? In order to be consistent it would become mandatory to return a Result.t() everywhere, even where not necessary, not only for consistency sake but for backwards compatibility. Now if we ever need to add another error condition, or some metadata about what went wrong, its easy to do so.

@victorolinasc

Regarding your last code example, was just reading this blog post (Better Control Flow Using The “with” Macro, see last example). And I thought it was a neat trick to differentiate between the return values / errors Still a bit lengthy, I know.

def some_fun(arg) do
  with {:function1, {:ok, result1}} <- {:function1, function1(arg)},
       {:function2, {:ok, result2}} <- {:function2, function2(result1)} do
     IO.puts "happy path"
  else
    {:function1, error} -> error # error returned by function1
    {:function2, error} -> error # error returned by function2
  end
end

I use this a lot in our code base and once you get used to the verbosity of it it’s actually really good. It’s especially useful for differentiating between a bunch of boolean conditions.