Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More conventional output #64

Closed
gworthe opened this issue Feb 13, 2017 · 3 comments
Closed

More conventional output #64

gworthe opened this issue Feb 13, 2017 · 3 comments

Comments

@gworthe
Copy link

gworthe commented Feb 13, 2017

Hello, thanks for this great library.

It seems to me like there's some convention in elixir to make normal function return output as tuple {:ok, value} or {:error, error} and for function with ! to return only value or raise error.

Would it be difficult to make Tesla work like this?

@teamon
Copy link
Member

teamon commented Feb 14, 2017

Hi,

Right now all get/post/put/etc. functions return either Tesla.Env struct or raise an error.

The issue with returning :ok/:error tuples is both very hard and trivial.
It's hard because there are many ways the library could behave and many different expectations from its users - there is no simple one-size-fits-all solution. And it's trivial in terms of implementation.
Please take a look at the two following middlewares:

defmodule Tesla.Middleware.TupleForGoodResponses do
  @moduledoc """
  Return :ok/:error tuples for "good" responses, i.e. when status code is in the range (200 - 399)
  """
  def call(env, next, _opts) do
    case Tesla.run(env, next) do
      %{status: status} = env when status < 400
        -> {:ok, env}
      env
        -> {:error, env}
    end
  end
end
defmodule Tesla.Middleware.TupleForSuccessfulTransaction do
  @moduledoc """
  Return :ok/:error tuples for successful HTTP transations, i.e. when the request is completed
  (no network errors etc) - but it can still be an application-level error (i.e. 404 or 500)
  """
  def call(env, next, _opts) do
    try do
      {:ok, Tesla.run(env, next)}
    rescue
      %Tesla.Error{} = er ->
        {:error, er}
    end
  end
end

As you can see, implementing both scenarios is pretty easy (and we should probably include some kind of these middlewares but definitely with better names).

And since the topic of return values has been brought up here, @michalmuskala got a minute? :)

@michalmuskala
Copy link

Sorry for the late answer.

An ideal situation would be to have both a raising and a tuple-return version of each function. But, if there are a lot of functions, that's not really sustainable. I really like how the new ex_aws is structured - exactly to combat that issue.

Let's look at a fetch_repos function for an example GitHub API client. In that architecture, instead of having the function perform the actual request, it returns some data encoding the request without executing it. This data, can be later passed to a request/1 or a request! function that actually perform the request. This allows having both versions of every function (the raising and non-raising one) with minimal additional code - the error handling is contained and separate from the actual logic.

@teamon
Copy link
Member

teamon commented Apr 5, 2017

Thank for all the feedback!

Creating two version of each function (e.g. get and get!) would require making a decision under what conditions the ! should raise and this can vary in different situations. The same can be achieved using trivial middleware (see examples above).

I'm closing this issue for now. It can be reopened if there is any more input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants