Baffled by assert_error_sent behavior

Just solved an issue that (in my head) breaks “everything should be clear” motto, so I’d love to have some feedback or maybe a logical explanation :slight_smile:

Basically I don’t understand why this doesn’t work

test "renders page not found when id is nonexistent", %{conn: conn, admin: admin} do
  assert_error_sent 404, fn ->
      conn
      |> guardian_login(admin, :token, key: :admin)
      |> get(admin_product_path(conn, :show, -1))
  end
end

and throws

expected response status to be 404, but got 200 from:

 ** (Ecto.NoResultsError) expected at least one result but got none in query:

while this works fine:

test "renders page not found when id is nonexistent", %{conn: conn, admin: admin} do
  conn = guardian_login(conn, admin, :token, key: :admin)
  assert_error_sent 404, fn ->
      get(conn, admin_product_path(conn, :show, -1))
  end
end

The first one uses pipes and the second has a single call and closes on an already logged in conn, in the end they both should return the same thing, or am I missing something? Is there something about converting the exception to response that only works with a separate call?

Can you show the code that is called via admin_product_path(conn, :show, -1)?

it’s a route helper for resources "/products", ProductController in admin scope

Here is a way to “reproduce” it in a most generic way possible:

mix phoenix.new test
# adjust db config for dev and test
mix ecto.create
mix phoenix.gen.html Product products title:string text:text
# add resources "/products", ProductController to router
mix ecto.migrate
mix test

Tests will be green and will have one already:

test "renders page not found when id is nonexistent", %{conn: conn} do
  assert_error_sent 404, fn ->
    get conn, product_path(conn, :show, -1)
  end
end

add another test similar to the generated one but with a pipe:

test "fails to render page not found when id is nonexistent", %{conn: conn} do
  assert_error_sent 404, fn ->
    conn
    |> get(product_path(conn, :new))
    |> get(product_path(conn, :show, -1))
  end
end

bang!

expected response status to be 404, but got 200 from:

 ** (Ecto.NoResultsError) expected at least one result but got none in query:

What I’m curious in is that callback on ProductController, what conditions it returns 200 or 404, also I’m curious if guardian is setting things outside the returned conn

so was I so I kicked guardian :slight_smile: see instruction above, it happens without any external libs by just visiting another URL in closure using pipes

controller is generic bare bones generated by gen.html:

def show(conn, %{"id" => id}) do
  product = Repo.get!(Product, id)
  render(conn, "show.html", product: product)
end

Phoenix does the magic here:

Huh, I’m at a loss… o.O

Can you generate a minimal self-contained example and upload to github? That way we can play with it or deduce if a bug report needs to be given to elixir or the test suite. :slight_smile:

yes I can :slight_smile: Will do in a few, thanks for the help

1 Like

so, I’ve packed the whole thing in a repo https://github.com/bcat-eu/phoenix_error_test

Not sure if it’s that minimal, but it’s all bootstrapped code except for the extra test https://github.com/bcat-eu/phoenix_error_test/blob/master/test/controllers/product_controller_test.exs#L41-L47

You need to recycle the connection before sending it back through the endpoint. My hunch is the first call to get sends a respond with 200, and when your next request raises, it respects the already set status, even though it shouldn’t be 200 at that point. When making multiple requests with the same connection, you need to take care to recycle the connection so the session is written and fields are reprised for a “fresh” request. Try this:

test "fails to render page not found when id is nonexistent", %{conn: conn} do
  assert_error_sent 404, fn ->
    conn
    |> guardian_login(admin, :token, key: :admin)
    |> recycle()
    |> get(admin_product_path(conn, :show, -1))
  end
end
2 Likes

Good idea, would make sense to me, somehow doesn’t work though :slight_smile:

I’ve added this test to the repo: https://github.com/bcat-eu/phoenix_error_test/blob/master/test/controllers/product_controller_test.exs#L49-L56

1 Like

I don’t have an answer for you, but maybe this will help you or others get further along debugging.

I added some IO.inspects to the "same with recycle" pipeline like this:

test "same with recycle", %{conn: conn} do
  assert_error_sent 404, fn ->
    conn
    |> IO.inspect
    |> get(product_path(conn, :new))
    |> IO.inspect
    |> recycle()
    |> IO.inspect
    |> get(product_path(conn, :show, -1))
  end
end

The status field is initially nil, changes to 200 after the first get, and changes back to nil after recycle/0. The other test that’s passing also ends with a nil status. Something else must be different.

I then inspected the conn going into the "renders page not found ..." test’s get and found one potentially significant difference: its conn's :req_headers is an empty list, whereas in "same with recycle" it’s

[{"cookie","_test_key=SFMyNTY.g3QAAAABbQAAAAtfY3NyZl90b2tlbm0AAAAYWndxNjV3YTdwUkdUM0p6R2NaaEs5dz09.PnqDGiYVBG03CIWwfp-biCzUlrX-mn3yCwaM8IxgAtc"}]`

I don’t recall how to write plugs at the moment, but I suspect if you clear the :req_headers in your pipe test before the get it’ll pass. Couldn’t tell you why, though.

1 Like

Hmm, nevermind. |> Plug.Conn.delete_req_header("cookie") successfully deletes the header, but the pipe test still fails.

When I think about it, it actually makes sense that the recycle solution doesn’t work since this issue should have affected both cases, not only pipes in lamda.

Unless it’s something simple that we fail to notice, we should deal with something that works differently in pipeline inside of assert_error_sent’s closure (Plug’s exception handling?).

Pipes outside of it should not be relevant, I use them in gazillion tests this way:

test "renders form for editing chosen resource", %{conn: conn, product: product, admin: admin} do
  conn =
    conn
    |> guardian_login(admin, :token, key: :admin)
    |> get(admin_product_path(conn, :edit, product))

  assert html_response(conn, 200) =~ "Edit product"
end

or

test "tries to render form for editing a resource", %{conn: conn, product: product, user: user} do
  conn =
    conn
    |> guardian_login(user, :token)
    |> get(admin_product_path(conn, :edit, product))

  assert redirected_to(conn) == admin_login_path(conn, :new)
end

so there seem to be no general problem with piping requests and checking status in the end.

I had this exact same issue, and signed up for an account just so I could answer this. I resolved it by moving guardian_login OUTSIDE of the assert_error_sent function:

conn = guardian_login(conn, user)
assert_error_sent 404, fn ->
  get conn, manage_console_path(conn, :show, -1)
end

Digging deeper, this appears to be related to how assert_error_sent catches the exception raised by plug. Internally it uses a receive block to get the conn response, which of course pops the first message of the process inbox. When you run guardian_login, guess what? The 200 from that successful login is sitting in the inbox, hence the “but got 200 from” message. It never gets a chance to read the 404 response.

Hope this helps!

4 Likes

Yep, the receive point makes sense, guardian login or normal get call kills it. Thanks for answering!

1 Like