Return Finch.Error instead of propagating Mint.TransportError

[sato11]

While I was giving a try to solve #186 I found this unit of changes might be independently testable and reviewable. So let me submit this first. What do you think?

---

Replaced with with case to roughly catch error.
This can at least stop propagating Mint.TransportError.

For other sorts of errors, it just returns themselves.
Since Finch.Error wants the reason given to be atom,
I've refrained from trying to pack everything into Finch.Error.

before:

iex(2)> Finch.build(:get, "https://") |> Finch.request(MyFinch)
{:error,
 %Mint.TransportError{
   reason: {:options,
    {:socket_options,
     [
       nodelay: true,
       keepalive: true,
       packet_size: 0,
       packet: 0,
       header: 0,
       active: false,
       mode: :binary
     ]}}
 }}

after:

iex(29)> Finch.build(:get, "https://") |> Finch.request(MyFinch)
{:error,
 %Finch.Error{
   reason: {:options,
    {:socket_options,
     [
       nodelay: true,
       keepalive: true,
       packet_size: 0,
       packet: 0,
       header: 0,
       active: false,
       mode: :binary
     ]}}
 }}

[wojtekmach]

This can at least stop propagating Mint.TransportError.

I believe there's nothing wrong with Finch returning exceptions other than Finch.Error. The only promise Finch makes is that on errors it returns {:error, Exception.t}. That is, any exception struct, not any particular one.

I think it is desirable to keep Mint.TransportError and others as they might have their own preferred way of displaying errors (the Exception.message/1 callback). Here's an example:

iex> Finch.start_link(name: MyFinch); r = Finch.build(:get, "http://bad") ; {:error, e} = Finch.request(r, MyFinch); raise e
** (Mint.TransportError) non-existing domain

on this branch we would have gotten:

** (FinchError) :nxdomain

[wojtekmach]

This is not really related to this particular change per se,

iex(29)> Finch.build(:get, "https://") |> Finch.request(MyFinch)
{:error,
 %Finch.Error{
   reason: {:options,
    {:socket_options,
     [
       nodelay: true,
       keepalive: true,
       packet_size: 0,
       packet: 0,
       header: 0,
       active: false,
       mode: :binary
     ]}}
 }}

this pretty unfortunate example because this is not at all when we want to have returned in this particular case. I think what is happening is due to invalid URL, Mint is somehow doing an incorrect :ssl.connect call. I'd argue that by wrapping the error in Finch.Error struct, it makes it a bit harder to debug it.

[sato11]

Thanks for the comment, @wojtekmach!

Well, sorry that I've misunderstood the premises in the first place, as well as I've misused the term propagation. 😭 I believed it was nicer but after reading your feedback I'm convinced it's fine to let it be. Regarding Exception.message/1, I was not quite aware of how to take advantage of it but now I believe I am.

Thanks again for the lesson, I much appreciate it 🙂