Add simple http client #1123

bettio · 2024-03-31T13:56:08Z

Add simple http client inspired to mint.

This kind of API works well with memory constrained devices, since it allows to handle chunks and connection events one by one.

TODO after this PR:

Rework default headers handling, such as Host and Content-Lenght
Handle chuncked body with delimiters

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

libs/eavmlib/src/ahttp_client.erl

UncleGrumpy

I am not sure about the format of your specs.

UncleGrumpy · 2024-05-08T14:54:29Z

That comment was about the function specs. Although it does apply to types as well. I did not get the first line highlighted and included in my comment so the context was not very clear.

UncleGrumpy

This looks good to me.

arpunk

Thank you for this 🎉

pguyot

We are lacking tests beyond the example. When trying to test this code beyond a simple redirect on atomvm.net (e.g. to get HTTP errors), I ended up realizing our ssl implementation is not compatible with TLS 1.3 by lack of proper verification options.

libs/eavmlib/src/ahttp_client.erl

pguyot · 2024-05-14T04:00:05Z

libs/eavmlib/src/ahttp_client.erl

+
+-define(DEFAULT_WANTED_HEADERS, [<<"Content-Length">>]).
+
+-record(http_client, {host, socket, parser, ref, parse_headers}).


Could we please have types for record fields?

libs/eavmlib/src/ahttp_client.erl

pguyot · 2024-05-14T04:05:56Z

libs/eavmlib/src/ahttp_client.erl

+%% be provided.
+%%
+%% `stream' option can be used with `stream_request_body/3' in order to upload a bigger
+%% binary in streaming mode. This option should be combined with `content-length' header.


Couldn't we stream without knowing the length? If it works we could just change the documentation with s/should be combined/may be combined/

Yes, but it requires chunked uploads with delimiters, that is beyond the scope of this PR. I would work on this with a further set of improvements.

libs/eavmlib/src/ahttp_client.erl

pguyot · 2024-05-14T05:30:17Z

libs/eavmlib/src/ahttp_client.erl

+split_header([Name | Tail], HeaderLine) ->
+ NameLen = byte_size(Name),
+ case HeaderLine of
+ <<Name:NameLen/binary, $:, Value/binary>> -> {ok, Name, Value};


We have a case issue here.

pguyot · 2024-05-14T05:35:54Z

libs/eavmlib/src/ahttp_client.erl

+%%
+%% The updated returned connection should be used in place of the previous one as a
+%% parameter for the next call.
+%% The returned reference can be used later for matching responses when using `stream/2'.


I do not understand this usage, and it's not apparent in example.

This is the only place that I find the documentation a bit unclear too. Perhaps inserting a short example here would help clear up any confusion.

it can be used when handling multiple http requests from the same process, each request will have it's own reference.

I tried to clarify it

libs/eavmlib/src/ahttp_client.erl

UncleGrumpy · 2024-05-14T19:45:03Z

libs/eavmlib/src/ahttp_client.erl

+%% @param Method a http method such as "GET", "POST", "PUT", etc...
+%% @param Path the path to the http resource, such as "/"
+%% @param Headers a list of headers
+%% @param Body the body that is sent to the server, may be undefined or nil when there is no body


Also whether it is nil or none the exact term should probably be code syntax highlighted with an Erlang (`') code block in the @param definition, so that it is understood to be a atom, and not "empty" or zero... Unless you go with the [] option.

UncleGrumpy · 2024-05-16T17:19:57Z

libs/eavmlib/src/ahttp_client.erl

+%% The first returned response to a new request is a status `{status, Ref, 200}' for
+%% a successful response. After that headers and data may follow.
+%%
+%% Since the response might span multiple socket messages, `stream/3' may be called


Should this be stream/2, or stream_request_body/3? I don't see a stream/3.

libs/eavmlib/src/ahttp_client.erl

Add simple http client that can be used in different situations, such as when performing updates, inspired to Mint Elixir http client. Signed-off-by: Davide Bettio <[email protected]>

Disable it since `ahttp_client` cannot be run right now. Signed-off-by: Davide Bettio <[email protected]>

bettio changed the base branch from main to release-0.6 March 31, 2024 13:56

bettio force-pushed the http-client branch from 2bf9d88 to 8b53fdb Compare April 2, 2024 21:44

bettio force-pushed the http-client branch 3 times, most recently from 0bb3026 to 6f6bdf3 Compare May 7, 2024 22:24

UncleGrumpy reviewed May 8, 2024

View reviewed changes

libs/eavmlib/src/ahttp_client.erl Outdated Show resolved Hide resolved

UncleGrumpy reviewed May 8, 2024

View reviewed changes

bettio force-pushed the http-client branch 2 times, most recently from 1e33d44 to 4470de7 Compare May 8, 2024 13:05

bettio force-pushed the http-client branch 6 times, most recently from cb06fc9 to 1f23730 Compare May 13, 2024 21:15

bettio marked this pull request as ready for review May 13, 2024 21:16

bettio force-pushed the http-client branch from 1f23730 to c171f67 Compare May 13, 2024 21:18

UncleGrumpy approved these changes May 13, 2024

View reviewed changes

arpunk approved these changes May 13, 2024

View reviewed changes

pguyot reviewed May 14, 2024

View reviewed changes

UncleGrumpy reviewed May 14, 2024

View reviewed changes

bettio force-pushed the http-client branch from c171f67 to c740f10 Compare May 16, 2024 12:45

UncleGrumpy reviewed May 16, 2024

View reviewed changes

bettio force-pushed the http-client branch from c740f10 to 1c8feaf Compare May 16, 2024 22:33

petermm reviewed May 18, 2024

View reviewed changes

libs/eavmlib/src/ahttp_client.erl Show resolved Hide resolved

bettio force-pushed the http-client branch 4 times, most recently from af0e539 to 22ec303 Compare May 22, 2024 22:57

Add simple http client

4fe36c1

Add simple http client that can be used in different situations, such as when performing updates, inspired to Mint Elixir http client. Signed-off-by: Davide Bettio <[email protected]>

bettio force-pushed the http-client branch from 9959815 to e6f4c3b Compare June 5, 2024 20:34

WASM: temporary disable test_eavmlib

080a5d6

Disable it since `ahttp_client` cannot be run right now. Signed-off-by: Davide Bettio <[email protected]>

bettio force-pushed the http-client branch from e6f4c3b to 080a5d6 Compare June 6, 2024 22:57

bettio merged commit 007919b into atomvm:release-0.6 Jun 7, 2024
79 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add simple http client #1123

Add simple http client #1123

bettio commented Mar 31, 2024 •

edited

Loading

UncleGrumpy left a comment

UncleGrumpy commented May 8, 2024 •

edited

Loading

UncleGrumpy left a comment

arpunk left a comment

pguyot left a comment

pguyot May 14, 2024

bettio May 14, 2024

pguyot May 14, 2024

bettio May 16, 2024

pguyot May 14, 2024

bettio May 16, 2024

pguyot May 14, 2024

UncleGrumpy May 14, 2024

bettio May 16, 2024

bettio May 16, 2024

UncleGrumpy May 14, 2024 •

edited

Loading

bettio May 16, 2024

UncleGrumpy May 16, 2024 •

edited

Loading

bettio May 16, 2024


		-define(DEFAULT_WANTED_HEADERS, [<<"Content-Length">>]).

		-record(http_client, {host, socket, parser, ref, parse_headers}).

Add simple http client #1123

Add simple http client #1123

Conversation

bettio commented Mar 31, 2024 • edited Loading

UncleGrumpy left a comment

Choose a reason for hiding this comment

UncleGrumpy commented May 8, 2024 • edited Loading

UncleGrumpy left a comment

Choose a reason for hiding this comment

arpunk left a comment

Choose a reason for hiding this comment

pguyot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UncleGrumpy May 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UncleGrumpy May 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bettio commented Mar 31, 2024 •

edited

Loading

UncleGrumpy commented May 8, 2024 •

edited

Loading

UncleGrumpy May 14, 2024 •

edited

Loading

UncleGrumpy May 16, 2024 •

edited

Loading