Merge pull request #3919 from wasmerio/reqwest-errors

Handle non-200 status codes when downloading WEBC files

lib/wasi-web/Cargo.toml

@@ -41,6 +41,7 @@ dummy-waker = "^1"
 wat = "1.0"
 anyhow = "1.0.66"
 futures = "0.3.25"
+http = "0.2.9"
 
 [dependencies.parking_lot]
 version = "^0.11"

lib/wasi-web/src/common.rs

@@ -141,7 +141,7 @@ pub async fn fetch(
     method: &str,
     _gzip: bool,
     cors_proxy: Option<String>,
-    headers: Vec<(String, String)>,
+    headers: &http::HeaderMap,
     data: Option<Vec<u8>>,
 ) -> Result<Response, anyhow::Error> {
     let mut opts = RequestInit::new();
@@ -162,7 +162,8 @@ pub async fn fetch(
 
         let set_headers = request.headers();
         for (name, val) in headers.iter() {
-            set_headers.set(name.as_str(), val.as_str()).map_err(|_| {
+            let val = String::from_utf8_lossy(val.as_bytes());
+            set_headers.set(name.as_str(), &val).map_err(|_| {
                 anyhow::anyhow!("could not apply request header: '{name}': '{val}'")
             })?;
         }

lib/wasi-web/src/glue.rs

@@ -9,7 +9,6 @@ use tokio::sync::mpsc;
 use tracing::{debug, error, info, trace, warn};
 use wasm_bindgen::{prelude::*, JsCast};
 use wasmer_wasix::{
-    bin_factory::ModuleCache,
     capabilities::Capabilities,
     os::{Console, InputEvent, Tty, TtyOptions},
     Pipe,
@@ -38,9 +37,9 @@ pub fn main() {
     set_panic_hook();
 }
 
-pub const DEFAULT_BOOT_WEBC: &'static str = "sharrattj/bash";
+pub const DEFAULT_BOOT_WEBC: &str = "sharrattj/bash";
 //pub const DEFAULT_BOOT_WEBC: &str = "sharrattj/dash";
-pub const DEFAULT_BOOT_USES: [&'static str; 2] = ["sharrattj/coreutils", "sharrattj/catsay"];
+pub const DEFAULT_BOOT_USES: [&str; 2] = ["sharrattj/coreutils", "sharrattj/catsay"];
 
 #[wasm_bindgen]
 pub fn start() -> Result<(), JsValue> {
@@ -134,20 +133,18 @@ pub fn start() -> Result<(), JsValue> {
         tty_options,
     );
 
-    let compiled_modules = Arc::new(ModuleCache::new(None, None, false));
-
     let location = url::Url::parse(location.as_str()).unwrap();
     let mut console = if let Some(init) = location
         .query_pairs()
         .filter(|(key, _)| key == "init")
         .next()
         .map(|(_, val)| val.to_string())
     {
-        let mut console = Console::new(init.as_str(), runtime.clone(), compiled_modules);
+        let mut console = Console::new(init.as_str(), runtime.clone());
         console = console.with_no_welcome(true);
         console
     } else {
-        let mut console = Console::new(DEFAULT_BOOT_WEBC, runtime.clone(), compiled_modules);
+        let mut console = Console::new(DEFAULT_BOOT_WEBC, runtime.clone());
         console = console.with_uses(DEFAULT_BOOT_USES.iter().map(|a| a.to_string()).collect());
         console
     };

lib/wasi-web/src/pool.rs

@@ -662,7 +662,7 @@ pub fn wasm_entry_point(
         run(TaskWasmRunProperties {
             ctx,
             store,
-            result: task.result,
+            trigger_result: task.result,
         });
     };
 }

lib/wasi-web/src/runtime.rs

@@ -6,6 +6,7 @@ use std::time::Duration;
 use std::{future::Future, io, pin::Pin, sync::Arc, task::Poll};
 
 use futures::future::BoxFuture;
+use http::{HeaderMap, StatusCode};
 use js_sys::Promise;
 use tokio::{
     io::{AsyncRead, AsyncSeek, AsyncWrite},
@@ -20,7 +21,7 @@ use wasmer_wasix::{
     http::{DynHttpClient, HttpRequest, HttpResponse},
     os::{TtyBridge, TtyOptions},
     runtime::task_manager::TaskWasm,
-    VirtualFile, VirtualNetworking, VirtualTaskManager, WasiRuntime, WasiThreadError, WasiTtyState,
+    VirtualFile, VirtualNetworking, VirtualTaskManager, WasiThreadError, WasiTtyState,
 };
 use web_sys::WebGl2RenderingContext;
 
@@ -416,7 +417,7 @@ impl VirtualFile for TermLog {
     }
 }
 
-impl WasiRuntime for WebRuntime {
+impl wasmer_wasix::Runtime for WebRuntime {
     fn networking(&self) -> &wasmer_wasix::virtual_net::DynVirtualNetworking {
         &self.net
     }
@@ -483,33 +484,28 @@ struct WebHttpClient {
 impl WebHttpClient {
     async fn do_request(request: HttpRequest) -> Result<HttpResponse, anyhow::Error> {
         let resp = crate::common::fetch(
-            &request.url,
-            &request.method,
+            request.url.as_str(),
+            request.method.as_str(),
             request.options.gzip,
             request.options.cors_proxy,
-            request.headers,
+            &request.headers,
             request.body,
         )
         .await?;
 
-        let ok = resp.ok();
         let redirected = resp.redirected();
-        let status = resp.status();
-        let status_text = resp.status_text();
+        let status = StatusCode::from_u16(resp.status())?;
 
         let data = crate::common::get_response_data(resp).await?;
 
-        let headers = Vec::new();
         // FIXME: we can't implement this as the method resp.headers().keys() is missing!
         // how else are we going to parse the headers?
+        let headers = HeaderMap::new();
 
         debug!("received {} bytes", data.len());
         let resp = HttpResponse {
-            pos: 0,
-            ok,
             redirected,
             status,
-            status_text,
             headers,
             body: Some(data),
         };

lib/wasi/src/http/client.rs

@@ -1,6 +1,8 @@
 use std::{collections::HashSet, ops::Deref, sync::Arc};
 
 use futures::future::BoxFuture;
+use http::{HeaderMap, Method, StatusCode};
+use url::Url;
 
 /// Defines http client permissions.
 #[derive(Clone, Debug)]
@@ -47,46 +49,62 @@ pub struct HttpRequestOptions {
 
 // TODO: use types from http crate?
 pub struct HttpRequest {
-    pub url: String,
-    pub method: String,
-    pub headers: Vec<(String, String)>,
+    pub url: Url,
+    pub method: Method,
+    pub headers: HeaderMap,
     pub body: Option<Vec<u8>>,
     pub options: HttpRequestOptions,
 }
 
 impl std::fmt::Debug for HttpRequest {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let HttpRequest {
+            url,
+            method,
+            headers,
+            body,
+            options,
+        } = self;
+
         f.debug_struct("HttpRequest")
-            .field("url", &self.url)
-            .field("method", &self.method)
-            .field("headers", &self.headers)
-            .field("body", &self.body.as_deref().map(String::from_utf8_lossy))
-            .field("options", &self.options)
+            .field("url", &format_args!("{}", url))
+            .field("method", method)
+            .field("headers", headers)
+            .field("body", &body.as_deref().map(String::from_utf8_lossy))
+            .field("options", &options)
             .finish()
     }
 }
 
 // TODO: use types from http crate?
 pub struct HttpResponse {
-    pub pos: usize,
     pub body: Option<Vec<u8>>,
-    pub ok: bool,
     pub redirected: bool,
-    pub status: u16,
-    pub status_text: String,
-    pub headers: Vec<(String, String)>,
+    pub status: StatusCode,
+    pub headers: HeaderMap,
+}
+
+impl HttpResponse {
+    pub fn is_ok(&self) -> bool {
+        !self.status.is_client_error() && !self.status.is_server_error()
+    }
 }
 
 impl std::fmt::Debug for HttpResponse {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let HttpResponse {
+            body,
+            redirected,
+            status,
+            headers,
+        } = self;
+
         f.debug_struct("HttpResponse")
-            .field("pos", &self.pos)
-            .field("body", &self.body.as_deref().map(String::from_utf8_lossy))
-            .field("ok", &self.ok)
-            .field("redirected", &self.redirected)
-            .field("status", &self.status)
-            .field("status_text", &self.status_text)
-            .field("headers", &self.headers)
+            .field("ok", &self.is_ok())
+            .field("redirected", &redirected)
+            .field("status", &status)
+            .field("headers", &headers)
+            .field("body", &body.as_deref().map(String::from_utf8_lossy))
             .finish()
     }
 }

lib/wasi/src/http/client_impl.rs

@@ -1,12 +1,13 @@
-use std::string::FromUtf8Error;
 use std::sync::Arc;
 
-use crate::bindings::wasix_http_client_v1 as sys;
-use crate::{capabilities::Capabilities, Runtime};
+use http::{HeaderMap, HeaderValue};
+use url::Url;
 
 use crate::{
+    bindings::wasix_http_client_v1 as sys,
+    capabilities::Capabilities,
     http::{DynHttpClient, HttpClientCapabilityV1},
-    WasiEnv,
+    Runtime, WasiEnv,
 };
 
 impl std::fmt::Display for sys::Method<'_> {
@@ -92,11 +93,12 @@ impl sys::WasixHttpClientV1 for WasixHttpClientImpl {
             .headers
             .into_iter()
             .map(|h| {
-                let value = String::from_utf8(h.value.to_vec())?;
-                Ok((h.key.to_string(), value))
+                let value = HeaderValue::from_bytes(h.value).map_err(|e| e.to_string())?;
+                let key =
+                    http::HeaderName::from_bytes(h.key.as_bytes()).map_err(|e| e.to_string())?;
+                Ok((key, value))
             })
-            .collect::<Result<Vec<_>, FromUtf8Error>>()
-            .map_err(|_| "non-utf8 request header")?;
+            .collect::<Result<HeaderMap, String>>()?;
 
         // FIXME: stream body...
 
@@ -108,9 +110,22 @@ impl sys::WasixHttpClientV1 for WasixHttpClientImpl {
             None => None,
         };
 
+        let method = match request.method {
+            sys::Method::Get => http::Method::GET,
+            sys::Method::Head => http::Method::HEAD,
+            sys::Method::Post => http::Method::POST,
+            sys::Method::Put => http::Method::PUT,
+            sys::Method::Delete => http::Method::DELETE,
+            sys::Method::Connect => http::Method::CONNECT,
+            sys::Method::Options => http::Method::OPTIONS,
+            sys::Method::Trace => http::Method::TRACE,
+            sys::Method::Patch => http::Method::PATCH,
+            sys::Method::Other(other) => return Err(format!("Unknown method: {other}")),
+        };
+
         let req = crate::http::HttpRequest {
-            url: request.url.to_string(),
-            method: request.method.to_string(),
+            url: Url::parse(request.url).map_err(|e| e.to_string())?,
+            method,
             headers,
             body,
             options: crate::http::HttpRequestOptions {
@@ -128,10 +143,10 @@ impl sys::WasixHttpClientV1 for WasixHttpClientImpl {
 
         let res_headers = res
             .headers
-            .into_iter()
-            .map(|(key, value)| sys::HeaderResult {
-                key,
-                value: value.into_bytes(),
+            .iter()
+            .map(|(name, value)| sys::HeaderResult {
+                key: name.to_string(),
+                value: value.as_bytes().to_vec(),
             })
             .collect();
 
@@ -143,7 +158,7 @@ impl sys::WasixHttpClientV1 for WasixHttpClientImpl {
 
         Ok({
             sys::Response {
-                status: res.status,
+                status: res.status.as_u16(),
                 headers: res_headers,
                 body: res_body,
                 // TODO: provide redirect urls?

lib/wasi/src/http/reqwest.rs

@@ -30,23 +30,14 @@ impl ReqwestHttpClient {
             .build()
             .context("Failed to construct http request")?;
 
-        let response = client.execute(request).await?;
-
-        let status = response.status().as_u16();
-        let status_text = response.status().as_str().to_string();
-        // TODO: prevent redundant header copy.
-        let headers = response
-            .headers()
-            .iter()
-            .map(|(k, v)| (k.to_string(), v.to_str().unwrap().to_string()))
-            .collect();
+        let mut response = client.execute(request).await?;
+        let headers = std::mem::take(response.headers_mut());
+
+        let status = response.status();
         let data = response.bytes().await?.to_vec();
 
         Ok(HttpResponse {
-            pos: 0usize,
-            ok: true,
             status,
-            status_text,
             redirected: false,
             body: Some(data),
             headers,