Begin implementation of wasi-http #5929

brendandburns · 2023-03-04T21:47:22Z

Initial draft, working, but with limitations.

Known issues:

Outbound requests only, inbound HTTP serving is not supported
~~Body for requests is not supported~~
~~Request options (e.g. timeout values) are not supported.~~
~~There are panics all over the place if you pass in bad pointers or unexpected values.~~

brendandburns · 2023-03-13T04:00:34Z

@pchickey I think this is ready for an initial high-level review.

Everything is implemented in terms of the wit-bindgen interfaces, and then an adapter is introduced which maps from the wit-bindgen to the current wasmtime::Linker implementation.

This means that it is very forward compatible with the component model linker going forward.

If you can take a quick look at the rough layout and see if this is on the path to merging that would be great.

After we get the rough layout right, we can move on to a more detailed review.

Thanks!

brendandburns · 2023-03-13T04:01:01Z

There's also a pretty extensive example that can be used to test this implementation in crates/wasi-http/example

pchickey

Exciting! Here's my first pass of feedback

crates/wasi-http/src/component_impl.rs

crates/wasi-http/Cargo.toml

pchickey · 2023-03-13T20:52:18Z

crates/wasi-http/src/component_impl.rs

+        },
+    )?;
+    linker.func_wrap(
+        "types",


I'm a bit surprised all these functions landed in the types module name, that may be something we have to look into...

yeah, surprised me to, seems like some sort of aliasing in wit-bindgen

brendandburns · 2023-03-14T04:44:12Z

Comments addressed ~~except the macro vs fn comment because I'm not clear how to resolve the compile error.~~

Please take another look.

Mossaka · 2023-03-15T22:29:52Z

crates/wasi-http/src/http_impl.rs

+            crate::types::Method::Options => Method::OPTIONS,
+            crate::types::Method::Trace => Method::TRACE,
+            crate::types::Method::Patch => Method::PATCH,
+            _ => return Err(anyhow::anyhow!("unknown method!")),


Suggested change

_ => return Err(anyhow::anyhow!("unknown method!")),

_ => return bail!("unknown method!"),

crates/wasi-http/src/lib.rs

brendandburns · 2023-03-18T05:30:52Z

Ok, this was switched to hyper. I'm not sure it really saved you very many dependencies, but if there are other ways to reduce the dependency count, let me know. Hyper doesn't implement TLS, so I had to pull in hyper_native_tls

I think this is ready to go, let me know if there is a good way to add tests, it seems the other modules don't really have tests.

brendandburns · 2023-03-21T03:33:57Z

@pchickey

Ok, I implemented timeouts. I think that the client side implementation is now basically feature complete. Ready for a more detailed review whenever you have time.

Thanks!

brendandburns · 2023-03-21T03:34:27Z

Oh, and I plan on starting the server side implementation in a different branch.

eduardomourar

I have tested this with a real-life Wasm module and it works as expected.

crates/wasi-http/Cargo.toml

Co-authored-by: Eduardo de Moura Rodrigues <16357187+eduardomourar@users.noreply.github.com>

Mossaka

mostly lgtm!

crates/wasi-http/src/streams_impl.rs

brendandburns · 2023-04-05T03:29:49Z

@pchickey I conditionalized tls to disable it on riscv64gc

~~I tried the prtest:full thing but it didn't seem to work for me, maybe I did it wrong...~~

~~Anyhow, perhaps we can give this another try? If there's a way for me to trigger the full test runs w/o you adding it to the merge queue, let me know what I need to change in my commit message.~~

Turns out the gh status was just cached and lagging, prtest:full appears to work correctly.

Turns out that ring doesn't work on s390x either, so I conditionalized that also. We may want to investigate how we can go back to native SSL, but that's for another PR...

Fix formatting, also disable tls on s390x

brendandburns · 2023-04-05T04:26:22Z

cc @pchickey

Ok, next issue hit:

The wasmtime::component::bindgen! macro expects a folder named wit in the same directory as the Cargo.toml

The challenge is that if we want to pull the wasi-http repository in as a git submodule, then the directory structure is messed up, so I used a symbolic link to link wit -> wasi-http/wit but it looks like those symbolic links don't work properly for Windows.

I can get rid of the git submodule, and just check in the wit files directly into this repo, it will make it a little more annoying to keep the wit files in-sync but it's do-able.

Let me know what you think.

`

brendandburns · 2023-04-05T04:29:21Z

It looks like maybe setting git config --global core.symlinks true in our builds would fix things? But I'm not certain where those builds are configured, I can't seem to find them in main.yml

cfallin · 2023-04-05T04:54:44Z

Is it possible to modify the macro to have a configurable path? I think we'd probably prefer that over most any directory-structure or build-time hack (symlinks, copying files, or the like).

eduardomourar · 2023-04-05T06:55:23Z

I believe the macro does accept a path property and you can check it here: https://github.com/bytecodealliance/wasmtime/blob/main/crates/component-macro/tests/codegen.rs

brendandburns · 2023-04-05T13:56:10Z

@eduardomourar thanks for the pointer (I don't think that's documented anywhere :() I updated this to use path and I removed the symlink.

crosses fingers

Add a path parameter to wit-bindgen, remove symlink.

Fix tests for places where SSL isn't supported.

brendandburns · 2023-04-05T17:48:51Z

@pchickey This passed complete CI, but it needed a rebase. I'm pretty sure that it will pass the merge queue this time.

Thanks for the patience!

crates/wasi-http/Cargo.toml

brendandburns · 2023-04-05T21:35:24Z

🥂 🎊 🥂

Thanks for the patience here, I'll send in some follow-on PRs.

brendandburns mentioned this pull request Mar 4, 2023

add Wit interfaces WebAssembly/wasi-http#3

Merged

brendandburns force-pushed the wasi_http branch 3 times, most recently from f746c98 to 9506b57 Compare March 13, 2023 03:57

brendandburns changed the title ~~[WIP] Begin implementation of wasi-http~~ Begin implementation of wasi-http Mar 13, 2023

brendandburns mentioned this pull request Mar 13, 2023

[WIP] Add a wasi-http support as a module #5678

Closed

3 tasks

pchickey reviewed Mar 14, 2023

View reviewed changes

Mossaka reviewed Mar 15, 2023

View reviewed changes

This was referenced Mar 17, 2023

Namespacing for Worlds WebAssembly/component-model#177

Closed

feat: add basic scaffolding for wasi http bytecodealliance/preview2-prototyping#85

Merged

brendandburns added 2 commits March 18, 2023 05:19

Integrate experimental HTTP into wasmtime.

1e7c789

Reset Cargo.lock

63887e0

brendandburns force-pushed the wasi_http branch from fe8cef7 to 63887e0 Compare March 18, 2023 05:28

eduardomourar mentioned this pull request Mar 18, 2023

Use WASI to simplify resource provider development richardhboyd/cloudformation-cli-rust-plugin-for-cloudformation-resource-type-providers-in-rust#1

Open

brendandburns added 2 commits March 18, 2023 15:45

Switch to bail!, plumb options partially.

8ecc77e

Implement timeouts.

8657c7c

brendandburns added 2 commits March 21, 2023 03:44

Remove generated files & wasm, add Makefile

c983d73

Remove generated code textfile

febe6bd

brendandburns mentioned this pull request Mar 23, 2023

Initial implementation of wasi-http serving #6091

Closed

eduardomourar suggested changes Mar 23, 2023

View reviewed changes

crates/wasi-http/Cargo.toml Outdated Show resolved Hide resolved

crates/wasi-http/Cargo.toml Outdated Show resolved Hide resolved

brendandburns and others added 2 commits March 23, 2023 16:48

Update crates/wasi-http/Cargo.toml

320d620

Co-authored-by: Eduardo de Moura Rodrigues <16357187+eduardomourar@users.noreply.github.com>

Update crates/wasi-http/Cargo.toml

356ffa6

Co-authored-by: Eduardo de Moura Rodrigues <16357187+eduardomourar@users.noreply.github.com>

eduardomourar approved these changes Mar 24, 2023

View reviewed changes

Mossaka approved these changes Mar 24, 2023

View reviewed changes

crates/wasi-http/src/streams_impl.rs Outdated Show resolved Hide resolved

brendandburns force-pushed the wasi_http branch from ade7761 to 6d6feeb Compare April 5, 2023 03:40

prtest:full

b6b70f5

Fix formatting, also disable tls on s390x

brendandburns force-pushed the wasi_http branch from 6d6feeb to b6b70f5 Compare April 5, 2023 04:04

prtest:full

c16aa23

Add a path parameter to wit-bindgen, remove symlink.

brendandburns force-pushed the wasi_http branch 4 times, most recently from 9f18454 to 85a2e52 Compare April 5, 2023 17:10

prtest:full

35e6a59

Fix tests for places where SSL isn't supported.

brendandburns force-pushed the wasi_http branch from 85a2e52 to 35e6a59 Compare April 5, 2023 17:10

Merge branch 'main' into wasi_http

9b2312b

Merge branch 'main' into wasi_http

e536195

pchickey reviewed Apr 5, 2023

View reviewed changes

crates/wasi-http/Cargo.toml Show resolved Hide resolved

Update crates/wasi-http/Cargo.toml

224ba1c

pchickey approved these changes Apr 5, 2023

View reviewed changes

pchickey enabled auto-merge April 5, 2023 20:00

pchickey added this pull request to the merge queue Apr 5, 2023

Merged via the queue into bytecodealliance:main with commit 2d34dbe Apr 5, 2023

brendandburns mentioned this pull request Apr 5, 2023

Some improvements to the wasi-http client implementation of write. #6161

Merged

rvolosatovs mentioned this pull request Apr 24, 2023

feat(http): allow custom OutgoingHandler implementations #6272

Closed

pchickey mentioned this pull request May 9, 2023

Add release notes for 9.0.0 #6364

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Begin implementation of wasi-http #5929

Begin implementation of wasi-http #5929

brendandburns commented Mar 4, 2023 •

edited

Loading

brendandburns commented Mar 13, 2023

brendandburns commented Mar 13, 2023

pchickey left a comment

pchickey Mar 13, 2023

brendandburns Mar 14, 2023

brendandburns commented Mar 14, 2023 •

edited

Loading

Mossaka Mar 15, 2023

brendandburns commented Mar 18, 2023

brendandburns commented Mar 21, 2023

brendandburns commented Mar 21, 2023

eduardomourar left a comment

Mossaka left a comment

brendandburns commented Apr 5, 2023 •

edited

Loading

brendandburns commented Apr 5, 2023

brendandburns commented Apr 5, 2023

cfallin commented Apr 5, 2023

eduardomourar commented Apr 5, 2023

brendandburns commented Apr 5, 2023

brendandburns commented Apr 5, 2023

brendandburns commented Apr 5, 2023

	_ => return Err(anyhow::anyhow!("unknown method!")),
	_ => return bail!("unknown method!"),

Begin implementation of wasi-http #5929

Begin implementation of wasi-http #5929

Conversation

brendandburns commented Mar 4, 2023 • edited Loading

brendandburns commented Mar 13, 2023

brendandburns commented Mar 13, 2023

pchickey left a comment

Choose a reason for hiding this comment

pchickey Mar 13, 2023

Choose a reason for hiding this comment

brendandburns Mar 14, 2023

Choose a reason for hiding this comment

brendandburns commented Mar 14, 2023 • edited Loading

Mossaka Mar 15, 2023

Choose a reason for hiding this comment

brendandburns commented Mar 18, 2023

brendandburns commented Mar 21, 2023

brendandburns commented Mar 21, 2023

eduardomourar left a comment

Choose a reason for hiding this comment

Mossaka left a comment

Choose a reason for hiding this comment

brendandburns commented Apr 5, 2023 • edited Loading

brendandburns commented Apr 5, 2023

brendandburns commented Apr 5, 2023

cfallin commented Apr 5, 2023

eduardomourar commented Apr 5, 2023

brendandburns commented Apr 5, 2023

brendandburns commented Apr 5, 2023

brendandburns commented Apr 5, 2023

brendandburns commented Mar 4, 2023 •

edited

Loading

brendandburns commented Mar 14, 2023 •

edited

Loading

brendandburns commented Apr 5, 2023 •

edited

Loading