Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: inets: Add map-based request interface to httpc #9473

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jchristgit
Copy link
Contributor

@jchristgit jchristgit commented Feb 23, 2025

This is a very rough first draft towards potentially adding a new map-based request interface to httpc, the reason being that the existing request interface is a bit clunky to use & this might make it more straightforward. Most of the changes are adding types to many of the arguments to the requests.

This somewhat comes out of this thread on the Erlang Forums: https://erlangforums.com/t/httpc-httpd-improvements/2622

Example usage:

> {ok, #{status := Status} = Response} = httpc:request(#{method => get, uri => "https://erlang.org"}).

If there is interest, I'm happy to refine this further. Some open questions:

  • Should we perhaps create different request types for synchronous and asynchronous requests to allow clearly distinguishing these in the specs? Since they differ quite a bit, maybe we should add async as a top-level field in the request map.
  • Should we perhaps create different request types for requests without and requests with a body to allow clearly distinguishing them in the specs? Alternatively, we could incorporate the content type into the request body field, which might be simpler.

Copy link
Contributor

github-actions bot commented Feb 23, 2025

CT Test Results

No tests were run for this PR. This is either because the build failed, or the PR is based on a branch without GH actions tests configured.

Results for commit 12e9c89

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@tsloughter
Copy link
Contributor

Nice. I too started on a map interface. I did so first as a POC in a separate project to try things out before proposing them to OTP, https://github.com/tsloughter/reqerl

I never liked the 3-tuple return value of httpc so went with a single map. It is named reqerl here but figured if people liked some of the suggestions I'd propose it as either being httpc in OTP or httpc2 (hm, guess that'd be confusing if it didn't support http2...):

> reqerl:get("https://reqbin.org/json")
#{status => 200, headers => #{...}, body => [#{<<"x">> => 1,<<"y">> => 2},#{<<"x">> => 3,<<"y">> => 4}]}

I got stalled when I was debating back and forth about how I wanted to do middleware -- something I consider important to even a builtin http client because of its importance for observability. But at the same time I agree with people like @josevalim who who think it'd be great to have a new "gen_http" in OTP as a low level interface to HTTP.

I plan to pick back up reqerl again in the near future.

@tsloughter
Copy link
Contributor

tsloughter commented Feb 24, 2025

Oh, blergh, I just realized you had #{status := Status} = Response :), I'm so used to the current httpc I read that as #{status := Status}, Response.

My bad!

@jhogberg jhogberg added the team:PS Assigned to OTP team PS label Feb 24, 2025
@jchristgit
Copy link
Contributor Author

Nice. I too started on a map interface. I did so first as a POC in a separate project to try things out before proposing them to OTP, https://github.com/tsloughter/reqerl

Thank you for sharing! I'm also 50/50 with the thoughts of supporting e.g. middleware in OTP's HTTP client as opposed to keeping it low level. I really like the reqerl_interceptor idea to e.g. decode responses automatically!

My goal here is mainly just to make the main request / response interface a bit cleaner to use. That way I can kind of skip any deeper design discussions 😁. Maybe later, the inner guts could be changed so this would just call out to what gen_http would be, so it would continue working for the "sending simple requests" usecase as is, but other libraries could build on top of it.

Anyways, that's very far future thinking, right now my goal is just to improve the docs and "ergonomics" here a bit 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants