Bandit - a pure Elixir HTTP server for Plug & WebSock applications

[lud]

Hello,

We have that test in out suite to ensure our http client (based on req) will return transport errors correctly:

    base =
      mock_http_server(fn "/test", conn ->
        send(conn.private.parent, :called)
        exit(:normal)
      end)

    assert {:error, %Req.TransportError{reason: :closed}} = send_post(retry: :transient, retry_delay: 1)

    for _ <- 1..4, do: assert_receive(:called)

I could give some details about the implementation but basically we used exit(:normal) to force the connection to close. The function above is more or less the plug given to Bandit.

With the last update of Bandit, this returns a 500 error instead. Is there a simple way, callable from this test, to force the connection to close?

Thank you.

Edit: I did Process.exit(conn.owner, :kill) and it seems to work but I am not sure it’s very clean

[wojtekmach]

Btw unless you need to hit a real http server like Bandit, you can simulate getting that particular error and asserting how Req handles it by using Req.Test.transport_error/2:

iex> Mix.install [:req, :plug]
iex> Req.get(retry: false, plug: fn conn -> Req.Test.transport_error(conn, :closed) end)
{:error, %Req.TransportError{reason: :closed}}

[lud]

Thank you

I know this is controversial but we are doing real HTTP in the tests, as there are multiple layers and ways to mock each of them, at some point we decided it was simpler to test the real thing. This is for a custom HTTP client so it must do HTTP properly and this allows to not depend on a particular library such as Req.

That being said I doubt we will move from req anytime soon and our mock_http_server is also an abstraction that could leverage the test features of Req, but this has to be setup properly. It’s on my list though.

Today I just need a quick fix.

edit for @Eiji

I found a lot of people who cleary reject the idea of performing an actual http call during the tests, more so if you also start the test http server from the test (which we do, we never actually call a 3rd party server), especially when alternative solutions are possible like with Req. This is what I mean by controversial, I don’t personally think it’s bad.

[Eiji]

@lud

I know this is controversial but we are doing real HTTP in the tests

Just to clarify …

It’s not controversial at all. Simply good thing done improperly can cause tests to fail. There are cases when you have to make sure that your app correctly handles 3rd-party server response and in such case you simply cache everything and put into git history. This way you have fast and not failing tests and “real” (but cached) replies.

When the server updates it’s API you simply remove cached files and run tests again once and commit changed cache. This way tests do not fail for example if said server is overloaded and therefore responding too late.

[wojtekmach]

I know this is controversial but we are doing real HTTP in the tests

To me, not controversial at all. I use the following very very often:

{:ok, pid} =
  Bandit.start_link(
    plug: fn conn, _ -> Plug.Conn.send_resp(conn, 200, "ok") end,
    port: 0
  )
{:ok, {_ip, port}} = ThousandIsland.listener_info(pid)
url = "http://localhost:#{port}"

This is the 80% Bypass I most commonly need. In fact I’d like to be able to use Req.Test mocks backed by Bandit somehow, if that makes sense, but I couldn’t find the right API. Apologies for hijacking the topic, I’ll bow out now but if it piques anyones interest I’m happy to talk through it further elsewhere.

[lud]

Yes this is the core of what we have currently!

but I couldn’t find the right API

What is missing beyond what you showed there ?

[mtrudel]

@lud

With the last update of Bandit, this returns a 500 error instead.

It’s returning an error because you’re not using the Plug API correctly; the contract is clear that your Plug MUST return a Plug.Conn struct with a set response in order to be valid, and by exiting out of the Plug, you’re not doing that. This is coming to light for you in the 1.6.x release train since we’ve been focusing on tightening up error handling at this exact boundary (previously, we were rather loose on this which is why you weren’t seeing any errors, even if you were doing something unsupported). But what you’re seeing is 100% expected behaviour. There’s an error in your plug implementation, which is obviously server-side, and so is a 5xx series error.

In terms of what you’re attempting to do, there’s nothing explicit in the Plug API that allows you to force the connection closed. Strictly speaking, the Plug API is 1:1 with requests inside a connection (such as keepalive requests within a long-lived HTTP connection, or streams within an HTTP/2 connection). The ‘correct’ way to do this (at least in HTTP/1) is found in RFC9112§9.3, and is for the server to send a connection: close header in its response, to which the client should then close the connection on its side.

[lud]

I know, I know, it’s not a valid plug. As I described I am not testing a plug implementation here, but a HTTP client.

I’m looking for a way to make the server crash while in the middle of serving the request, just by using the data from the conn. Killing the owner seems to work fine. So I am obviously expecting that code to return an error.

The behaviour I’m trying to mimic (remote server crashing) is not sending connection: close headers, so i’m not sure it would be a good test here, but I may try.

Thank you.

[mtrudel]

The behaviour I’m trying to mimic (remote server crashing) is not sending connection: close headers, so i’m not sure it would be a good test here, but I may try.

Ah! That’s an oversight on Bandit’s part (RFC9112§9.6 says we should be sending that header before closing the connection if possible, and it’s easily possible to do so).

Just added this behaviour to Bandit, if you want to test against main!

[lud]

Hello again @mtrudel , I have a question regarding Plug.Conn.send_chunked and Plug.Conn.chunk with Bandit.

Is there any guarantee that when the Plug.Conn.chunk(conn, foo) call returns, the HTTP/TCP layer confirms that the client did receive the data? Or can the connection close after the function returns but before the client has fetched the whole chunk?

This is for SSE streams, I want to know if when I just sent a chunk, I can consider it received by the client.

Thank you.