CODEWITHSUNDEEP
elixirphoenix-framework
Raphael Rafatpanah
Raphael Rafatpanah

Reputation: 20007

What is the idiomatic way to write the following logic in Elixir?

I have an endpoint check that returns a 200 response if the request contains a valid auth_token and 401 otherwise.

def check(conn, _) do
  if auth_token = get_session(conn, :auth_token) do
    send_resp(conn, 200, "")
  end

  send_resp(conn, 401, "")
end

This works fine in the case when auth_token is nil, however, I'm getting a weird error for the case where it is defined.

[error] Ranch listener AppWeb.Endpoint.HTTP had connection process started with :cowboy_clear:start_link/4 at #PID<0.6370.0> exit with reason: {:function_clause, [{:cowboy_http, :commands, [{:state, #PID<0.5887.0>, AppWeb.Endpoint.HTTP, #Port<0.1651>, :ranch_tcp, :undefined, %{env: %{dispatch: [{:_, [], [{:_, [], Phoenix.Endpoint.Cowboy2Handler, {AppWeb.Endpoint, []}}]}]}, stream_handlers: [Plug.Cowboy.Stream]}, "", %{}, {{127, 0, 0, 1}, 61063}, {{127, 0, 0, 1}, 4000}, :undefined, #Reference<0.3022468478.150208513.95764>, true, 2, {:ps_request_line, 0}, 65535, 1, :done, 1000, [{:stream, 1, {Plug.Cowboy.Stream, {:state, :undefined, AppWeb.Endpoint.HTTP, #PID<0.6371.0>, :undefined, :undefined, :undefined, :undefined, 0, :nofin, "", 0, :undefined, :normal}}, "GET", :"HTTP/1.1", :undefined, :undefined, 0, []}], [{:child, #PID<0.6371.0>, 1, 5000, :undefined}]}, 1, [{:response, "401 Unauthorized", %{"cache-control" => "max-age=0, private, must-revalidate", "content-length" => "0", "date" => "Sat, 29 Aug 2020 17:32:18 GMT", "server" => "Cowboy", "x-request-id" => "Fi_Nf1nWhRxOi1UAAQPB"}, ""}]], [file: '/Users/raph/Code/App/api/deps/cowboy/src/cowboy_http.erl', line: 954]}, {:cowboy_http, :loop, 1, [file: '/Users/raph/Code/App/api/deps/cowboy/src/cowboy_http.erl', line: 254]}, {:proc_lib, :init_p_do_apply, 3, [file: 'proc_lib.erl', line: 226]}]}

Note: the response is sent successfully in all cases, and the above error appears afterwards.

What does the error mean and how can I avoid it?

Update

I just noticed that it appears that the above code is attempting to send two responses:

[info] Sent 200 in 41ms
[info] Sent 401 in 41ms

Given that my real code is closer to:

def check(conn, _) do
  if auth_token = get_session(conn, :auth_token) do
    if user = Accounts.get_user_by_session_token(auth_token) do
      send_resp(conn, 200, Map.get(user, :email))
    end
  end

  send_resp(conn, 401, "")
end

What would be the idiomatic way to write the above logic such that send_resp is only called once?

The below implementation works without errors, but feels like it can be refactored in a more idiomatic way.

def check(conn, _) do
  if auth_token = get_session(conn, :auth_token) do
    if user = Accounts.get_user_by_session_token(auth_token) do
      send_resp(conn, 200, Map.get(user, :email))
    else
      send_resp(conn, 401, "")
    end
  else
    send_resp(conn, 401, "")
  end
end

Upvotes: 0

Views: 286

Answers (1)

potibas
potibas

Reputation: 627

Assuming there's a User struct, I think this is a little bit more idiomatic:

def check(conn, _) do
  with token when is_binary(token) <- get_session(conn, :auth_token),
       %User{} = user <- Accounts.get_user_by_session_token(token) do
    send_resp(conn, 200, user.email)
  else
    _ -> send_resp(conn, 401, "")
  end
end

Upvotes: 5

Related Questions