Sensible way to abstract away request client library in my application?

I’ve started to test my application and immediately run into the mock debate. Just done reading:

The above suggests using explicit contracts and config variables to manage environment-specific dependencies initially, and then goes on to suggest the simpler method of injecting dependency directly.

However, my current method is a little different. I am using explicit contracts and delegation such as:

config :myapp, request_client: MyApp.Tesla.Request

defmodule MyApp.Request do
  @client Application.get_env(:myapp, :request_client)

  defdelegate make, to: @client

  @callback make(etc) :: etc
end

defmodule MyApp.Tesla.Request do
  @behaviour MyApp.Request

  use Tesla
  plug Tesla.Middleware.Retry, delay: 1000, max_retries:: 3

  def make do
    # perform request, return response
  end
end

This gives me a common interface and call MyApp.Request.make, while also giving me the ability to switch out the client with configuration, which I will use during testing.

Does this look sensible? Is there something inherently wrong with the above approach or, in your opinion, an improvement?

The second question is about testing MyApp.Tesla.Request – perhaps I can switch out the client I use in testing, but surely I still need to test that my Tesla request is configured/running properly.

This looks okay. You can also use mox that basically does the same thing + has verifications/assertions you can use in tests.

Was just reading through mox documentation then. I’ll see how far I get in my tests.

Maybe examples here will make it a bit more clear: http://slides.com/hubertlepicki/mox

1 Like

Not entirely sure how mox is working, but I have this:

defmodule Example do
  @callback function(integer()) :: integer()
  @callback another(integer()) :: integer()
end

Mox.defmock(ExampleMock, for: Example)

test "adheres to contract" do
  ExampleMock
  |> expect(:function, fn x -> x end)

  assert ExampleMock.function("string") == "string"
end

…passes. But my type specifies integer and I have another function defined in my @callback which hasn’t been defined in the ExampleMock (though those guarantees don’t appear to exist anyway when using @behaviour in derived modules). Are those types (integer(), etc) just for documentation purposes? They don’t have any effect beyond that?

Seems a shame to have all of this ceremony without the guarantees. Seem more like guidelines than explicit contracts?

I think they can be checked by dialyzer.

Ahh I see. I’ve not delved into that yet.

Yep, these are not checked by Elixir compiler, so serve as documentation only unless you use dialyzer.

Fair enough. I’d like to use dialyzer but that’s a whole can of worms I’m unwilling to open. Included it in a project and it’s coming up with all kinds of non-issues/noise, even with warnings disabled. Not enough time in the day to be fighting with another tool :stuck_out_tongue:

@spec is good enough for me.

It is not that complicate to use dialyzer with dialyxir… just add

{:dialyxir, "~> 0.5.1", only: [:dev], runtime: false}

to your deps.

Then run (it take some time at first, but then it’s ok)

$ mix deps.get
$ mix dialyzer

I had to ignore some warnings coming from some Erlang sources included in my project. To configure this is also simple, just add to mix.exs, in the project section.

      dialyzer: [
        ignore_warnings: "dialyzer.ignore-warnings"
      ],

and create a file dialyzer.ignore-warnings containing the rules You want to ignore.

Yep, that’s what I tried.

I just can’t be doing with cryptic stuff like the following about code which works:

`

The pattern {'ok', Vresponse@1} can never match the type #{'__client__':=fun(), '__module__':=atom(), '__struct__':='Elixir.Tesla.Env', 'body':=_, 'headers':=#{binary()=>binary()}, 'method':='delete' | 'get' | 'head' | 'options' | 'patch' | 'post' | 'put' | 'trace', 'opts':=[any()], 'query':=[{atom() | binary(),binary() | [{atom() | binary(),binary() | [{_,_}]}]}], 'status':=integer(), 'url':=binary()}
`

Well it can match the type because it does. I would love to have this checking if it worked.

Also:

:0: Unknown function 'Elixir.GenStage':start_link/2 :0: Unknown function 'Elixir.Request':make/1 lib/gen_stage.ex:1: Callback info about the 'Elixir.GenStage' behaviour is not available

…and in my templates (using slime):

/templates/layout/app.html.slim:1: Guard test _@7::bitstring() =:= 'false' can never succeed

It’s cryptic, but dialyxir is always right.

How is that right, though? When it is talking about a match which happens successfully, which is being asserted in my tests? I don’t understand how it can never match that type when the type is being matched in Elixir.

The line is very simply:

case result do
  {:ok, response} -> {:ok, Response.adapt(response)}
end

Unless I have to define this in another way?

But how is result beeing bound?

That’s:

result = case method do
  "GET" -> get(url)
end

Where get is a function from Tesla.get.

Together:

result = case method do
  "GET" -> get(url, headers)
  "POST" -> post(url, headers)
end

case result do
  {:ok, response} -> {:ok, Response.adapt(response)}
  {:error, error} -> {:error, ResponseError.adapt(error)}
  _               -> {:error, ResponseError.adapt(%{reason: :unknown})}
end

So dialyzer is totally right.

Tesla.get/* and Tesla.post/* all are returning Tesla.Env.t according to their @spec. A Tesla.Env.t will never match a {:ok, _} as it is a struct, not a tuple.

If though Tesla.get/* does return a tuple instead, you need to file a bugreport at tesla to fix their specs (or implementation).

1 Like

I don’t understand. That code works – the result of Tesla.get is a tuple with {:ok, response} or {:error, struct (with reason)}?

In that case - if this is a bug in their spec/impl, apart from telling Tesla about it, what would be the proper way to handle this? Patch the spec myself? Wait for it to be fixed? Tell dialyzer to ignore?

1 Like

Its easiy.

You are using Tesla.get/1, which is @speced to return Tesla.Env.t, but implemented to return {:ok, any()} | {:error, any()} (or something similar).

You are running dialyzer in the context of your application, so dialyzer will trust in the specs of external libraries without validating them.

If you were running dialyzer in the context of Tesla it will probably complain that the functions do not fullfill their contract (do not remember the exact message).

1 Like

The proper way is to file an issue, perhaps even prepare and do a PR and then ignore the warning until the next release of tesla.

After the next release of tesla do whatever is appropriate for the way they used to fix.

1 Like

Thanks for the breakdown. I understand why it’s failing now.

1 Like