Skip to content
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.

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

Support interceptor in spin outbound http #2817

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

lann
Copy link
Collaborator

@lann lann commented Sep 10, 2024

  • Update interceptor interface:
    • Move into new module
    • Add new InterceptRequest wrapper type to handle differences between spin and wasi requests
    • Make intercept request param owned to be returned in InterceptOutcome::Continue branch, replacing awkward std::mem::take

@lann lann requested a review from rylev September 10, 2024 19:25
use spin_world::async_trait;
use wasmtime_wasi_http::{body::HyperOutgoingBody, HttpResult};

pub type HyperBody = HyperOutgoingBody;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems a bit unnecessary? I don't see how HyperBody is clearer than HyperOutgoingBody

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's used in the Complete variant below, where it is actually an incoming body. I don't really know why wasmtime_wasi_http has both types; they alias the same underlying type...

pub trait OutboundHttpInterceptor: Send + Sync {
/// Intercept an outgoing HTTP request.
///
/// If this method returns [`InterceptedResponse::Continue`], the (possibly
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the docs here are slightly wrong since we're no longer updating requests but rather passing the request and back out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You can still update it, just as owned-mut rather than &mut. 🤷

}

impl InterceptRequest {
pub fn into_hyper_request(self) -> Request<HyperBody> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this also be pub(crate)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

External impls need it to get the request body in the InterceptOutcome::Complete path.

Signed-off-by: Lann Martin <lann.martin@fermyon.com>
@lann lann merged commit c0bfb61 into spinframework:main Sep 11, 2024
17 checks passed
@lann lann deleted the intercept-spin branch September 11, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants