From 8b523e780b39a7dc68896194f429a8cd20fe3590 Mon Sep 17 00:00:00 2001 From: Trevor Elliott Date: Fri, 10 Nov 2023 21:56:59 -0800 Subject: [PATCH] wasi-http: Migrate to more descriptive error variant (#7434) * Migrate to a more specific error-code variant in wasi-http Co-authored-by: Pat Hickey * Optional fields, and align with upstream pr * Update for upstream changes to the error-code variant * Sync with the upstream implementation * Missed updating an error for riscv64 and s390x * More debuggable error prtest:full * Try to stabilize the test on windows --------- Co-authored-by: Pat Hickey --- .../src/bin/api_proxy_streaming.rs | 2 +- .../http_outbound_request_invalid_dnsname.rs | 12 +- .../http_outbound_request_invalid_version.rs | 17 ++- .../http_outbound_request_unknown_method.rs | 9 +- ...ttp_outbound_request_unsupported_scheme.rs | 13 +- crates/test-programs/src/http.rs | 9 +- crates/wasi-http/src/body.rs | 28 ++--- crates/wasi-http/src/http_impl.rs | 88 +++++++------ crates/wasi-http/src/lib.rs | 65 +--------- crates/wasi-http/src/types.rs | 74 +++++------ crates/wasi-http/src/types_impl.rs | 21 +++- crates/wasi-http/tests/all/main.rs | 6 +- crates/wasi-http/wit/deps/http/handler.wit | 6 +- crates/wasi-http/wit/deps/http/types.wit | 117 ++++++++++++++---- crates/wasi/wit/deps/http/handler.wit | 6 +- crates/wasi/wit/deps/http/types.wit | 117 ++++++++++++++---- src/commands/serve.rs | 17 ++- 17 files changed, 359 insertions(+), 248 deletions(-) diff --git a/crates/test-programs/src/bin/api_proxy_streaming.rs b/crates/test-programs/src/bin/api_proxy_streaming.rs index 92ad6fb0283c..9c54eb474b49 100644 --- a/crates/test-programs/src/bin/api_proxy_streaming.rs +++ b/crates/test-programs/src/bin/api_proxy_streaming.rs @@ -451,7 +451,7 @@ mod executor { pub fn outgoing_request_send( request: OutgoingRequest, - ) -> impl Future> { + ) -> impl Future> { future::poll_fn({ let response = outgoing_handler::handle(request, None); diff --git a/crates/test-programs/src/bin/http_outbound_request_invalid_dnsname.rs b/crates/test-programs/src/bin/http_outbound_request_invalid_dnsname.rs index 100f57d6d022..0b6deddef755 100644 --- a/crates/test-programs/src/bin/http_outbound_request_invalid_dnsname.rs +++ b/crates/test-programs/src/bin/http_outbound_request_invalid_dnsname.rs @@ -1,4 +1,4 @@ -use test_programs::wasi::http::types::{Method, Scheme}; +use test_programs::wasi::http::types::{ErrorCode, Method, Scheme}; fn main() { let res = test_programs::http::request( @@ -10,9 +10,13 @@ fn main() { None, ); - let error = res.unwrap_err().to_string(); + let e = res.unwrap_err(); assert!( - error.starts_with("Error::InvalidUrl(\""), - "bad error: {error}" + matches!( + e.downcast_ref::() + .expect("expected a wasi-http ErrorCode"), + ErrorCode::DnsError(_) | ErrorCode::ConnectionRefused, + ), + "Unexpected error: {e:#?}" ); } diff --git a/crates/test-programs/src/bin/http_outbound_request_invalid_version.rs b/crates/test-programs/src/bin/http_outbound_request_invalid_version.rs index 59c652fc2b9a..568026859a13 100644 --- a/crates/test-programs/src/bin/http_outbound_request_invalid_version.rs +++ b/crates/test-programs/src/bin/http_outbound_request_invalid_version.rs @@ -1,17 +1,14 @@ -use test_programs::wasi::http::types::{Method, Scheme}; +use test_programs::wasi::http::types::{ErrorCode, Method, Scheme}; fn main() { let addr = std::env::var("HTTP_SERVER").unwrap(); let res = test_programs::http::request(Method::Connect, Scheme::Http, &addr, "/", None, Some(&[])); - let error = res.unwrap_err().to_string(); - if !error.starts_with("Error::ProtocolError(\"") { - panic!( - r#"assertion failed: `(left == right)` - left: `"{error}"`, - right: `"Error::ProtocolError(\"invalid HTTP version parsed\")"` - or `"Error::ProtocolError(\"operation was canceled\")"`)"# - ) - } + assert!(matches!( + res.unwrap_err() + .downcast::() + .expect("expected a wasi-http ErrorCode"), + ErrorCode::HttpProtocolError, + )); } diff --git a/crates/test-programs/src/bin/http_outbound_request_unknown_method.rs b/crates/test-programs/src/bin/http_outbound_request_unknown_method.rs index 6211bf0c8b99..50d5751f5458 100644 --- a/crates/test-programs/src/bin/http_outbound_request_unknown_method.rs +++ b/crates/test-programs/src/bin/http_outbound_request_unknown_method.rs @@ -2,7 +2,7 @@ use test_programs::wasi::http::types::{Method, Scheme}; fn main() { let res = test_programs::http::request( - Method::Other("OTHER".to_owned()), + Method::Other("bad\nmethod".to_owned()), Scheme::Http, "localhost:3000", "/", @@ -10,9 +10,6 @@ fn main() { None, ); - let error = res.unwrap_err(); - assert_eq!( - error.to_string(), - "Error::InvalidUrl(\"unknown method OTHER\")" - ); + // This error arises from input validation in the `set_method` function on `OutgoingRequest`. + assert_eq!(res.unwrap_err().to_string(), "failed to set method"); } diff --git a/crates/test-programs/src/bin/http_outbound_request_unsupported_scheme.rs b/crates/test-programs/src/bin/http_outbound_request_unsupported_scheme.rs index 262cb11e62f0..84ce1a5ba92c 100644 --- a/crates/test-programs/src/bin/http_outbound_request_unsupported_scheme.rs +++ b/crates/test-programs/src/bin/http_outbound_request_unsupported_scheme.rs @@ -1,4 +1,4 @@ -use test_programs::wasi::http::types::{Method, Scheme}; +use test_programs::wasi::http::types::{ErrorCode, Method, Scheme}; fn main() { let res = test_programs::http::request( @@ -10,9 +10,10 @@ fn main() { None, ); - let error = res.unwrap_err(); - assert_eq!( - error.to_string(), - "Error::InvalidUrl(\"unsupported scheme WS\")" - ); + assert!(matches!( + res.unwrap_err() + .downcast::() + .expect("expected a wasi-http ErrorCode"), + ErrorCode::HttpProtocolError, + )); } diff --git a/crates/test-programs/src/http.rs b/crates/test-programs/src/http.rs index 3dbcb6846b84..64a182ac8e09 100644 --- a/crates/test-programs/src/http.rs +++ b/crates/test-programs/src/http.rs @@ -113,19 +113,16 @@ pub fn request( http_types::OutgoingBody::finish(outgoing_body, None); let incoming_response = match future_response.get() { - Some(result) => result.map_err(|_| anyhow!("incoming response errored"))?, + Some(result) => result.map_err(|()| anyhow!("response already taken"))?, None => { let pollable = future_response.subscribe(); pollable.block(); future_response .get() .expect("incoming response available") - .map_err(|_| anyhow!("incoming response errored"))? + .map_err(|()| anyhow!("response already taken"))? } - } - // TODO: maybe anything that appears in the Result<_, E> position should impl - // Error? anyway, just use its Debug here: - .map_err(|e| anyhow!("{e:?}"))?; + }?; drop(future_response); diff --git a/crates/wasi-http/src/body.rs b/crates/wasi-http/src/body.rs index 03cc695bcdfd..c4ada9ef908f 100644 --- a/crates/wasi-http/src/body.rs +++ b/crates/wasi-http/src/body.rs @@ -14,7 +14,7 @@ use wasmtime_wasi::preview2::{ Subscribe, }; -pub type HyperIncomingBody = BoxBody; +pub type HyperIncomingBody = BoxBody; /// Small wrapper around `BoxBody` which adds a timeout to every frame. struct BodyWithTimeout { @@ -45,12 +45,12 @@ impl BodyWithTimeout { impl Body for BodyWithTimeout { type Data = Bytes; - type Error = types::Error; + type Error = types::ErrorCode; fn poll_frame( self: Pin<&mut Self>, cx: &mut Context<'_>, - ) -> Poll, types::Error>>> { + ) -> Poll, types::ErrorCode>>> { let me = Pin::into_inner(self); // If the timeout timer needs to be reset, do that now relative to the @@ -66,9 +66,7 @@ impl Body for BodyWithTimeout { // Register interest in this context on the sleep timer, and if the // sleep elapsed that means that we've timed out. if let Poll::Ready(()) = me.timeout.as_mut().poll(cx) { - return Poll::Ready(Some(Err(types::Error::TimeoutError( - "frame timed out".to_string(), - )))); + return Poll::Ready(Some(Err(types::ErrorCode::ConnectionReadTimeout))); } // Without timeout business now handled check for the frame. If a frame @@ -222,7 +220,7 @@ impl Subscribe for HostIncomingBodyStream { } impl HostIncomingBodyStream { - fn record_frame(&mut self, frame: Option, types::Error>>) { + fn record_frame(&mut self, frame: Option, types::ErrorCode>>) { match frame { Some(Ok(frame)) => match frame.into_data() { // A data frame was received, so queue up the buffered data for @@ -300,7 +298,7 @@ pub enum HostFutureTrailers { /// /// Note that `Ok(None)` means that there were no trailers for this request /// while `Ok(Some(_))` means that trailers were found in the request. - Done(Result, types::Error>), + Done(Result, types::ErrorCode>), } #[async_trait::async_trait] @@ -328,9 +326,7 @@ impl Subscribe for HostFutureTrailers { // this just in case though. Err(_) => { debug_assert!(false, "should be unreachable"); - *self = HostFutureTrailers::Done(Err(types::Error::ProtocolError( - "stream hung up before trailers were received".to_string(), - ))); + *self = HostFutureTrailers::Done(Err(types::ErrorCode::ConnectionTerminated)); } } } @@ -362,7 +358,7 @@ impl Subscribe for HostFutureTrailers { } } -pub type HyperOutgoingBody = BoxBody; +pub type HyperOutgoingBody = BoxBody; pub enum FinishMessage { Finished, @@ -384,7 +380,7 @@ impl HostOutgoingBody { } impl Body for BodyImpl { type Data = Bytes; - type Error = types::Error; + type Error = types::ErrorCode; fn poll_frame( mut self: Pin<&mut Self>, cx: &mut Context<'_>, @@ -406,9 +402,9 @@ impl HostOutgoingBody { FinishMessage::Trailers(trailers) => { Poll::Ready(Some(Ok(Frame::trailers(trailers)))) } - FinishMessage::Abort => Poll::Ready(Some(Err( - types::Error::ProtocolError("response corrupted".into()), - ))), + FinishMessage::Abort => { + Poll::Ready(Some(Err(types::ErrorCode::HttpProtocolError))) + } }, Poll::Ready(Err(RecvError { .. })) => Poll::Ready(None), } diff --git a/crates/wasi-http/src/http_impl.rs b/crates/wasi-http/src/http_impl.rs index 47ff08930647..040348a51226 100644 --- a/crates/wasi-http/src/http_impl.rs +++ b/crates/wasi-http/src/http_impl.rs @@ -1,22 +1,22 @@ -use crate::bindings::http::{ - outgoing_handler, - types::{self as http_types, Scheme}, +use crate::{ + bindings::http::{ + outgoing_handler, + types::{self, Scheme}, + }, + types::{HostFutureIncomingResponse, HostOutgoingRequest, OutgoingRequest}, + WasiHttpView, }; -use crate::types::{self, HostFutureIncomingResponse, OutgoingRequest}; -use crate::WasiHttpView; use bytes::Bytes; use http_body_util::{BodyExt, Empty}; use hyper::Method; -use types::HostOutgoingRequest; use wasmtime::component::Resource; impl outgoing_handler::Host for T { fn handle( &mut self, request_id: Resource, - options: Option>, - ) -> wasmtime::Result, outgoing_handler::Error>> - { + options: Option>, + ) -> wasmtime::Result, types::ErrorCode>> { let opts = options.and_then(|opts| self.table().get(&opts).ok()); let connect_timeout = opts @@ -32,32 +32,30 @@ impl outgoing_handler::Host for T { .unwrap_or(std::time::Duration::from_millis(600 * 1000)); let req = self.table().delete(request_id)?; + let mut builder = hyper::Request::builder(); - let method = match req.method { - crate::bindings::http::types::Method::Get => Method::GET, - crate::bindings::http::types::Method::Head => Method::HEAD, - crate::bindings::http::types::Method::Post => Method::POST, - crate::bindings::http::types::Method::Put => Method::PUT, - crate::bindings::http::types::Method::Delete => Method::DELETE, - crate::bindings::http::types::Method::Connect => Method::CONNECT, - crate::bindings::http::types::Method::Options => Method::OPTIONS, - crate::bindings::http::types::Method::Trace => Method::TRACE, - crate::bindings::http::types::Method::Patch => Method::PATCH, - crate::bindings::http::types::Method::Other(method) => { - return Ok(Err(outgoing_handler::Error::InvalidUrl(format!( - "unknown method {method}" - )))); - } - }; + builder = builder.method(match req.method { + types::Method::Get => Method::GET, + types::Method::Head => Method::HEAD, + types::Method::Post => Method::POST, + types::Method::Put => Method::PUT, + types::Method::Delete => Method::DELETE, + types::Method::Connect => Method::CONNECT, + types::Method::Options => Method::OPTIONS, + types::Method::Trace => Method::TRACE, + 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)), + }, + }); let (use_tls, scheme, port) = match req.scheme.unwrap_or(Scheme::Https) { - Scheme::Http => (false, "http://", 80), - Scheme::Https => (true, "https://", 443), - Scheme::Other(scheme) => { - return Ok(Err(outgoing_handler::Error::InvalidUrl(format!( - "unsupported scheme {scheme}" - )))) - } + Scheme::Http => (false, http::uri::Scheme::HTTP, 80), + Scheme::Https => (true, http::uri::Scheme::HTTPS, 443), + + // We can only support http/https + Scheme::Other(_) => return Ok(Err(types::ErrorCode::HttpProtocolError)), }; let authority = if let Some(authority) = req.authority { @@ -69,14 +67,20 @@ impl outgoing_handler::Host for T { } else { String::new() }; + builder = builder.header(hyper::header::HOST, &authority); + + let mut uri = http::Uri::builder() + .scheme(scheme) + .authority(authority.clone()); + + if let Some(path) = req.path_with_query { + uri = uri.path_and_query(path); + } - let mut builder = hyper::Request::builder() - .method(method) - .uri(format!( - "{scheme}{authority}{}", - req.path_with_query.as_ref().map_or("", String::as_ref) - )) - .header(hyper::header::HOST, &authority); + builder = builder.uri(uri.build().map_err(|e| { + eprintln!("uri build error: {e:#?}"); + types::ErrorCode::HttpRequestUriInvalid + })?); for (k, v) in req.headers.iter() { builder = builder.header(k, v); @@ -84,9 +88,11 @@ impl outgoing_handler::Host for T { let body = req .body - .unwrap_or_else(|| Empty::::new().map_err(|_| unreachable!()).boxed()); + .unwrap_or_else(|| Empty::::new().map_err(|_| todo!("thing")).boxed()); - let request = builder.body(body).map_err(types::http_protocol_error)?; + let request = builder + .body(body) + .map_err(|err| types::ErrorCode::InternalError(Some(err.to_string())))?; Ok(Ok(self.send_request(OutgoingRequest { use_tls, diff --git a/crates/wasi-http/src/lib.rs b/crates/wasi-http/src/lib.rs index 936c48a1f4db..486c6b776d96 100644 --- a/crates/wasi-http/src/lib.rs +++ b/crates/wasi-http/src/lib.rs @@ -37,64 +37,9 @@ pub mod bindings { pub use wasi::http; } -impl From for crate::bindings::http::types::Error { - fn from(err: wasmtime_wasi::preview2::TableError) -> Self { - Self::UnexpectedError(err.to_string()) - } -} - -impl From for crate::bindings::http::types::Error { - fn from(err: anyhow::Error) -> Self { - Self::UnexpectedError(err.to_string()) - } -} - -impl From for crate::bindings::http::types::Error { - fn from(err: std::io::Error) -> Self { - let message = err.to_string(); - match err.kind() { - std::io::ErrorKind::InvalidInput => Self::InvalidUrl(message), - std::io::ErrorKind::AddrNotAvailable => Self::InvalidUrl(message), - _ => { - if message.starts_with("failed to lookup address information") { - Self::InvalidUrl("invalid dnsname".to_string()) - } else { - Self::ProtocolError(message) - } - } - } - } -} - -impl From for crate::bindings::http::types::Error { - fn from(err: hyper::Error) -> Self { - let message = err.message().to_string(); - if err.is_timeout() { - Self::TimeoutError(message) - } else if err.is_parse_status() || err.is_user() { - Self::InvalidUrl(message) - } else if err.is_body_write_aborted() - || err.is_canceled() - || err.is_closed() - || err.is_incomplete_message() - || err.is_parse() - { - Self::ProtocolError(message) - } else { - Self::UnexpectedError(message) - } - } -} - -impl From for crate::bindings::http::types::Error { - fn from(err: tokio::time::error::Elapsed) -> Self { - Self::TimeoutError(err.to_string()) - } -} - -#[cfg(not(any(target_arch = "riscv64", target_arch = "s390x")))] -impl From for crate::bindings::http::types::Error { - fn from(_err: rustls::client::InvalidDnsNameError) -> Self { - Self::InvalidUrl("invalid dnsname".to_string()) - } +pub(crate) fn dns_error(rcode: String, info_code: u16) -> bindings::http::types::ErrorCode { + bindings::http::types::ErrorCode::DnsError(bindings::http::types::DnsErrorPayload { + rcode: Some(rcode), + info_code: Some(info_code), + }) } diff --git a/crates/wasi-http/src/types.rs b/crates/wasi-http/src/types.rs index c7041fcad5c0..a5bc098dada1 100644 --- a/crates/wasi-http/src/types.rs +++ b/crates/wasi-http/src/types.rs @@ -4,6 +4,7 @@ use crate::{ bindings::http::types::{self, Method, Scheme}, body::{HostIncomingBody, HyperIncomingBody, HyperOutgoingBody}, + dns_error, }; use http_body_util::BodyExt; use hyper::header::HeaderName; @@ -50,7 +51,7 @@ pub trait WasiHttpView: Send { fn new_response_outparam( &mut self, result: tokio::sync::oneshot::Sender< - Result, types::Error>, + Result, types::ErrorCode>, >, ) -> wasmtime::Result> { let id = self.table().push(HostResponseOutparam { result })?; @@ -108,16 +109,30 @@ async fn handler( first_byte_timeout: Duration, request: http::Request, between_bytes_timeout: Duration, -) -> Result { +) -> Result { let tcp_stream = TcpStream::connect(authority.clone()) .await - .map_err(invalid_url)?; + .map_err(|e| match e.kind() { + std::io::ErrorKind::AddrNotAvailable => { + dns_error("address not available".to_string(), 0) + } + + _ => { + if e.to_string() + .starts_with("failed to lookup address information") + { + dns_error("address not available".to_string(), 0) + } else { + types::ErrorCode::ConnectionRefused + } + } + })?; let (mut sender, worker) = if use_tls { #[cfg(any(target_arch = "riscv64", target_arch = "s390x"))] { - return Err(crate::bindings::http::types::Error::UnexpectedError( - "unsupported architecture for SSL".to_string(), + return Err(crate::bindings::http::types::ErrorCode::InternalError( + Some("unsupported architecture for SSL".to_string()), )); } @@ -141,18 +156,20 @@ async fn handler( let connector = tokio_rustls::TlsConnector::from(std::sync::Arc::new(config)); let mut parts = authority.split(":"); let host = parts.next().unwrap_or(&authority); - let domain = rustls::ServerName::try_from(host)?; + let domain = rustls::ServerName::try_from(host) + .map_err(|_| dns_error("invalid dns name".to_string(), 0))?; let stream = connector .connect(domain, tcp_stream) .await - .map_err(|e| crate::bindings::http::types::Error::ProtocolError(e.to_string()))?; + .map_err(|_| types::ErrorCode::TlsProtocolError)?; let (sender, conn) = timeout( connect_timeout, hyper::client::conn::http1::handshake(stream), ) .await - .map_err(|_| timeout_error("connection"))??; + .map_err(|_| types::ErrorCode::ConnectionTimeout)? + .map_err(|_| types::ErrorCode::ConnectionTimeout)?; let worker = preview2::spawn(async move { match conn.await { @@ -172,7 +189,8 @@ async fn handler( hyper::client::conn::http1::handshake(tcp_stream), ) .await - .map_err(|_| timeout_error("connection"))??; + .map_err(|_| types::ErrorCode::ConnectionTimeout)? + .map_err(|_| types::ErrorCode::HttpProtocolError)?; let worker = preview2::spawn(async move { match conn.await { @@ -187,9 +205,12 @@ async fn handler( let resp = timeout(first_byte_timeout, sender.send_request(request)) .await - .map_err(|_| timeout_error("first byte"))? - .map_err(hyper_protocol_error)? - .map(|body| body.map_err(|e| e.into()).boxed()); + .map_err(|_| types::ErrorCode::ConnectionReadTimeout)? + .map_err(|_| types::ErrorCode::HttpProtocolError)? + .map(|body| { + body.map_err(|_| types::ErrorCode::HttpProtocolError) + .boxed() + }); Ok(IncomingResponseInternal { resp, @@ -197,25 +218,6 @@ async fn handler( between_bytes_timeout, }) } - -pub fn timeout_error(kind: &str) -> types::Error { - types::Error::TimeoutError(format!("{kind} timed out")) -} - -pub fn http_protocol_error(e: http::Error) -> types::Error { - types::Error::ProtocolError(e.to_string()) -} - -pub fn hyper_protocol_error(e: hyper::Error) -> types::Error { - types::Error::ProtocolError(e.to_string()) -} - -fn invalid_url(e: std::io::Error) -> types::Error { - // TODO: DNS errors show up as a Custom io error, what subset of errors should we consider for - // InvalidUrl here? - types::Error::InvalidUrl(e.to_string()) -} - impl From for types::Method { fn from(method: http::Method) -> Self { if method == http::Method::GET { @@ -268,7 +270,7 @@ pub struct HostIncomingRequest { pub struct HostResponseOutparam { pub result: - tokio::sync::oneshot::Sender, types::Error>>, + tokio::sync::oneshot::Sender, types::ErrorCode>>, } pub struct HostOutgoingRequest { @@ -347,11 +349,11 @@ pub struct IncomingResponseInternal { } type FutureIncomingResponseHandle = - AbortOnDropJoinHandle>>; + AbortOnDropJoinHandle>>; pub enum HostFutureIncomingResponse { Pending(FutureIncomingResponseHandle), - Ready(anyhow::Result>), + Ready(anyhow::Result>), Consumed, } @@ -364,7 +366,9 @@ impl HostFutureIncomingResponse { matches!(self, Self::Ready(_)) } - pub fn unwrap_ready(self) -> anyhow::Result> { + pub fn unwrap_ready( + self, + ) -> anyhow::Result> { match self { Self::Ready(res) => res, Self::Pending(_) | Self::Consumed => { diff --git a/crates/wasi-http/src/types_impl.rs b/crates/wasi-http/src/types_impl.rs index 00400a07c860..cf35b5b3b21f 100644 --- a/crates/wasi-http/src/types_impl.rs +++ b/crates/wasi-http/src/types_impl.rs @@ -1,5 +1,5 @@ use crate::{ - bindings::http::types::{self, Error, Headers, Method, Scheme, StatusCode, Trailers}, + bindings::http::types::{self, Headers, Method, Scheme, StatusCode, Trailers}, body::{FinishMessage, HostFutureTrailers, HostIncomingBody, HostOutgoingBody}, types::{ FieldMap, HostFields, HostFutureIncomingResponse, HostIncomingRequest, @@ -17,7 +17,14 @@ use wasmtime_wasi::preview2::{ Pollable, Table, }; -impl crate::bindings::http::types::Host for T {} +impl crate::bindings::http::types::Host for T { + fn http_error_code( + &mut self, + _err: wasmtime::component::Resource, + ) -> wasmtime::Result> { + todo!() + } +} /// Take ownership of the underlying [`FieldMap`] associated with this fields resource. If the /// fields resource references another fields, the returned [`FieldMap`] will be cloned. @@ -525,7 +532,7 @@ impl crate::bindings::http::types::HostResponseOutparam for T { fn set( &mut self, id: Resource, - resp: Result, Error>, + resp: Result, types::ErrorCode>, ) -> wasmtime::Result<()> { let val = match resp { Ok(resp) => Ok(self.table().delete(resp)?.try_into()?), @@ -620,7 +627,7 @@ impl crate::bindings::http::types::HostFutureTrailers for T { fn get( &mut self, id: Resource, - ) -> wasmtime::Result>, Error>>> { + ) -> wasmtime::Result>, types::ErrorCode>>> { let trailers = self.table().get_mut(&id)?; match trailers { HostFutureTrailers::Waiting(_) => return Ok(None), @@ -773,7 +780,9 @@ impl crate::bindings::http::types::HostFutureIncomingResponse f fn get( &mut self, id: Resource, - ) -> wasmtime::Result, Error>, ()>>> { + ) -> wasmtime::Result< + Option, types::ErrorCode>, ()>>, + > { let resp = self.table().get_mut(&id)?; match resp { @@ -786,7 +795,7 @@ impl crate::bindings::http::types::HostFutureIncomingResponse f match std::mem::replace(resp, HostFutureIncomingResponse::Consumed).unwrap_ready() { Err(e) => { // Trapping if it's not possible to downcast to an wasi-http error - let e = e.downcast::()?; + let e = e.downcast::()?; return Ok(Some(Ok(Err(e)))); } diff --git a/crates/wasi-http/tests/all/main.rs b/crates/wasi-http/tests/all/main.rs index 45094d52e621..323d41892feb 100644 --- a/crates/wasi-http/tests/all/main.rs +++ b/crates/wasi-http/tests/all/main.rs @@ -15,7 +15,7 @@ use wasmtime_wasi::preview2::{ self, pipe::MemoryOutputPipe, Table, WasiCtx, WasiCtxBuilder, WasiView, }; use wasmtime_wasi_http::{ - bindings::http::types::Error, + bindings::http::types::ErrorCode, body::HyperIncomingBody, types::{self, HostFutureIncomingResponse, IncomingResponseInternal, OutgoingRequest}, WasiHttpCtx, WasiHttpView, @@ -128,7 +128,7 @@ async fn run_wasi_http( component_filename: &str, req: hyper::Request, send_request: Option, -) -> anyhow::Result>, Error>> { +) -> anyhow::Result>, ErrorCode>> { let stdout = MemoryOutputPipe::new(4096); let stderr = MemoryOutputPipe::new(4096); let table = Table::new(); @@ -457,7 +457,7 @@ async fn do_wasi_http_echo(uri: &str, url_header: Option<&str>) -> Result<()> { let request = request.body(BoxBody::new(StreamBody::new(stream::iter( body.chunks(16 * 1024) - .map(|chunk| Ok::<_, Error>(Frame::data(Bytes::copy_from_slice(chunk)))) + .map(|chunk| Ok::<_, ErrorCode>(Frame::data(Bytes::copy_from_slice(chunk)))) .collect::>(), ))))?; diff --git a/crates/wasi-http/wit/deps/http/handler.wit b/crates/wasi-http/wit/deps/http/handler.wit index 21b97a354cc7..a34a0649d5b0 100644 --- a/crates/wasi-http/wit/deps/http/handler.wit +++ b/crates/wasi-http/wit/deps/http/handler.wit @@ -22,7 +22,9 @@ interface incoming-handler { /// This interface defines a handler of outgoing HTTP Requests. It should be /// imported by components which wish to make HTTP Requests. interface outgoing-handler { - use types.{outgoing-request, request-options, future-incoming-response, error}; + use types.{ + outgoing-request, request-options, future-incoming-response, error-code + }; /// This function is invoked with an outgoing HTTP Request, and it returns /// a resource `future-incoming-response` which represents an HTTP Response @@ -37,5 +39,5 @@ interface outgoing-handler { handle: func( request: outgoing-request, options: option - ) -> result; + ) -> result; } diff --git a/crates/wasi-http/wit/deps/http/types.wit b/crates/wasi-http/wit/deps/http/types.wit index 4662f3fd6ba8..41226894eb2c 100644 --- a/crates/wasi-http/wit/deps/http/types.wit +++ b/crates/wasi-http/wit/deps/http/types.wit @@ -3,7 +3,7 @@ /// their headers, trailers, and bodies. interface types { use wasi:clocks/monotonic-clock@0.2.0-rc-2023-11-05.{duration}; - use wasi:io/streams@0.2.0-rc-2023-11-05.{input-stream, output-stream}; + use wasi:io/streams@0.2.0-rc-2023-11-05.{input-stream, output-stream, error as stream-error}; use wasi:io/poll@0.2.0-rc-2023-11-05.{pollable}; /// This type corresponds to HTTP standard Methods. @@ -27,22 +27,100 @@ interface types { other(string) } - /// TODO: perhaps better align with HTTP semantics? - /// This type enumerates the different kinds of errors that may occur when - /// initially returning a response. - variant error { - invalid-url(string), - timeout-error(string), - protocol-error(string), - unexpected-error(string) + /// These cases are inspired by the IANA HTTP Proxy Error Types: + /// https://www.iana.org/assignments/http-proxy-status/http-proxy-status.xhtml#table-http-proxy-error-types + variant error-code { + DNS-timeout, + DNS-error(DNS-error-payload), + destination-not-found, + destination-unavailable, + destination-IP-prohibited, + destination-IP-unroutable, + connection-refused, + connection-terminated, + connection-timeout, + connection-read-timeout, + connection-write-timeout, + connection-limit-reached, + TLS-protocol-error, + TLS-certificate-error, + TLS-alert-received(TLS-alert-received-payload), + HTTP-request-denied, + HTTP-request-length-required, + HTTP-request-body-size(option), + HTTP-request-method-invalid, + HTTP-request-URI-invalid, + HTTP-request-URI-too-long, + HTTP-request-header-section-size(option), + HTTP-request-header-size(option), + HTTP-request-trailer-section-size(option), + HTTP-request-trailer-size(field-size-payload), + HTTP-response-incomplete, + HTTP-response-header-section-size(option), + HTTP-response-header-size(field-size-payload), + HTTP-response-body-size(option), + HTTP-response-trailer-section-size(option), + HTTP-response-trailer-size(field-size-payload), + HTTP-response-transfer-coding(option), + HTTP-response-content-coding(option), + HTTP-response-timeout, + HTTP-upgrade-failed, + HTTP-protocol-error, + loop-detected, + configuration-error, + /// This is a catch-all error for anything that doesn't fit cleanly into a + /// more specific case. It also includes an optional string for an + /// unstructured description of the error. Users should not depend on the + /// string for diagnosing errors, as it's not required to be consistent + /// between implementations. + internal-error(option) + } + + /// Defines the case payload type for `DNS-error` above: + record DNS-error-payload { + rcode: option, + info-code: option + } + + /// Defines the case payload type for `TLS-alert-received` above: + record TLS-alert-received-payload { + alert-id: option, + alert-message: option + } + + /// Defines the case payload type for `HTTP-response-{header,trailer}-size` above: + record field-size-payload { + field-name: option, + field-size: option } - /// This tyep enumerates the different kinds of errors that may occur when + /// Attempts to extract a http-related `error` from the stream `error` + /// provided. + /// + /// Stream operations which return `stream-error::last-operation-failed` have + /// a payload with more information about the operation that failed. This + /// payload can be passed through to this function to see if there's + /// http-related information about the error to return. + /// + /// Note that this function is fallible because not all stream-related errors + /// are http-related errors. + http-error-code: func(err: borrow) -> option; + + /// This type enumerates the different kinds of errors that may occur when /// setting or appending to a `fields` resource. variant header-error { + /// This error indicates that a `field-key` or `field-value` was + /// syntactically invalid when used with an operation that sets headers in a + /// `fields`. invalid-syntax, + + /// This error indicates that a forbidden `field-key` was used when trying + /// to set a header in a `fields`. forbidden, - immutable + + /// This error indicates that the operation on the `fields` was not + /// permitted because the fields are immutable. + immutable, } /// Field keys are always strings. @@ -83,21 +161,14 @@ interface types { /// Set all of the values for a key. Clears any existing values for that /// key, if they have been set. - /// - /// The operation can fail if the name or value arguments are invalid, or if - /// the name is forbidden. set: func(name: field-key, value: list) -> result<_, header-error>; /// Delete all values for a key. Does nothing if no values for the key - /// exist. Returns and error if the `field-key` is syntactically invalid, or - /// if the `field-key` is forbidden. + /// exist. delete: func(name: field-key) -> result<_, header-error>; /// Append a value for a key. Does not change or delete any existing /// values for that key. - /// - /// The operation can fail if the name or value arguments are invalid, or if - /// the name is forbidden. append: func(name: field-key, value: field-value) -> result<_, header-error>; /// Retrieve the full set of keys and values in the Fields. Like the @@ -150,7 +221,7 @@ interface types { resource outgoing-request { /// Construct a new `outgoing-request` with a default `method` of `GET`, and - /// default values for `path-with-query`, `scheme`, and `authority. + /// `none` values for `path-with-query`, `scheme`, and `authority`. /// /// * `headers` is the HTTP Headers for the Request. /// @@ -263,7 +334,7 @@ interface types { /// implementation determine how to respond with an HTTP error response. set: static func( param: response-outparam, - response: result, + response: result, ); } @@ -338,7 +409,7 @@ interface types { /// as well as any trailers, were received successfully, or that an error /// occured receiving them. The optional `trailers` indicates whether or not /// trailers were present in the body. - get: func() -> option, error>>; + get: func() -> option, error-code>>; } /// Represents an outgoing HTTP Response. @@ -434,7 +505,7 @@ interface types { /// occured. Errors may also occur while consuming the response body, /// but those will be reported by the `incoming-body` and its /// `output-stream` child. - get: func() -> option>>; + get: func() -> option>>; } } diff --git a/crates/wasi/wit/deps/http/handler.wit b/crates/wasi/wit/deps/http/handler.wit index 21b97a354cc7..a34a0649d5b0 100644 --- a/crates/wasi/wit/deps/http/handler.wit +++ b/crates/wasi/wit/deps/http/handler.wit @@ -22,7 +22,9 @@ interface incoming-handler { /// This interface defines a handler of outgoing HTTP Requests. It should be /// imported by components which wish to make HTTP Requests. interface outgoing-handler { - use types.{outgoing-request, request-options, future-incoming-response, error}; + use types.{ + outgoing-request, request-options, future-incoming-response, error-code + }; /// This function is invoked with an outgoing HTTP Request, and it returns /// a resource `future-incoming-response` which represents an HTTP Response @@ -37,5 +39,5 @@ interface outgoing-handler { handle: func( request: outgoing-request, options: option - ) -> result; + ) -> result; } diff --git a/crates/wasi/wit/deps/http/types.wit b/crates/wasi/wit/deps/http/types.wit index 4662f3fd6ba8..41226894eb2c 100644 --- a/crates/wasi/wit/deps/http/types.wit +++ b/crates/wasi/wit/deps/http/types.wit @@ -3,7 +3,7 @@ /// their headers, trailers, and bodies. interface types { use wasi:clocks/monotonic-clock@0.2.0-rc-2023-11-05.{duration}; - use wasi:io/streams@0.2.0-rc-2023-11-05.{input-stream, output-stream}; + use wasi:io/streams@0.2.0-rc-2023-11-05.{input-stream, output-stream, error as stream-error}; use wasi:io/poll@0.2.0-rc-2023-11-05.{pollable}; /// This type corresponds to HTTP standard Methods. @@ -27,22 +27,100 @@ interface types { other(string) } - /// TODO: perhaps better align with HTTP semantics? - /// This type enumerates the different kinds of errors that may occur when - /// initially returning a response. - variant error { - invalid-url(string), - timeout-error(string), - protocol-error(string), - unexpected-error(string) + /// These cases are inspired by the IANA HTTP Proxy Error Types: + /// https://www.iana.org/assignments/http-proxy-status/http-proxy-status.xhtml#table-http-proxy-error-types + variant error-code { + DNS-timeout, + DNS-error(DNS-error-payload), + destination-not-found, + destination-unavailable, + destination-IP-prohibited, + destination-IP-unroutable, + connection-refused, + connection-terminated, + connection-timeout, + connection-read-timeout, + connection-write-timeout, + connection-limit-reached, + TLS-protocol-error, + TLS-certificate-error, + TLS-alert-received(TLS-alert-received-payload), + HTTP-request-denied, + HTTP-request-length-required, + HTTP-request-body-size(option), + HTTP-request-method-invalid, + HTTP-request-URI-invalid, + HTTP-request-URI-too-long, + HTTP-request-header-section-size(option), + HTTP-request-header-size(option), + HTTP-request-trailer-section-size(option), + HTTP-request-trailer-size(field-size-payload), + HTTP-response-incomplete, + HTTP-response-header-section-size(option), + HTTP-response-header-size(field-size-payload), + HTTP-response-body-size(option), + HTTP-response-trailer-section-size(option), + HTTP-response-trailer-size(field-size-payload), + HTTP-response-transfer-coding(option), + HTTP-response-content-coding(option), + HTTP-response-timeout, + HTTP-upgrade-failed, + HTTP-protocol-error, + loop-detected, + configuration-error, + /// This is a catch-all error for anything that doesn't fit cleanly into a + /// more specific case. It also includes an optional string for an + /// unstructured description of the error. Users should not depend on the + /// string for diagnosing errors, as it's not required to be consistent + /// between implementations. + internal-error(option) + } + + /// Defines the case payload type for `DNS-error` above: + record DNS-error-payload { + rcode: option, + info-code: option + } + + /// Defines the case payload type for `TLS-alert-received` above: + record TLS-alert-received-payload { + alert-id: option, + alert-message: option + } + + /// Defines the case payload type for `HTTP-response-{header,trailer}-size` above: + record field-size-payload { + field-name: option, + field-size: option } - /// This tyep enumerates the different kinds of errors that may occur when + /// Attempts to extract a http-related `error` from the stream `error` + /// provided. + /// + /// Stream operations which return `stream-error::last-operation-failed` have + /// a payload with more information about the operation that failed. This + /// payload can be passed through to this function to see if there's + /// http-related information about the error to return. + /// + /// Note that this function is fallible because not all stream-related errors + /// are http-related errors. + http-error-code: func(err: borrow) -> option; + + /// This type enumerates the different kinds of errors that may occur when /// setting or appending to a `fields` resource. variant header-error { + /// This error indicates that a `field-key` or `field-value` was + /// syntactically invalid when used with an operation that sets headers in a + /// `fields`. invalid-syntax, + + /// This error indicates that a forbidden `field-key` was used when trying + /// to set a header in a `fields`. forbidden, - immutable + + /// This error indicates that the operation on the `fields` was not + /// permitted because the fields are immutable. + immutable, } /// Field keys are always strings. @@ -83,21 +161,14 @@ interface types { /// Set all of the values for a key. Clears any existing values for that /// key, if they have been set. - /// - /// The operation can fail if the name or value arguments are invalid, or if - /// the name is forbidden. set: func(name: field-key, value: list) -> result<_, header-error>; /// Delete all values for a key. Does nothing if no values for the key - /// exist. Returns and error if the `field-key` is syntactically invalid, or - /// if the `field-key` is forbidden. + /// exist. delete: func(name: field-key) -> result<_, header-error>; /// Append a value for a key. Does not change or delete any existing /// values for that key. - /// - /// The operation can fail if the name or value arguments are invalid, or if - /// the name is forbidden. append: func(name: field-key, value: field-value) -> result<_, header-error>; /// Retrieve the full set of keys and values in the Fields. Like the @@ -150,7 +221,7 @@ interface types { resource outgoing-request { /// Construct a new `outgoing-request` with a default `method` of `GET`, and - /// default values for `path-with-query`, `scheme`, and `authority. + /// `none` values for `path-with-query`, `scheme`, and `authority`. /// /// * `headers` is the HTTP Headers for the Request. /// @@ -263,7 +334,7 @@ interface types { /// implementation determine how to respond with an HTTP error response. set: static func( param: response-outparam, - response: result, + response: result, ); } @@ -338,7 +409,7 @@ interface types { /// as well as any trailers, were received successfully, or that an error /// occured receiving them. The optional `trailers` indicates whether or not /// trailers were present in the body. - get: func() -> option, error>>; + get: func() -> option, error-code>>; } /// Represents an outgoing HTTP Response. @@ -434,7 +505,7 @@ interface types { /// occured. Errors may also occur while consuming the response body, /// but those will be reported by the `incoming-body` and its /// `output-stream` child. - get: func() -> option>>; + get: func() -> option>>; } } diff --git a/src/commands/serve.rs b/src/commands/serve.rs index 927cc755b728..5aad87decdbb 100644 --- a/src/commands/serve.rs +++ b/src/commands/serve.rs @@ -14,7 +14,9 @@ use wasmtime::{Engine, Store, StoreLimits}; use wasmtime_wasi::preview2::{ self, StreamError, StreamResult, Table, WasiCtx, WasiCtxBuilder, WasiView, }; -use wasmtime_wasi_http::{body::HyperOutgoingBody, WasiHttpCtx, WasiHttpView}; +use wasmtime_wasi_http::{ + bindings::http::types as http_types, body::HyperOutgoingBody, WasiHttpCtx, WasiHttpView, +}; #[cfg(feature = "wasi-nn")] use wasmtime_wasi_nn::WasiNnCtx; @@ -363,9 +365,16 @@ impl hyper::service::Service for ProxyHandler { let mut store = inner.cmd.new_store(&inner.engine, req_id)?; - let req = store - .data_mut() - .new_incoming_request(req.map(|body| body.map_err(|e| e.into()).boxed()))?; + let req = store.data_mut().new_incoming_request(req.map(|body| { + body.map_err(|err| { + if err.is_timeout() { + http_types::ErrorCode::HttpResponseTimeout + } else { + http_types::ErrorCode::InternalError(Some(err.message().to_string())) + } + }) + .boxed() + }))?; let out = store.data_mut().new_response_outparam(sender)?;