-
Notifications
You must be signed in to change notification settings - Fork 9
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
net/http client with wasip2 #31
Comments
Yes, a |
awesome, I have started work on this PR. thank you. |
Hi I am popping up on this issue as the problem that I am currently facing might be related. Problem:
Build commands:
Output:
After some research, I vaguely understood that http invocations were not originally supported in WASI. I would like to know, if the current issue is to have a core implementation in net/http to make HTTP calls for WASI target. Here is the code snippet:
|
For wasi/wasip2, we need to use special bindings to be able to use host functions. For wasip2 i have written some bindings on top of bindings generated via https://github.com/ydnar/wasm-tools-go I was planning on submitting the bindings i wrote as part of this issue. Currently a rough version of those bindings is available at https://github.com/rajatjindal/wasm-console/pkg/http-client (Sorry for any typos as i am typing from my phone) |
You can generate WASI-http bindings with wit-bindgen or wit-binden-go. Once you have the bindings you can implement a custom round trip. Once this issue is closed - you will be able to use this "WASI" round trip to make request with the http client. Right now I believe we are not able to overwrite the net transport - or atleast it is ignored. To get around this - just call your round trip function directly. I am cleaning up my implementation of this and will share it here once I push it. For wit-binden-go - you need to be on the dev branch of tiny-go for wasip2 support. You can do this with wasip1 but it's a lot easier using wasip2. Hope this helps - also on mobile - apologies if it doesn't make sense. |
Also interested in getting this added - let me know if I can help or test anything. |
I submitted a fix for this in this PR #32 |
I have been made aware that adding the bindings to tinygo might not be a good idea. could folks more familiar with pros/cons of adding the bindings discuss and help us reach a decision here. I am working on the bindings on the side (currently they are checked-in as part of wasm-console repo. depending on what we decide here, I will move it to a separate repo or submit a PR here. many thanks in advance. |
Great to see PR #32! Looks awesome. Thank you for submitting this! For what it's worth, I would not add the HTTP bindings to tinygo. At least not now. The WIT definitions still seem to be going through a lot of iteration. For that reason alone, I feel its best to place them in a x package or leave them to the developer to implement. What you have pushed gives enough flexibility for a round trip implementation that uses the bindings for HTTP. That said, I am not close to the wasip2 implementation or proposal progress. I'm sure others have much more informed reasoning. Regardless, thanks again! |
Tested the changes locally - not sure if this is just something I am missing in my setup but I did have to add UnknownNetworkError to unixsock for my module to build using the latest net package.
https://github.com/tinygo-org/net/blob/main/unixsock.go - looks like this was added in #25. This surfaced after updating the submodule in tinygo from commit Happy to push a PR if others observe this problem. |
submitted #33 to solve the above - it is not directly related to this issue but the error surfaced when testing a custom transport through tinygo ( net package caused compile errors). |
With both of the PRs above merged - I believe this can be closed out. I will look at getting a PR added to tinygo to update the submodule reference - but updating locally and building from source works for now. leaving examples of how this is being implemented and used in the wild |
please see tinygo-org/tinygo#4351 😸 |
now that we have added support for wasip2 to tinygo, i am wondering if we can add support for that in net/http package. The generated bindings are little raw for endusers, and most folks end up developing a little more ergonomic sdk on top of it.
right now whenever I have to develop a new wasip2 component that makes use of net/http for outgoing request, I end up having to copy/paste that code (or import) the client I wrote on top of wasip2 bindings
what would be nice is to add those to this repo with conditional build tag of
wasip2
. do you think adding such code will be something folks would be willing to accept to this repo?many thanks for considering
Rajat Jindal
The text was updated successfully, but these errors were encountered: