Error handling with "with"

Sort of a newbie code review question, but I’m curious if there’s a way to work around this. I’m still trying to wrap my head around good practices for error tuples. The precise code is in a Phoenix app but it could well exist outside of it. I have three functions in three modules:

# TaskRegistry
  def get_status(id) do
    with [{id, status}] <- :ets.lookup(:task_registry, id)
    do
      {:ok, {id, status}}
    else
      [] -> {:error, :file_not_found}
      _ -> {:error, :unknown}
    end
  end

  # TaskServer
  def handle_call({:status, id}, _, state) do
    with {:ok, {_, status}} <- TaskRegistry.get_status(id)
    do
      {:reply, {:ok, status}, state}
    else
      error -> {:reply, error, state}
    end
  end

  # TaskController
  def status(%Plug.Conn{path_params: %{"id" => id}} = conn, _params) do
    with {:ok, status} <- TaskServer.status(id)
    do
      json(conn, %{status: status})
    else
      {:error, :file_not_found} -> conn |> put_status(404) |> json(%{status: "not found"})
    end
  end

This doesn’t really seem right to me, that each of these has a with clause. But without it on each failed request the TaskServer crashes (and Phoenix returns HTTP 500, which is undesirable but not really relevant to the question). It doesn’t seem right that the GenServer should crash just because someone put in something wrong. I’m not sure how to refactor this well, though.

All these functions would be much better off with case instead of with. Avoid else in with is a pretty good advice.

6 Likes

It’s hard to say exactly what causes this crash without the code, but I’d guess it’s a MatchError when using code like the above but without with, turning <- to =

For an expression with one statement in the “happy path” of a with, case will be more readable:

  def get_status(id) do
    case :ets.lookup(:task_registry, id) do
      [{id, status}] -> {:ok, {id, status}}
      [] -> {:error, :file_not_found}
      _ -> {:error, :unknown}
    end
  end
6 Likes