bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
15.38k stars 1.3k forks source link

calling the `wasi` crate's HTTP APIs from the CLI world crashes wasmtime #7998

Closed yoshuawuyts closed 8 months ago

yoshuawuyts commented 8 months ago

Test Case

GitHub doesn't like .wasm file uploads it seems:

We don’t support that file type.
Try again with GIF, JPEG, JPG, MOV, MP4, PNG, SVG, WEBM, CPUPROFILE, CSV, DMP, DOCX, FODG, FODP, FODS, FODT, GZ, JSON, JSONC, LOG, MD, ODF, ODG, ODP, ODS, ODT, PATCH, PDF, PPTX, TGZ, TXT, XLS, XLSX or ZIP.

Steps to Reproduce

Using this branch of yoshuawuyts/wasm-http-tools:

git clone yoshuawuyts/wasm-http-tools
git checkout yoshuawuyts:failing-test-repro
./run.sh

Expected Results

I expected this to run and not crash.

Actual Results

    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
Error: failed to run main module `target/main.wasm`

Caused by:
    0: failed to invoke `run` function
    1: error while executing at wasm backtrace:
           0: 0x2c796f - wit-component:shim!indirect-wasi:http/outgoing-handler@0.2.0-handle
           1: 0xbedb - <unknown>!wasi::bindings::wasi::http::outgoing_handler::handle::hbd9a4f4888720497
           2: 0x2719 - <unknown>!main::main::h5f582b6fe5e8781c
           3: 0x1c32 - <unknown>!core::ops::function::FnOnce::call_once::h03dbeeeaf506d8dd
           4: 0x1a58 - <unknown>!std::sys_common::backtrace::__rust_begin_short_backtrace::hb341647f02aac7d1
           5: 0x18e4 - <unknown>!std::rt::lang_start::{{closure}}::h7606763051b64eac
           6: 0x16084 - <unknown>!std::rt::lang_start_internal::h11043ae9961d0df9
           7: 0x1880 - <unknown>!std::rt::lang_start::hd1c6474d6799145b
           8: 0x2a95 - <unknown>!__main_void
           9: 0x17fb - <unknown>!_start
          10: 0x2c3aed - wit-component:adapter:wasi_snapshot_preview1!wasi:cli/run@0.2.0#run
       note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable may show more debugging information
    2: ErrorCode::HttpRequestUriInvalid
yosh@MacBook-Pro wasm-http-tools %      

Versions and Environment

Wasmtime version or commit: v18.0.1

Operating system: MacOS

Architecture: ARM

Extra Info

Talked to Dan; this seems like it might be an issue with the adapter? It's failing right in the middle. The run.sh command calls wasmtime run -S http - and perhaps that's not being picked up correctly in a CLI world? Unsure; we probably need to investigate.

pchickey commented 8 months ago

For wasm uploads we usually stick them in a .zip. I think the restriction is related to CORS

alexcrichton commented 8 months ago

I believe that this is an expected error related to the "protocol" that wasi-http is expecting. I can get your example to work by changing it to:

use wasi::http::{
    outgoing_handler::{handle, OutgoingRequest},
    types::{Fields, Method, RequestOptions, Scheme},
};
use wasi_http_client::Poller;

fn main() {
    let mut poller = Poller::new();
    let fields = Fields::new();
    let req = OutgoingRequest::new(fields);
    req.set_method(&Method::Get).unwrap();
    req.set_scheme(Some(&Scheme::Https)).unwrap();
    req.set_path_with_query(Some("/")).unwrap();
    req.set_authority(Some("example.com")).unwrap();
    let res = handle(req, None).unwrap();
    let pollable = res.subscribe();
    poller.insert(pollable);
    // assert!(&pollable.ready(), "pollable was not ready");
    poller.block_until();
    drop(poller);
    dbg!(res);
}

Specifically path_with_query doesn't take the whole hostname, just the / and the trailing bits. You'll also need to call set_authority and set_scheme. Finally the poller is a "child resource" of res so you'll need to drop that first before res. After all that though I see:

[crates/wasi-http-client/examples/main.rs:21:5] res = FutureIncomingResponse {
    handle: Resource {
        handle: 0,
    },
}

Due to this if you hit confusing errors I might also recommend WASMTIME_LOG=warn to help diagnose a bit.

yoshuawuyts commented 8 months ago

That did the trick; thank you!

kaivol commented 7 months ago

I also came across this and wondered whether this is really the desired behavior. Shouldn't handle return an error code that can be handled in the user code instead of simply crashing?

alexcrichton commented 7 months ago

That's a good point! @kaivol would you be up for opening a dedicated issue for that?

kaivol commented 7 months ago

Yes I can do that See #8269