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.

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

Jump to bottom

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

Open
david-perez opened this issue Apr 26, 2024 · 1 comment
Open

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

david-perez opened this issue Apr 26, 2024 · 1 comment
Labels
breaking-change This will require a breaking change bug Something isn't working server Rust server SDK

Comments

@david-perez
Copy link
Contributor

david-perez commented Apr 26, 2024 •
edited

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:

  • document in the Rust docs for the generated config object that layers get run in B position, HTTP plugins in C position, and model plugins in D position.
    • Call out how to add a layer in A position.
  • add a test to our example Pokemon service that ensures that middleware gets invoked in ABCD order regardless of whether we use build or build_unchecked.
@david-perez david-perez added bug Something isn't working server Rust server SDK breaking-change This will require a breaking change labels Apr 26, 2024
@david-perez
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking-change This will require a breaking change bug Something isn't working server Rust server SDK
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@david-perez