From 9dd8d53b127b3c7fa9afa102705fe3c517b0aeac Mon Sep 17 00:00:00 2001 From: Trevor Elliott Date: Tue, 11 Jun 2024 13:22:31 -0700 Subject: [PATCH] Fix trap handling, and run trap-test with components --- cli/tests/trap-test/src/main.rs | 14 +++++-- lib/src/component/http_req.rs | 3 +- lib/src/component/http_resp.rs | 65 +++++++++++++++++++-------------- lib/src/component/mod.rs | 8 ++++ lib/src/component/types.rs | 44 +++++++++++++++++++++- lib/src/execute.rs | 13 +++++-- 6 files changed, 110 insertions(+), 37 deletions(-) diff --git a/cli/tests/trap-test/src/main.rs b/cli/tests/trap-test/src/main.rs index d8ab745b..566aac4f 100644 --- a/cli/tests/trap-test/src/main.rs +++ b/cli/tests/trap-test/src/main.rs @@ -13,10 +13,8 @@ pub type Error = Box; /// Handy alias for the return type of async Tokio tests pub type TestResult = Result<(), Error>; -#[tokio::test(flavor = "multi_thread")] -async fn fatal_error_traps() -> TestResult { +async fn fatal_error_traps_impl(adapt_core_wasm: bool) -> TestResult { let module_path = format!("{RUST_FIXTURE_PATH}/response.wasm"); - let adapt_core_wasm = false; let ctx = ExecuteCtx::new( module_path, ProfilingStrategy::None, @@ -45,3 +43,13 @@ async fn fatal_error_traps() -> TestResult { Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn fatal_error_traps() -> TestResult { + fatal_error_traps_impl(false).await +} + +#[tokio::test(flavor = "multi_thread")] +async fn fatal_error_traps_component() -> TestResult { + fatal_error_traps_impl(true).await +} diff --git a/lib/src/component/http_req.rs b/lib/src/component/http_req.rs index 16c2f7ad..9a71556d 100644 --- a/lib/src/component/http_req.rs +++ b/lib/src/component/http_req.rs @@ -2,6 +2,7 @@ use { super::{ fastly::api::{http_req, http_types, types}, headers::write_values, + types::TrappableError, }, crate::{ config::{Backend, ClientCertInfo}, @@ -223,7 +224,7 @@ impl http_req::Host for Session { name: String, max_len: u64, cursor: u32, - ) -> Result, Option)>, types::Error> { + ) -> Result, Option)>, TrappableError> { let headers = &self.request_parts(h.into())?.headers; let values = headers.get_all(HeaderName::from_str(&name)?); diff --git a/lib/src/component/http_resp.rs b/lib/src/component/http_resp.rs index 12d7c7f6..c26e5b8b 100644 --- a/lib/src/component/http_resp.rs +++ b/lib/src/component/http_resp.rs @@ -1,7 +1,8 @@ use { super::fastly::api::{http_resp, http_types, types}, - super::headers::write_values, + super::{headers::write_values, types::TrappableError}, crate::{error::Error, session::Session}, + cfg_if::cfg_if, http::{HeaderName, HeaderValue}, hyper::http::response::Response, std::str::FromStr, @@ -79,6 +80,8 @@ impl http_resp::Host for Session { max_len: u64, cursor: u32, ) -> Result, Option)>, types::Error> { + {} + let headers = &self.response_parts(h.into())?.headers; let (buf, next) = write_values( @@ -134,36 +137,44 @@ impl http_resp::Host for Session { name: String, max_len: u64, cursor: u32, - ) -> Result, Option)>, types::Error> { - if name.len() > MAX_HEADER_NAME_LEN { - return Err(Error::InvalidArgument.into()); - } - - let headers = &self.response_parts(h.into())?.headers; - - let values = headers.get_all(HeaderName::from_str(&name)?); - - let (buf, next) = write_values( - values.into_iter(), - b'\0', - usize::try_from(max_len).unwrap(), - cursor, - ); - - if buf.is_empty() { - if next.is_none() { - return Ok(None); + ) -> Result, Option)>, TrappableError> { + cfg_if! { + if #[cfg(feature = "test-fatalerror-config")] { + // Avoid warnings: + let _ = (h, name, max_len, cursor); + return Err(Error::FatalError("A fatal error occurred in the test-only implementation of header_values_get".to_string()).into()); } else { - // It's an error if we couldn't write even a single value. - return Err(Error::BufferLengthError { - buf: "buf", - len: "buf.len()", + if name.len() > MAX_HEADER_NAME_LEN { + return Err(Error::InvalidArgument.into()); } - .into()); + + let headers = &self.response_parts(h.into())?.headers; + + let values = headers.get_all(HeaderName::from_str(&name)?); + + let (buf, next) = write_values( + values.into_iter(), + b'\0', + usize::try_from(max_len).unwrap(), + cursor, + ); + + if buf.is_empty() { + if next.is_none() { + return Ok(None); + } else { + // It's an error if we couldn't write even a single value. + return Err(Error::BufferLengthError { + buf: "buf", + len: "buf.len()", + } + .into()); + } + } + + Ok(Some((buf, next))) } } - - Ok(Some((buf, next))) } async fn header_values_set( diff --git a/lib/src/component/mod.rs b/lib/src/component/mod.rs index 9991dd52..dde1233b 100644 --- a/lib/src/component/mod.rs +++ b/lib/src/component/mod.rs @@ -12,6 +12,14 @@ component::bindgen!({ "wasi:io": wasmtime_wasi::bindings::io, "wasi:cli": wasmtime_wasi::bindings::cli, }, + + trappable_error_type: { + "fastly:api/types/error" => types::TrappableError, + }, + + trappable_imports: [ + "header-values-get" + ], }); pub fn link_host_functions(linker: &mut component::Linker) -> anyhow::Result<()> { diff --git a/lib/src/component/types.rs b/lib/src/component/types.rs index 0a265b7e..e35ed453 100644 --- a/lib/src/component/types.rs +++ b/lib/src/component/types.rs @@ -1,3 +1,43 @@ -use {super::fastly::api::types, crate::session::Session}; +use { + super::fastly::api::types, + crate::{ + error::{self, HandleError}, + session::Session, + }, + http::header::InvalidHeaderName, +}; -impl types::Host for Session {} +pub enum TrappableError { + Error(types::Error), + Trap(anyhow::Error), +} + +impl types::Host for Session { + fn convert_error(&mut self, err: TrappableError) -> wasmtime::Result { + match err { + TrappableError::Error(err) => Ok(err), + TrappableError::Trap(err) => Err(err), + } + } +} + +impl From for TrappableError { + fn from(_: HandleError) -> Self { + Self::Error(types::Error::BadHandle) + } +} + +impl From for TrappableError { + fn from(_: InvalidHeaderName) -> Self { + Self::Error(types::Error::GenericError) + } +} + +impl From for TrappableError { + fn from(e: error::Error) -> Self { + match e { + error::Error::FatalError(_) => Self::Trap(anyhow::anyhow!(e.to_string())), + _ => Self::Error(e.into()), + } + } +} diff --git a/lib/src/execute.rs b/lib/src/execute.rs index e1c9b925..4057f617 100644 --- a/lib/src/execute.rs +++ b/lib/src/execute.rs @@ -465,15 +465,20 @@ impl ExecuteCtx { let result = compute .fastly_api_reactor() .call_serve(&mut store, req.into(), body.into()) - .await - .map_err(ExecutionError::Typechecking)?; + .await; let outcome = match result { - Ok(()) => Ok(()), - Err(()) => { + Ok(Ok(())) => Ok(()), + + Ok(Err(())) => { event!(Level::ERROR, "WebAssembly exited with an error"); Err(ExecutionError::WasmTrap(anyhow::Error::msg("failed"))) } + + Err(e) => { + event!(Level::ERROR, "WebAssembly trapped: {:?}", e); + Err(ExecutionError::WasmTrap(e)) + } }; // Ensure the downstream response channel is closed, whether or not a response was