Skip to content

[v0.3] model bodies after stream<u8, trailers, error-code>, trap less #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Apr 11, 2025
Merged

[v0.3] model bodies after stream<u8, trailers, error-code>, trap less #162

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
aba4ece
feat(p3): be less trappy
rvolosatovs Mar 13, 2025
1b2455f
chore(p3): orphan `headers` returned by `headers()`
rvolosatovs Mar 13, 2025
128af94
doc(p3): improve wording
rvolosatovs Mar 19, 2025
55db2ff
feat(p3): avoid auto-close semantics
rvolosatovs Mar 19, 2025
70ad39c
p3: allow multiple active body refs to exist, drop `into-parts`
rvolosatovs Mar 20, 2025
eac8416
p3: add `request-options#clone`
rvolosatovs Mar 20, 2025
f927cf4
p3: model `body` after `stream<u8, trailers, error-code>`
rvolosatovs Mar 19, 2025
865daa5
p3: remove `body` resource
rvolosatovs Mar 21, 2025
c3517c6
p3: use `result`, not `option` in `body()`
rvolosatovs Mar 21, 2025
ae89575
p3: make request/response bodies optional
rvolosatovs Mar 24, 2025
848eaf5
p3: document optional content stream optimization
rvolosatovs Mar 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 75 additions & 93 deletions wit-0.3.0-draft/types.wit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,77 +230,36 @@ interface types {
/// Trailers is an alias for Fields.
type trailers = fields;

/// Represents an HTTP Request or Response's Body.
///
/// A body has both its contents - a stream of bytes - and a (possibly empty)
/// set of trailers, indicating that the full contents of the body have been
/// received. This resource represents the contents as a `stream<u8>` and the
/// delivery of trailers as a `trailers`, and ensures that the user of this
/// 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.
///
/// This function returns a future, which will resolve
/// to an error code if transmitting stream data fails.
///
/// The returned future resolves to success once body stream
/// is fully transmitted.
new: static func(
%stream: stream<u8>,
) -> tuple<body, future<result<_, error-code>>>;

/// Construct a new `body` with the specified stream and trailers.
///
/// This function returns a future, which will resolve
/// to an error code if transmitting stream data or trailers fails.
///
/// The returned future resolves to success once body stream and trailers
/// are fully transmitted.
new-with-trailers: static func(
%stream: stream<u8>,
trailers: future<trailers>
) -> tuple<body, future<result<_, error-code>>>;

/// Returns the contents of the body, as a stream of bytes.
///
/// This function may be called multiple times as long as any `stream`s
/// returned by previous calls have been dropped first.
///
/// On success, this function returns a stream and a future, which will resolve
/// to an error code if receiving data from stream fails.
/// The returned future resolves to success if body is closed.
%stream: func() -> result<tuple<stream<u8>, future<result<_, error-code>>>>;

/// Takes ownership of `body`, and returns an unresolved optional `trailers` result.
///
/// This function will trap if a `stream` child is still alive.
finish: static func(this: body) -> future<result<option<trailers>, error-code>>;
}

/// Represents an HTTP Request.
resource request {

/// Construct a new `request` with a default `method` of `GET`, and
/// `none` values for `path-with-query`, `scheme`, and `authority`.
///
/// * `headers` is the HTTP Headers for the Response.
/// * `body` is the optional contents of the body, possibly including
/// trailers.
/// * `options` is optional `request-options` to be used if the request is
/// sent over a network connection.
/// `headers` is the HTTP Headers for the Request.
///
/// `contents` is the optional body content stream with `none`
/// representing a zero-length content stream.
/// Once it is closed, `trailers` future must resolve to a result.
/// If `trailers` resolves to an error, underlying connection
/// will be closed immediately.
///
/// `options` is optional `request-options` resource to be used
/// if the request is sent over a network connection.
///
/// It is possible to construct, or manipulate with the accessor functions
/// below, an `request` with an invalid combination of `scheme`
/// below, a `request` with an invalid combination of `scheme`
/// and `authority`, or `headers` which are not permitted to be sent.
/// It is the obligation of the `handler.handle` implementation
/// to reject invalid constructions of `request`.
constructor(
///
/// The returned future resolves to result of transmission of this request.
new: static func(
headers: headers,
body: option<body>,
contents: option<stream<u8>>,
trailers: future<result<option<trailers>, error-code>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thought on this: are there downsides to always having to pass a future here? I'm not sure what the overhead (both resource- and boilerplate-wise) of creating an managing a future is. If it's trivial, then this doesn't much matter, but otherwise, should we consider wrapping this into an option<>?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the "stream<T, U, V> world", if we go with optional body in constructors, the parameter would be option<stream<u8, option<trailers>, error-code>>, meaning that users could either pass no body stream or pass a body stream, with potentially empty contents, but mandatory final element (option<trailers> or error-code).
From what I understand, there would not be a way to optionally pass a final stream element.

With that in mind, if we want to make the future optional, we'd need to take an option<tuple<stream<u8>, future<result<option<trailers>, error-code>>>>, which would then directly translate to option<stream<u8, option<trailers>, error-code>> in the future. (see also #162 (comment))

FWIW, here's what it takes to create a new future in Rust guest and pass to a constructor: https://github.com/bytecodealliance/wasip3-prototyping/blob/ad4ccf03172b22320c4a22971c51b4f7249a2846/crates/test-programs/src/p3/http.rs#L78-L81
and then send a value: https://github.com/bytecodealliance/wasip3-prototyping/blob/ad4ccf03172b22320c4a22971c51b4f7249a2846/crates/test-programs/src/p3/http.rs#L112
So 2 LoC, plus an import of futures::SinkExt to expose futures::SinkExt::send

I would not expect creating a future handle to be an expensive operation, especially considering the fact that we're most likely dealing with network I/O here.

I'm happy to switch to a tuple here, if that's what everyone prefers

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the overhead shouldn't be too significant. Also, it encourages the generally good engineering practice of explicitly signalling the final success or failure of the request/response in the result.

options: option<request-options>
);
) -> tuple<request, future<result<_, error-code>>>;

/// Get the Method for the Request.
method: func() -> method;
Expand Down Expand Up @@ -348,22 +307,28 @@ interface types {
///
/// The returned `headers` resource is immutable: `set`, `append`, and
/// `delete` operations will fail with `header-error.immutable`.
///
/// This headers resource is a child: it must be dropped before the parent
/// `request` is dropped, or its ownership is transferred to another
/// component by e.g. `handler.handle`.
headers: func() -> headers;

/// Get the body associated with the Request, if any.
/// Get body of the Request.
///
/// This body resource is a child: it must be dropped before the parent
/// `request` is dropped, or its ownership is transferred to another
/// component by e.g. `handler.handle`.
body: func() -> option<body>;

/// Takes ownership of the `request` and returns the `headers`, `body`
/// and `request-options`, if any.
into-parts: static func(this: request) -> tuple<headers, option<body>, option<request-options>>;
/// Stream returned by this method represents the contents of the body.
/// Once the stream is reported as closed, callers should await the returned future
/// to determine whether the body was received successfully.
/// The future will only resolve after the stream is reported as closed.
///
/// The stream and future returned by this method are children:
/// they should be closed or consumed before the parent `response`
/// is dropped, or its ownership is transferred to another component
/// by e.g. `handler.handle`.
///
/// This method may be called multiple times.
///
/// This method will return an error if it is called while either:
/// - a stream or future returned by a previous call to this method is still open
/// - a stream returned by a previous call to this method has reported itself as closed
/// Thus there will always be at most one readable stream open for a given body.
/// Each subsequent stream picks up where the last stream left off, up until it is finished.
body: func() -> result<tuple<stream<u8>, future<result<option<trailers>, error-code>>>>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on the answer to my question above, this might also have to change? Though even if we make the trailers ctor parameter optional, I think we could leave this as-is, by constructing a future internally if none was passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not introduce a distinction here using the same reasoning as laid out by @lukewagner in #162 (comment)

If the future was optional, users of this API would have no way of knowing if request/response receipt was successful if the future was none.

E.g. if a component read one byte from stream<u8>, after which it was closed and future was none - was the underlying I/O successful or was there a failure?

The future resolving to a success means that the incoming request/response has been received it it's entirety. It seems that if it were optional, none would be only possible in very niche use cases, e.g. if the resource was constructed by the guest and then requested by itself via body.

I would rather avoid having to document/specify this behavior making this interface more self-documenting

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for the explanation and references: that all makes sense to me!

}

/// Parameters for making an HTTP Request. Each of these parameters is
Expand Down Expand Up @@ -400,6 +365,10 @@ interface types {
/// body stream. An error return value indicates that this timeout is not
/// supported or that this handle is immutable.
set-between-bytes-timeout: func(duration: option<duration>) -> result<_, request-options-error>;

/// Make a deep copy of the `request-options`.
/// The resulting `request-options` is mutable.
clone: func() -> request-options;
}

/// This type corresponds to the HTTP standard Status Code.
Expand All @@ -408,17 +377,24 @@ interface types {
/// Represents an HTTP Response.
resource response {

/// Construct an `response`, with a default `status-code` of `200`. If a
/// different `status-code` is needed, it must be set via the
/// Construct a new `response`, with a default `status-code` of `200`.
/// If a different `status-code` is needed, it must be set via the
/// `set-status-code` method.
///
/// * `headers` is the HTTP Headers for the Response.
/// * `body` is the optional contents of the body, possibly including
/// trailers.
constructor(
/// `headers` is the HTTP Headers for the Response.
///
/// `contents` is the optional body content stream with `none`
/// representing a zero-length content stream.
/// Once it is closed, `trailers` future must resolve to a result.
/// If `trailers` resolves to an error, underlying connection
/// will be closed immediately.
///
/// The returned future resolves to result of transmission of this response.
new: static func(
headers: headers,
body: option<body>,
);
contents: option<stream<u8>>,
trailers: future<result<option<trailers>, error-code>>,
) -> tuple<response, future<result<_, error-code>>>;

/// Get the HTTP Status Code for the Response.
status-code: func() -> status-code;
Expand All @@ -427,25 +403,31 @@ interface types {
/// given is not a valid http status code.
set-status-code: func(status-code: status-code) -> result;

/// Get the headers associated with the Request.
/// Get the headers associated with the Response.
///
/// The returned `headers` resource is immutable: `set`, `append`, and
/// `delete` operations will fail with `header-error.immutable`.
///
/// This headers resource is a child: it must be dropped before the parent
/// `response` is dropped, or its ownership is transferred to another
/// component by e.g. `handler.handle`.
headers: func() -> headers;

/// Get the body associated with the Response, if any.
/// Get body of the Response.
///
/// This body resource is a child: it must be dropped before the parent
/// `response` is dropped, or its ownership is transferred to another
/// component by e.g. `handler.handle`.
body: func() -> option<body>;

/// Takes ownership of the `response` and returns the `headers` and `body`,
/// if any.
into-parts: static func(this: response) -> tuple<headers, option<body>>;
/// Stream returned by this method represents the contents of the body.
/// Once the stream is reported as closed, callers should await the returned future
/// to determine whether the body was received successfully.
/// The future will only resolve after the stream is reported as closed.
///
/// The stream and future returned by this method are children:
/// they should be closed or consumed before the parent `response`
/// is dropped, or its ownership is transferred to another component
/// by e.g. `handler.handle`.
///
/// This method may be called multiple times.
///
/// This method will return an error if it is called while either:
/// - a stream or future returned by a previous call to this method is still open
/// - a stream returned by a previous call to this method has reported itself as closed
/// Thus there will always be at most one readable stream open for a given body.
/// Each subsequent stream picks up where the last stream left off, up until it is finished.
body: func() -> result<tuple<stream<u8>, future<result<option<trailers>, error-code>>>>;
}
}