Fix errors becoming traps in wasi-http (#8272)

This commit fixes an issue with errors in the `wasmtime-wasi-http` crate
by using the `trappable_error_type` bindgen configuration option in the
same manner as other WASI interfaces in the `wasmtime-wasi` crate.
Unfortunately due to coherence the `TrappableError<T>` type itself could
not be used but it was small enough it wasn't much effort to duplicate.

Closes #8269

crates/test-programs/src/bin/http_outbound_request_missing_path_and_query.rs

@@ -0,0 +1,16 @@
+use test_programs::wasi::http::outgoing_handler::{handle, OutgoingRequest};
+use test_programs::wasi::http::types::{Fields, Method, Scheme};
+
+fn main() {
+    let fields = Fields::new();
+    let req = OutgoingRequest::new(fields);
+    req.set_method(&Method::Get).unwrap();
+    req.set_scheme(Some(&Scheme::Https)).unwrap();
+    req.set_authority(Some("example.com")).unwrap();
+
+    // Don't set path/query
+    // req.set_path_with_query(Some("/")).unwrap();
+
+    let res = handle(req, None);
+    assert!(res.is_err());
+}

crates/wasi-http/src/error.rs

@@ -0,0 +1,55 @@
+use crate::bindings::http::types::ErrorCode;
+use std::error::Error;
+use std::fmt;
+use wasmtime_wasi::ResourceTableError;
+
+pub type HttpResult<T, E = HttpError> = Result<T, E>;
+
+/// A `wasi:http`-specific error type used to represent either a trap or an
+/// [`ErrorCode`].
+///
+/// Modeled after [`TrappableError`](wasmtime_wasi::TrappableError).
+#[repr(transparent)]
+pub struct HttpError {
+    err: anyhow::Error,
+}
+
+impl HttpError {
+    pub fn trap(err: impl Into<anyhow::Error>) -> HttpError {
+        HttpError { err: err.into() }
+    }
+
+    pub fn downcast(self) -> anyhow::Result<ErrorCode> {
+        self.err.downcast()
+    }
+
+    pub fn downcast_ref(&self) -> Option<&ErrorCode> {
+        self.err.downcast_ref()
+    }
+}
+
+impl From<ErrorCode> for HttpError {
+    fn from(error: ErrorCode) -> Self {
+        Self { err: error.into() }
+    }
+}
+
+impl From<ResourceTableError> for HttpError {
+    fn from(error: ResourceTableError) -> Self {
+        HttpError::trap(error)
+    }
+}
+
+impl fmt::Debug for HttpError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.err.fmt(f)
+    }
+}
+
+impl fmt::Display for HttpError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.err.fmt(f)
+    }
+}
+
+impl Error for HttpError {}

crates/wasi-http/src/http_impl.rs

@@ -17,7 +17,7 @@ impl<T: WasiHttpView> outgoing_handler::Host for T {
         &mut self,
         request_id: Resource<HostOutgoingRequest>,
         options: Option<Resource<types::RequestOptions>>,
-    ) -> wasmtime::Result<Result<Resource<HostFutureIncomingResponse>, types::ErrorCode>> {
+    ) -> crate::HttpResult<Resource<HostFutureIncomingResponse>> {
         let opts = options.and_then(|opts| self.table().get(&opts).ok());
 
         let connect_timeout = opts
@@ -47,7 +47,7 @@ impl<T: WasiHttpView> outgoing_handler::Host for T {
             types::Method::Patch => Method::PATCH,
             types::Method::Other(m) => match hyper::Method::from_bytes(m.as_bytes()) {
                 Ok(method) => method,
-                Err(_) => return Ok(Err(types::ErrorCode::HttpRequestMethodInvalid)),
+                Err(_) => return Err(types::ErrorCode::HttpRequestMethodInvalid.into()),
             },
         });
 
@@ -56,7 +56,7 @@ impl<T: WasiHttpView> outgoing_handler::Host for T {
             Scheme::Https => (true, http::uri::Scheme::HTTPS, 443),
 
             // We can only support http/https
-            Scheme::Other(_) => return Ok(Err(types::ErrorCode::HttpProtocolError)),
+            Scheme::Other(_) => return Err(types::ErrorCode::HttpProtocolError.into()),
         };
 
         let authority = if let Some(authority) = req.authority {
@@ -94,23 +94,13 @@ impl<T: WasiHttpView> outgoing_handler::Host for T {
             .body(body)
             .map_err(|err| internal_error(err.to_string()))?;
 
-        let result = self.send_request(OutgoingRequest {
+        self.send_request(OutgoingRequest {
             use_tls,
             authority,
             request,
             connect_timeout,
             first_byte_timeout,
             between_bytes_timeout,
-        });
-
-        // attempt to downcast the error to a ErrorCode
-        // so that the guest may handle it
-        match result {
-            Ok(response) => Ok(Ok(response)),
-            Err(err) => match err.downcast::<types::ErrorCode>() {
-                Ok(err) => Ok(Err(err)),
-                Err(err) => Err(err),
-            },
-        }
+        })
     }
 }

crates/wasi-http/src/lib.rs

@@ -1,6 +1,9 @@
+use crate::bindings::http::types::ErrorCode;
+pub use crate::error::{HttpError, HttpResult};
 pub use crate::types::{WasiHttpCtx, WasiHttpView};
 
 pub mod body;
+mod error;
 pub mod http_impl;
 pub mod io;
 pub mod proxy;
@@ -34,27 +37,28 @@ pub mod bindings {
             "wasi:http/types/incoming-request": super::types::HostIncomingRequest,
             "wasi:http/types/fields": super::types::HostFields,
             "wasi:http/types/request-options": super::types::HostRequestOptions,
-        }
+        },
+        trappable_error_type: {
+            "wasi:http/types/error-code" => crate::HttpError,
+        },
     });
 
     pub use wasi::http;
 }
 
-pub(crate) fn dns_error(rcode: String, info_code: u16) -> bindings::http::types::ErrorCode {
-    bindings::http::types::ErrorCode::DnsError(bindings::http::types::DnsErrorPayload {
+pub(crate) fn dns_error(rcode: String, info_code: u16) -> ErrorCode {
+    ErrorCode::DnsError(bindings::http::types::DnsErrorPayload {
         rcode: Some(rcode),
         info_code: Some(info_code),
     })
 }
 
-pub(crate) fn internal_error(msg: String) -> bindings::http::types::ErrorCode {
-    bindings::http::types::ErrorCode::InternalError(Some(msg))
+pub(crate) fn internal_error(msg: String) -> ErrorCode {
+    ErrorCode::InternalError(Some(msg))
 }
 
 /// Translate a [`http::Error`] to a wasi-http `ErrorCode` in the context of a request.
-pub fn http_request_error(err: http::Error) -> bindings::http::types::ErrorCode {
-    use bindings::http::types::ErrorCode;
-
+pub fn http_request_error(err: http::Error) -> ErrorCode {
     if err.is::<http::uri::InvalidUri>() {
         return ErrorCode::HttpRequestUriInvalid;
     }
@@ -65,8 +69,7 @@ pub fn http_request_error(err: http::Error) -> bindings::http::types::ErrorCode
 }
 
 /// Translate a [`hyper::Error`] to a wasi-http `ErrorCode` in the context of a request.
-pub fn hyper_request_error(err: hyper::Error) -> bindings::http::types::ErrorCode {
-    use bindings::http::types::ErrorCode;
+pub fn hyper_request_error(err: hyper::Error) -> ErrorCode {
     use std::error::Error;
 
     // If there's a source, we might be able to extract a wasi-http error from it.
@@ -82,8 +85,7 @@ pub fn hyper_request_error(err: hyper::Error) -> bindings::http::types::ErrorCod
 }
 
 /// Translate a [`hyper::Error`] to a wasi-http `ErrorCode` in the context of a response.
-pub fn hyper_response_error(err: hyper::Error) -> bindings::http::types::ErrorCode {
-    use bindings::http::types::ErrorCode;
+pub fn hyper_response_error(err: hyper::Error) -> ErrorCode {
     use std::error::Error;
 
     if err.is_timeout() {

crates/wasi-http/src/types.rs

@@ -5,7 +5,7 @@ use crate::io::TokioIo;
 use crate::{
     bindings::http::types::{self, Method, Scheme},
     body::{HostIncomingBody, HyperIncomingBody, HyperOutgoingBody},
-    dns_error, hyper_request_error,
+    dns_error, hyper_request_error, HttpResult,
 };
 use http_body_util::BodyExt;
 use hyper::header::HeaderName;
@@ -63,7 +63,7 @@ pub trait WasiHttpView: Send {
     fn send_request(
         &mut self,
         request: OutgoingRequest,
-    ) -> wasmtime::Result<Resource<HostFutureIncomingResponse>>
+    ) -> HttpResult<Resource<HostFutureIncomingResponse>>
     where
         Self: Sized,
     {
@@ -121,7 +121,7 @@ pub fn default_send_request(
         first_byte_timeout,
         between_bytes_timeout,
     }: OutgoingRequest,
-) -> wasmtime::Result<Resource<HostFutureIncomingResponse>> {
+) -> HttpResult<Resource<HostFutureIncomingResponse>> {
     let handle = wasmtime_wasi::runtime::spawn(async move {
         let resp = handler(
             authority,

crates/wasi-http/src/types_impl.rs

@@ -14,10 +14,14 @@ use std::str::FromStr;
 use wasmtime::component::{Resource, ResourceTable};
 use wasmtime_wasi::{
     bindings::io::streams::{InputStream, OutputStream},
-    Pollable,
+    Pollable, ResourceTableError,
 };
 
 impl<T: WasiHttpView> crate::bindings::http::types::Host for T {
+    fn convert_error_code(&mut self, err: crate::HttpError) -> wasmtime::Result<types::ErrorCode> {
+        err.downcast()
+    }
+
     fn http_error_code(
         &mut self,
         err: wasmtime::component::Resource<types::IoError>,
@@ -49,7 +53,10 @@ fn get_content_length(fields: &FieldMap) -> Result<Option<u64>, ()> {
 
 /// Take ownership of the underlying [`FieldMap`] associated with this fields resource. If the
 /// fields resource references another fields, the returned [`FieldMap`] will be cloned.
-fn move_fields(table: &mut ResourceTable, id: Resource<HostFields>) -> wasmtime::Result<FieldMap> {
+fn move_fields(
+    table: &mut ResourceTable,
+    id: Resource<HostFields>,
+) -> Result<FieldMap, ResourceTableError> {
     match table.delete(id)? {
         HostFields::Ref { parent, get_fields } => {
             let entry = table.get_any_mut(parent)?;
@@ -874,7 +881,7 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostOutgoingBody for T {
         &mut self,
         id: Resource<HostOutgoingBody>,
         ts: Option<Resource<Trailers>>,
-    ) -> wasmtime::Result<Result<(), types::ErrorCode>> {
+    ) -> crate::HttpResult<()> {
         let body = self.table().delete(id)?;
 
         let ts = if let Some(ts) = ts {
@@ -883,7 +890,8 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostOutgoingBody for T {
             None
         };
 
-        Ok(body.finish(ts))
+        body.finish(ts)?;
+        Ok(())
     }
 
     fn drop(&mut self, id: Resource<HostOutgoingBody>) -> wasmtime::Result<()> {

crates/wasi-http/tests/all/async_.rs

@@ -97,3 +97,13 @@ async fn http_outbound_request_content_length() -> Result<()> {
     let server = Server::http1()?;
     run(HTTP_OUTBOUND_REQUEST_CONTENT_LENGTH_COMPONENT, &server).await
 }
+
+#[test_log::test(tokio::test(flavor = "multi_thread"))]
+async fn http_outbound_request_missing_path_and_query() -> Result<()> {
+    let server = Server::http1()?;
+    run(
+        HTTP_OUTBOUND_REQUEST_MISSING_PATH_AND_QUERY_COMPONENT,
+        &server,
+    )
+    .await
+}

crates/wasi-http/tests/all/main.rs

@@ -17,13 +17,13 @@ use wasmtime_wasi_http::{
     body::HyperIncomingBody,
     io::TokioIo,
     types::{self, HostFutureIncomingResponse, IncomingResponseInternal, OutgoingRequest},
-    WasiHttpCtx, WasiHttpView,
+    HttpResult, WasiHttpCtx, WasiHttpView,
 };
 
 mod http_server;
 
 type RequestSender = Arc<
-    dyn Fn(&mut Ctx, OutgoingRequest) -> wasmtime::Result<Resource<HostFutureIncomingResponse>>
+    dyn Fn(&mut Ctx, OutgoingRequest) -> HttpResult<Resource<HostFutureIncomingResponse>>
         + Send
         + Sync,
 >;
@@ -59,7 +59,7 @@ impl WasiHttpView for Ctx {
     fn send_request(
         &mut self,
         request: OutgoingRequest,
-    ) -> wasmtime::Result<Resource<HostFutureIncomingResponse>> {
+    ) -> HttpResult<Resource<HostFutureIncomingResponse>> {
         if let Some(rejected_authority) = &self.rejected_authority {
             let (auth, _port) = request.authority.split_once(':').unwrap();
             if auth == rejected_authority {

crates/wasi-http/tests/all/sync.rs

@@ -96,3 +96,12 @@ fn http_outbound_request_content_length() -> Result<()> {
     let server = Server::http1()?;
     run(HTTP_OUTBOUND_REQUEST_CONTENT_LENGTH_COMPONENT, &server)
 }
+
+#[test_log::test]
+fn http_outbound_request_missing_path_and_query() -> Result<()> {
+    let server = Server::http1()?;
+    run(
+        HTTP_OUTBOUND_REQUEST_MISSING_PATH_AND_QUERY_COMPONENT,
+        &server,
+    )
+}