Do not raise errors on failure, but return an HTTPError struct

[edgurgel]

This would be a breaking change on the current API.

No more raising errors, just a struct. This change would make pattern matching easier and follow the way that most Erlang/Elixir APIs work.

Related: #26

[BlakeWilliams]

Would this use { :ok, response } and { :error, error } or just directly return the structs?

[edgurgel]

I was thinking on just structs as they can be easily pattern matched and have something like this:

case HTTPoison.get(url) do
  %HTTPoison.Response{status_code: 200, body: body} ->
    IO.puts body
  %HTTPoison.Response{status_code: 404} ->
    IO.puts "Not found :("
  %HTTPoison.Error(reason: reason) ->
     IO.puts "Error during request: #{inspect reason}"
end

[BlakeWilliams]

Seems reasonable. I definitely prefer this than having to catch an error, and it's more idiomatic.

[adamkittelson]

+1 for this, I implemented retries with exponential back-off in my simpledb library this week and my implementation would've been cleaner if I didn't have to handle both 500 range responses and rescue exceptions as well. (The simpledb API hits

httpoison/lib/httpoison/base.ex

Line 89 in 6d8b6aa

_ -> raise HTTPoison.HTTPError, message: "Failed to fetch the body"

every once in a while).

I did a Response or Error struct return value in my Zencoder library https://github.com/zencoder/zencoder-ex and an :ok or :error tuple situation for my simpledb library https://github.com/zencoder/zencoder-ex and having done it both ways I really haven't developed a preference for one or the other.

[josevalim]

I cam here to open this exact issue. You can go with get not raising and get! raising. I would also advocate for the simple {:ok, _} and {:error, _} just because it is definitely more concise than matching on the whole struct (and closer to Elixir's codebase too).

[edgurgel]

@josevalim, yeah I see your point. I was writing some examples of usage yesterday, and the good thing about the ok tuple is that you can easily use the response this way too:

{:ok, response} -> IO.puts response.body

To do it with just the struct, the code would look like this:

%HTTPoison.Response{} = response -> IO.puts response.body

I don't love the idea of bang methods but it's a nice way to people change and follow the old behaviour.

The other thing to think is the error tuple, should it be {error, reason} or {error, exception} or {error, error_struct}?. I don't like any of it.

• If we decide for the reason one, I would need to match the async response to have an error struct as well (https://github.com/edgurgel/httpoison/blob/master/lib/httpoison/base.ex#L37-L51).
• The exception will need to exist for bang methods.

The simples solution I see is use the fact that exception is a struct and use it everywhere I need to represent an error.

[josevalim]

It is ok to return {:error, struct}.

[edgurgel]

I just pushed the changes discussed here: 86471f9

Any feedback is welcome! If everything is fine I'll release 0.5.0 tomorrow!

[josevalim]

👍