req icon indicating copy to clipboard operation
req copied to clipboard

transport_error/2 raises when testing, instead of erroring

Open lucavenir opened this issue 1 year ago • 3 comments

I've read the documentation about transport_error/2 and maybe I'm understanding it wrong.

Following the docs as closely as I can, here's my (unexpectedly) failing test:

test "returns the error when the request fails" do
  Req.Test.stub(MyApp.MyModule, fn conn ->
    Req.Test.transport_error(conn, :some_error)
  end)

  assert {:error, :some_error} = my_get_function()
end

The test fails not because the match fails, but because my_get_function/0 raises:

(ArgumentError) unexpected Req.TransportError reason: :some_error

But I'm not using Req.get!/2. I'm using Req.get/2 just like the docs suggest. Here's my_get_function/0:

@doc false
def my_get_function() do
  :my_app
  |> Application.get_env(MyApp.MyModule, [])
  |> then(&Req.get(@url, &1)) # `get`, and not `get!`
  |> case do
    {:ok, %Req.Response{status: 200, body: body, headers: headers}} -> {:ok, body, headers}
    {:ok, response} -> {:error, response}
    error -> error
  end
end

Is there something I'm missing? Is this intended behavior?

lucavenir avatar Oct 12 '24 21:10 lucavenir

Thank you for the report, we definitely need to improve the error message.

The idea was you can only call transport_error/1 with errors that can happen in production so your test is more realistic. So transport_error(conn, :timeout) works but transport_error(conn, :timeout_typo) does not.

wojtekmach avatar Oct 12 '24 21:10 wojtekmach

Oh. This makes sense. And I like the choice! Thanks for the (very quick and appreciated!) response!

lucavenir avatar Oct 12 '24 22:10 lucavenir

It would also be nice to know which atoms we're allowed to inject there.

:timeout isn't always that handy: I might want to retry in production, but I'd like to avoid that specific error case when testing (else, I must wait several seconds).

lucavenir avatar Oct 12 '24 22:10 lucavenir