sneako/finch

Improve error messages on invalid URLs

Chiusa

#186 aperta il 24 apr 2022

 (4 commenti) (2 reazioni) (0 assegnatari)Elixir (137 fork)batch import
good first issue

Metriche repository

Star
 (1352 stelle)
Metriche merge PR
 (Merge medio 9g 4h) (5 PR mergiate in 30 g)

Descrizione

In https://github.com/wojtekmach/req/issues/81, we noticed Finch exits on an invalid URL:

Mix.install([
  {:finch, "~> 0.11.0"}
])

{:ok, _} = Finch.start_link(name: MyFinch)
Finch.build(:get, "http://") |> Finch.request(MyFinch) |> IO.inspect()
** (exit) :badarg
    (finch 0.11.0) lib/finch/http1/pool.ex:62: Finch.HTTP1.Pool.request/5
    (finch 0.11.0) lib/finch.ex:294: Finch.request/3
    bug.exs:6: (file)

I did a quick survey of a couple of similar edge cases and here are the result:

defmodule Main do
  def main do
    {:ok, _} = Finch.start_link(name: MyFinch)

    get("bad")
    get("http:/")
    get("http://")
    get("https://")
  end

  def get(url) do
    Finch.build(:get, url)
    |> Finch.request(MyFinch)
    |> IO.inspect(label: inspect(url))
  catch
    kind, reason ->
      {kind, reason}
      |> IO.inspect(label: inspect(url))
  end
end
"bad": {:error, %ArgumentError{message: "scheme is required for url: bad"}}
"http:/": {:error,
 %ArgumentError{
   message: "the :hostname option is required when address is not a binary"
 }}
"http://": {:exit, :badarg}
"https://": {:error,
 %Mint.TransportError{
   reason: {:options,
    {:socket_options,
     [
       nodelay: true,
       keepalive: true,
       packet_size: 0,
       packet: 0,
       header: 0,
       active: false,
       mode: :binary
     ]}}
 }}

(The "https://" one is especially odd!)

All these errors are slightly different, perhaps it would be worthwhile to make them a bit more uniform.

Guida contributor