Follow redirects improvement

[amatalai]

Previously redirect(env, next, 0) raised error immediately when if fact I would expect it to check this last location and raise only if it return another redirect

[teamon]

Hmm, isn't this the same as changing left <= 0 to left <0, so that left == 0 case goes into the second redirect(..) clause?

[amatalai]

Technically yes, but for me it seems to be more clear to have 0 remaining redirects than -1 remaining redirects.

[amatalai]

Added note about httpc, because this middleware won't work for httpc default config

defmodule RedirectTest.Hackney do
  use Tesla

  adapter :hackney

  plug Tesla.Middleware.FollowRedirects, max_redirects: 0
end

defmodule RedirectTest.Ibrowse do
  use Tesla

  adapter :ibrowse

  plug Tesla.Middleware.FollowRedirects, max_redirects: 0
end

defmodule RedirectTest.Httpc do
  use Tesla

  adapter :httpc

  plug Tesla.Middleware.FollowRedirects, max_redirects: 0
end

defmodule RedirectTest.FixedHttpc do
  use Tesla

  adapter :httpc, autoredirect: false

  plug Tesla.Middleware.FollowRedirects, max_redirects: 0
end

iex(1)> RedirectTest.Hackney.get("http://yahoo.pl")
** (Tesla.Error) too many redirects

iex(2)> RedirectTest.Ibrowse.get("http://yahoo.pl")
** (Tesla.Error) too many redirects

iex(3)> RedirectTest.Httpc.get("http://yahoo.pl")
%Tesla.Env{status: 200}

iex(4)> RedirectTest.FixedHttpc.get("http://yahoo.pl")
** (Tesla.Error) too many redirects

[aphillipo]

Shouldn't the default for httpc be the same as all the other http clients, i.e. autoredirect: false and then if people want this functionality they can enable Tesla.Middleware.FollowRedirects?

Seems like strange behaviour that changing the http client changes this and exactly the sort of thing something like Tesla should be hiding from the user?

[teamon]

I think you're right @aphillipo, we should make the httpc adapter have default opts with autoredirect: false