smithy-lang / smithy-rs

Code generation for the AWS SDK for Rust, as well as server and generic smithy client generation.
Apache License 2.0
491 stars 187 forks source link

Server's `build` and `build_unchecked` apply layers from the generated config object in a different order #3605

Open david-perez opened 4 months ago

david-perez commented 4 months ago

build() returns:

pub fn build(self) -> Result<
    PokemonService<
        ::aws_smithy_http_server::routing::RoutingService<
            ::aws_smithy_http_server::protocol::rest::router::RestRouter<L::Service>,
            ::aws_smithy_http_server::protocol::rest_json_1::RestJson1,
        >,
    >,
    MissingOperationsError,
>
where
    L: ::tower::Layer<::aws_smithy_http_server::routing::Route<Body>>,
{
    ...
    let svc = ::aws_smithy_http_server::routing::RoutingService::new(router);
    let svc = svc.map(|s| s.layer(self.layer));
    Ok(PokemonService { svc })
}

So the .layers registered in config will run in B position after routing.

On the other hand, build_unchecked() returns:

pub fn build_unchecked(self) -> PokemonService<L::Service>
where
    Body: Send + 'static,
    L: ::tower::Layer<
        ::aws_smithy_http_server::routing::RoutingService<::aws_smithy_http_server::protocol::rest::router::RestRouter<::aws_smithy_http_server::routing::Route<Body>>, ::aws_smithy_http_server::protocol::rest_json_1::RestJson1>
    >
{
    ...
    let svc = self
     .layer
     .layer(::aws_smithy_http_server::routing::RoutingService::new(router));
    PokemonService { svc }
}

So the .layers run in A position, before routing.

The correct behavior is build's. We introduced the config object as a nice way to register middleware within routing; users can always register middleware in A position by wrapping the built Service with tower::Layers as they'd do with any other Service.

When fixing this:

david-perez commented 4 months ago

Although this is a bug, this is a breaking change if people were relying on the return type of build_unchecked.