sneako/finch

Improve error messages on invalid URLs

Fermée

#186 ouverte le 24 avr. 2022

 (4 commentaires) (2 réactions) (0 personne assignée)Elixir (137 forks)batch import
good first issue

Métriques du dépôt

Stars
 (1 352 étoiles)
Métriques de merge PR
 (Merge moyen 9j 4h) (5 PRs mergées en 30 j)

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.

Guide contributeur