sneako/finch

Improve error messages on invalid URLs

Closed

#186 opened on Apr 24, 2022

 (4 comments) (2 reactions) (0 assignees)Elixir (137 forks)batch import
good first issue

Repository metrics

Stars
 (1,352 stars)
PR merge metrics
 (Avg merge 9d 4h) (5 merged PRs in 30d)

Description

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.

Contributor guide