From 65c6e80f1937264c555c1647cad9192137a3a36e Mon Sep 17 00:00:00 2001 From: Victor Adossi Date: Wed, 19 Feb 2025 20:42:10 +0900 Subject: [PATCH] feat: update to independent trailer retrieval This commit updates `wasi:http/types#body.finish` to avoid resolving optional trailers, but instead returning them for the caller to resolve. This may introduce some efficiency gains in the cases where trailers can be safely ignored, and does avoid the work done in checking whether they are present or not, at least. Signed-off-by: Victor Adossi --- .../component-async-tests/http/src/lib.rs | 25 ++++--------------- .../wit/deps/http/types.wit | 10 ++++---- .../test-programs/src/bin/async_http_echo.rs | 11 ++++++-- .../src/bin/async_http_middleware.rs | 22 +++++++++++++--- 4 files changed, 37 insertions(+), 31 deletions(-) diff --git a/crates/misc/component-async-tests/http/src/lib.rs b/crates/misc/component-async-tests/http/src/lib.rs index 3a344c627..e853ef51f 100644 --- a/crates/misc/component-async-tests/http/src/lib.rs +++ b/crates/misc/component-async-tests/http/src/lib.rs @@ -27,11 +27,8 @@ use { anyhow::anyhow, std::{fmt, future::Future, mem}, wasi::http::types::{ErrorCode, HeaderError, Method, RequestOptionsError, Scheme}, - wasmtime::{ - component::{ - Accessor, ErrorContext, FutureReader, Linker, Resource, ResourceTable, StreamReader, - }, - AsContextMut, + wasmtime::component::{ + Accessor, ErrorContext, FutureReader, Linker, Resource, ResourceTable, StreamReader, }, }; @@ -233,25 +230,13 @@ where async fn finish( accessor: &mut Accessor, this: Resource, - ) -> wasmtime::Result>, ErrorCode>> { + ) -> wasmtime::Result>>> { let trailers = accessor.with(|mut store| { let trailers = store.data_mut().table().delete(this)?.trailers; - trailers - .map(|v| v.read(store.as_context_mut()).map(|v| v.into_future())) - .transpose() + Ok(trailers) as wasmtime::Result<_> })?; - let maybe_trailers = if let Some(trailers) = trailers { - match trailers.await { - Some(Ok(trailers)) => Some(trailers), - Some(Err(_err_ctx)) => return Ok(Err(ErrorCode::InternalError(None))), - None => None, - } - } else { - None - }; - - Ok(Ok(maybe_trailers)) + Ok(trailers) } fn drop(&mut self, this: Resource) -> wasmtime::Result<()> { diff --git a/crates/misc/component-async-tests/wit/deps/http/types.wit b/crates/misc/component-async-tests/wit/deps/http/types.wit index 4c5bd4c4e..ec46d0a58 100644 --- a/crates/misc/component-async-tests/wit/deps/http/types.wit +++ b/crates/misc/component-async-tests/wit/deps/http/types.wit @@ -119,7 +119,7 @@ interface types { /// permitted because the fields are immutable. immutable, } - + /// This type enumerates the different kinds of errors that may occur when /// setting fields of a `request-options` resource. variant request-options-error { @@ -234,13 +234,13 @@ interface types { /// interface may only be consuming either the body contents or waiting on /// trailers at any given time. resource body { - + /// Construct a new `body` with the specified stream and trailers. constructor( %stream: stream, trailers: option> ); - + /// Returns the contents of the body, as a stream of bytes. /// /// This function may be called multiple times as long as any `stream`s @@ -249,9 +249,9 @@ interface types { /// Takes ownership of `body`, and returns a `trailers`. This function will /// trap if a `stream` child is still alive. - finish: static func(this: body) -> result, error-code>; + finish: static func(this: body) -> option>; } - + /// Represents an HTTP Request. resource request { diff --git a/crates/test-programs/src/bin/async_http_echo.rs b/crates/test-programs/src/bin/async_http_echo.rs index bcf395058..89dcd8d40 100644 --- a/crates/test-programs/src/bin/async_http_echo.rs +++ b/crates/test-programs/src/bin/async_http_echo.rs @@ -51,8 +51,15 @@ impl Handler for Component { drop(pipe_tx); - if let Some(trailers) = Body::finish(body).await.unwrap() { - trailers_tx.write(trailers).await; + if let Some(maybe_trailers) = Body::finish(body).await { + if let Some(trailers) = maybe_trailers.await { + trailers_tx + .write( + trailers + .expect("trailer does not resolve to an error when present"), + ) + .await; + } } }); diff --git a/crates/test-programs/src/bin/async_http_middleware.rs b/crates/test-programs/src/bin/async_http_middleware.rs index 9f8eef150..b85d2ce05 100644 --- a/crates/test-programs/src/bin/async_http_middleware.rs +++ b/crates/test-programs/src/bin/async_http_middleware.rs @@ -85,8 +85,15 @@ impl Handler for Component { drop(pipe_tx); } - if let Some(trailers) = Body::finish(body).await.unwrap() { - trailers_tx.write(trailers).await + if let Some(maybe_trailers) = Body::finish(body).await { + if let Some(trailers) = maybe_trailers.await { + trailers_tx + .write( + trailers + .expect("trailer does not resolve to an error when present"), + ) + .await; + } } }); @@ -138,8 +145,15 @@ impl Handler for Component { drop(pipe_tx); } - if let Some(trailers) = Body::finish(body).await.unwrap() { - trailers_tx.write(trailers).await; + if let Some(maybe_trailers) = Body::finish(body).await { + if let Some(trailers) = maybe_trailers.await { + trailers_tx + .write( + trailers + .expect("trailer does not resolve to an error when present"), + ) + .await; + } } });