edgurgel / edgurgel/httpoison

Overly loose typespecs

Open
#467 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Elixir
Stars
2.3k
Forks
349
PR merge metrics
No merged PRs in 30d

Description

I was a bit surprised to hit a bug in our code - we accidentally put a map instead of a keyword list in a Request struct, and that blew up in our staging environment:

```
2022-11-17T21:57:36.282000+00:00 ip-192-168-0-108/fb9f6b2 (elixir 1.14.1) lib/keyword.ex:1078: Keyword.has_key?(%{"follow_redirect" => true, "max_body_length" => 10240, "max_redirect" => 3, "recv_timeout" => 5000, "timeout" => 5000}, :proxy)
2022-11-17T21:57:36.282000+00:00 ip-192-168-0-108/fb9f6b2 (httpoison 1.8.2) lib/httpoison/base.ex:763: HTTPoison.Base.build_hackney_proxy_options/1
```

The reason I was surprised is that I assumed that a widely used library like HTTPoison by now would have correct typespecs everywhere, so seeing https://github.com/edgurgel/httpoison/commit/bd14138f2f88a8f5b7f9732776fc8157ab6802b3 was, well, not what I expected.

Can we batten down the hatches? At the moment, you can feed anything you want to HTTPoison and Dialyzer will not mind; this is the sort of mistake that we have the tools to catch quickly and unless I'm missing something (there's not a lot of detail on that commit) there's really no reason to make everything just `any()`. I mean, it's probably better to just remove the typespecs at this point altogether...

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reviewing the broad typespec change in commit bd14138f2f88a8f5b7f9732776fc8157ab6802b3 and the failure path in lib/httpoison/base.ex:763. Identify the affected HTTPoison inputs and tighten their types so Dialyzer can reject a map where a keyword list is required; done means the documented interfaces are precise and the reported mistake is caught.

Written by the indexing model from the issue text.

Assessment

Tech stack
elixir
Domain
backend, networking
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.