deislabs / wagi

Write HTTP handlers in WebAssembly with a minimal amount of work
Apache License 2.0
889 stars 45 forks source link

Centralise Wasm linking and instantiation #143

Closed itowlson closed 2 years ago

itowlson commented 2 years ago

There were two places we instantiated a Wasm module, one for dynamic route probing and one for route handling. This caused a bug because the first one did not link the HTTP API. Radu fixed this in #142 by adding HTTP linking at the first site, but, as he rightly noted, it would be better to have only one place where linking and instantiation was done. This PR does that.

This PR also attempts to add a test for calling HTTP modules, but there is an interaction between the Tokio test runner (or something) and the Wasmtime HTTP implementation (which spawns a blocking Tokio runtime precisely to avoid such interactions, but which something somehow thwarts) which causes the test to hang at https://github.com/deislabs/wasi-experimental-http/blob/3d9622cf4ee4c33f0a1b12f53b7790d237886358/crates/wasi-experimental-http-wasmtime/src/lib.rs#L547. For now, therefore, I have left the Wasm module in with instructions on how to test manually, but commented out the automated test. Further investigation would be nice but possibly a distraction; it's certainly not something I feel comfortable tackling.

radu-matei commented 2 years ago

Re: #139 , given it would need a sizeable rebase now, would you mind running cargo fmt here, please? Thanks!

itowlson commented 2 years ago

@radu-matei I don't want to run cargo fmt here because it will result in probably-extensive changes to code that isn't part of this PR, which I avoid because it makes PRs hard to review (at least, it makes them hard for me to review!). We can create a new PR that is guaranteed to be formatting-only changes.