From d957cbdd941c7180874dfdcbb289a9a0a1dfb949 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Thu, 4 Aug 2022 17:18:26 +0000
Subject: [PATCH 01/26] Draft background section

---
 design/src/rfcs/rfc0020_service_builder.md | 269 +++++++++++++++++++++
 1 file changed, 269 insertions(+)
 create mode 100644 design/src/rfcs/rfc0020_service_builder.md

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
new file mode 100644
index 0000000000..ee9a25b850
--- /dev/null
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -0,0 +1,269 @@
+# RFC: Better Service Builders
+
+> Status: RFC
+
+One might characterize `smithy-rs` as a tool for transforming a [Smithy service](https://awslabs.github.io/smithy/1.0/spec/core/model.html#service) into a [tower::Service](https://docs.rs/tower-service/latest/tower_service/trait.Service.html) builder. A Smithy model defines semantics and constrains the behavior of the generated service, but not completely - closures must be passed to the builder to before the `tower::Service` is fully specified. This builder structure is the primary API surface we provide to the customer, as a result, it is important that it meets their needs.
+
+This RFC proposes a new builder, deprecating the existing one, which addresses API deficiencies and takes steps to improve performance.
+
+## Terminology
+
+- **Model**: A [Smithy Model](https://awslabs.github.io/smithy/1.0/spec/core/model.html), usually pertaining to the one in use by the customer.
+- **Smithy Service**: The entry point of an API that aggregates resources and operations together within a Smithy model. Described in detail [here](https://awslabs.github.io/smithy/1.0/spec/core/model.html#service).
+- **Service**: The `tower::Service` trait is an interface for writing network applications in a modular and reusable way. `Service`s act on requests to produce responses.
+- **Service Builder**: A `tower::Service` builder, generated from a Smithy service, by `smithy-rs`.
+- **Middleware**: Broadly speaking, middleware modify requests and responses. Concretely, these are exist as implementations of [Layer](https://docs.rs/tower/latest/tower/layer/trait.Layer.html)/a `Service` wrapping an inner `Service`.
+
+## Background
+
+Before engaging with the proposal, a survey of the current state of affairs is required.
+
+The following is a reference model we will use throughout the RFC:
+
+```smithy
+operation Operation0 {
+    input: Input0,
+    output: Output0
+}
+
+operation Operation1 {
+    input: Input1,
+    output: Output1
+}
+
+service Service {
+    operations: [
+        Operation0,
+        Operation0,
+    ]
+}
+```
+
+We have purposely omitted details from the model that are unimportant to describing the proposal. We also omit distracting details from the Rust snippets.
+
+### Handlers
+
+Before we progress onto a more general look at the current service builder, we should familiarize ourselves with the specifics of the `Handler` trait:
+
+```rust
+pub trait Handler<T, Input> {
+    async fn call(self, req: http::Request) -> http::Response;
+}
+```
+
+Its purpose is to provide an even interface over closures of the form `FnOnce(Input) -> impl Future<Output = Output>` and `FnOnce(Input, State) -> impl Future<Output = Output>`. It's this abstraction which allows the customers to supply both `async fn operation0(input: Operation0Input) -> Operation0Output` and `async fn operation0(input: Operation0Input, state: Extension<S>) -> Operation0Output` to the service builder.
+
+We generate `Handler` implementations for said closures in [ServerOperationHandlerGenerator.kt](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationHandlerGenerator.kt), for `Operation0` these are:
+
+```rust
+impl<Fun, Fut> Handler<(), Operation0Input> for Fun
+where
+    Fun: FnOnce(crate::input::Operation0Input) -> Fut,
+    Fut: Future<Output = Operation0Output>,
+{
+    async fn call(self, request: http::Request) -> http::Response {
+        let input = /* Create `Operation0Input` from `request: http::Request` */;
+
+        // Use closure on the input
+        let output = self(input).await;
+
+        let response = /* Create `http::Response` from `output: Operation0Output` */
+        response
+    }
+}
+
+impl<Fun, Fut> Handler<Extension<S>, Operation0Input> for Fun
+where
+    Fun: FnOnce(Operation0Input, Extension<S>) -> Fut,
+    Fut: Future<Output = Operation0Output>,
+{
+    async fn call(self, request: http::Request) -> http::Response {
+        let input = /* Create `Operation0Input` from `request: http::Request` */;
+
+        // Use closure on the input and fetched extension data
+        let extension = Extension(request.extensions().get::<T>().clone());
+        let output = self(input, extension).await;
+
+        let response = /* Create `http::Response` from `output: Operation0Output` */
+        response
+    }
+}
+```
+
+Creating `Operation0Input` from a `http::Request` and `http::Response` from a `Operation0Output` involves the HTTP binding traits and protocol aware deserialization. The structure [RuntimeError](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/runtime_error.rs#L53-L5) enumerates internal error cases such as serialization/deserialization failures, `extensions().get::<T>()` failures, etc. We omit error handling in the snippets above, but, in full, they also involve protocol aware conversions from the `RuntimeError` and `http::Response`. The reader should make note of the influence of the model/protocol on the different sections of this procedure.
+
+The `request.extensions().get::<T>()` present in the `Fun: FnOnce(Operation0Input, Extension<S>) -> Fut` implementation is the current approach to injecting state into handlers. The customer is required to apply a [AddExtensionLayer](https://docs.rs/tower-http/latest/tower_http/add_extension/struct.AddExtensionLayer.html) to the output of the service builder so that, when the request reaches the handler, the `extensions().get::<T>()` will succeed.
+
+To convert the closures described above into a `Service` `OperationHandler` is used:
+
+```rust
+pub struct OperationHandler<H, T, Input> {
+    handler: H,
+    _marker: PhantomData<(T, Input)>,
+}
+
+impl<H, T, Input> Service<Request<B>> for OperationHandler<H, T, Input>
+where
+    H: Handler<T, I>,
+{
+    type Response = http::Response;
+    type Error = Infallible;
+
+    #[inline]
+    fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
+        Poll::Ready(Ok(()))
+    }
+
+    async fn call(&mut self, req: Request<B>) -> Result<Self::Response, Self::Error> {
+        self.handler.call(req).await.map(Ok)
+    }
+}
+```
+
+The service build uses both `Handler` and `OperationHandler`
+
+### Builder
+
+The service builder we currently provide to the customer takes the form of the `OperationRegistryBuilder`, generated from [ServerOperationRegistryGenerator.kt](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationRegistryGenerator.kt).
+
+Currently, the reference model would generate the following `OperationRegistryBuilder` and `OperationRegistry`:
+
+```rust
+pub struct OperationRegistryBuilder<Op0, In0, Op1, In1> {
+    operation1: Option<Op0>,
+    operation2: Option<Op1>,
+    _phantom: PhantomData<(In0, In1)>,
+}
+
+pub struct OperationRegistry<Op0, In0, Op1, In1> {
+    operation1: Op0,
+    operation2: Op1,
+    _phantom: PhantomData<(In0, In1)>,
+}
+```
+
+The `OperationRegistryBuilder` includes a setter per operation, and a fallible `build` method:
+
+```rust
+impl<Op0, In0, Op1, In1> OperationRegistryBuilder<Op0, In0, Op1, In1> {
+    pub fn operation0(mut self, value: Op0) -> Self {
+        self.operation0 = Some(value);
+        self
+    }
+    pub fn operation1(mut self, value: Op1) -> Self {
+        self.operation1 = Some(value);
+        self
+    }
+    pub fn build(
+        self,
+    ) -> Result<OperationRegistry<Op0, In0, Op1, In1>, OperationRegistryBuilderError> {
+        Ok(OperationRegistry {
+            operation0: self.operation0.ok_or(/* OperationRegistryBuilderError */)?,
+            operation1: self.operation1.ok_or(/* OperationRegistryBuilderError */)?,
+            _phantom: PhantomData,
+        })
+    }
+}
+```
+
+The `OperationRegistry` does not include any methods of its own, however it does enjoy a `From<OperationRegistry> for Router<B>` implementation:
+
+```rust
+impl<B, Op0, In0, Op1, In1> From<OperationRegistry<B, Op0, In0, Op1, In1>> for Router<B>
+where
+    Op0: Handler<B, In0, Operation0Input>,
+    Op1: Handler<B, In1, Operation1Input>,
+{
+    fn from(registry: OperationRegistry<B, Op0, In0, Op1, In1>) -> Self {
+        let operation0_request_spec = /* Construct Operation0 routing information */;
+        let operation1_request_spec = /* Construct Operation1 routing information */;
+
+        // Convert handlers into boxed services
+        let operation0_svc = Box::new(OperationHandler::new(registry.operation0));
+        let operation1_svc = Box::new(OperationHandler::new(registry.operation1));
+
+        // Initialize the protocol specific router
+        // We demonstrate it here with `new_rest_json_router`, but note that there is a different router constructor
+        // for each protocol.
+        aws_smithy_http_server::routing::Router::new_rest_json_router(vec![
+            (
+                operation0_request_spec,
+                operation0_svc
+            ),
+            (
+                operation1_request_spec,
+                operation1_svc
+            )
+        ])
+    }
+}
+```
+
+A customer using this API would follow this kind of pattern:
+
+```rust
+async fn handler0(input: Operation0Input) -> Operation0Output {
+    todo!()
+}
+
+async fn handler1(input: Operation1Input) -> Operation1Output {
+    todo!()
+}
+
+let app: Router = OperationRegistryBuilder::default()
+    // Use the setters
+    .operation0(handler0)
+    .operation1(handler1)
+    // Convert to `OperationRegistry`
+    .build()
+    .unwrap()
+    // Convert to `Router`
+    .into();
+```
+
+### Router
+
+The router today exists as
+
+```rust
+pub struct Route {
+    service: BoxCloneService<http::Request, http::Response, Infallible>,
+}
+
+enum Routes {
+    RestXml(Vec<(Route, RequestSpec)>),
+    RestJson1(Vec<(Route, RequestSpec)>),
+    AwsJson10(TinyMap<String, Route>),
+    AwsJson11(TinyMap<String, Route>),
+}
+
+pub struct Router {
+    routes: Routes,
+}
+```
+
+and enjoys the following `Service<http::Request>` implementation:
+
+```rust
+impl Service<http::Request> for Router
+{
+    type Response = http::Response;
+    type Error = Infallible;
+
+    fn poll_ready(&mut self, _: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
+        Poll::Ready(Ok(()))
+    }
+
+    async fn call(&mut self, request: http::Request) -> Result<Self::Response, Self::Error> {
+        match &self.routes {
+            Routes::/* protocol */(routes) => {
+                let route: Result<_, _> = /* perform route matching logic */;
+                match route {
+                    Ok(ok) => ok.oneshot().await,
+                    Err(err) => /* Convert routing error into http::Response */
+                }
+            }
+        }
+    }
+}
+```

From 9f85f836e27abefa416e110e85d11a1fc9eb930c Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Thu, 4 Aug 2022 18:19:16 +0000
Subject: [PATCH 02/26] Improvements to Background

---
 design/src/rfcs/rfc0020_service_builder.md | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index ee9a25b850..e19d4e6142 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -223,7 +223,7 @@ let app: Router = OperationRegistryBuilder::default()
 
 ### Router
 
-The router today exists as
+The [router today](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/routing/mod.rs#L58-L60) exists as
 
 ```rust
 pub struct Route {
@@ -257,7 +257,7 @@ impl Service<http::Request> for Router
     async fn call(&mut self, request: http::Request) -> Result<Self::Response, Self::Error> {
         match &self.routes {
             Routes::/* protocol */(routes) => {
-                let route: Result<_, _> = /* perform route matching logic */;
+                let route: Result<Route, _> = /* perform route matching logic */;
                 match route {
                     Ok(ok) => ok.oneshot().await,
                     Err(err) => /* Convert routing error into http::Response */
@@ -267,3 +267,16 @@ impl Service<http::Request> for Router
     }
 }
 ```
+
+Along side the protocol specific constructors, `Router` includes a `layer` method. This provides a way for the customer to apply a `tower::Layer` to all routes. For every protocol, `Router::layer`, has the approximately the same behavior:
+
+```rust
+let new_routes = old_routes
+    .into_iter()
+    // Apply the layer
+    .map(|route| layer.layer(route))
+    // Re-box the service, to restore `Route` type
+    .map(|svc| Box::new(svc))
+    // Collect the iterator back into a collection (`Vec` or `TinyMap`)
+    .collect();
+```

From f470d7067fb0e21c80de03dc8724ccae48b4cf44 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Thu, 4 Aug 2022 21:20:08 +0000
Subject: [PATCH 03/26] Background tweaks

---
 design/src/rfcs/rfc0020_service_builder.md | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index e19d4e6142..1b32f9400f 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -1,4 +1,4 @@
-# RFC: Better Service Builders
+# RFC: Service Builder Improvements
 
 > Status: RFC
 
@@ -16,7 +16,7 @@ This RFC proposes a new builder, deprecating the existing one, which addresses A
 
 ## Background
 
-Before engaging with the proposal, a survey of the current state of affairs is required.
+To provide context for the proposal we perform a survey of the current state of affairs.
 
 The following is a reference model we will use throughout the RFC:
 
@@ -43,7 +43,7 @@ We have purposely omitted details from the model that are unimportant to describ
 
 ### Handlers
 
-Before we progress onto a more general look at the current service builder, we should familiarize ourselves with the specifics of the `Handler` trait:
+An core concept in the current service builder is the `Handler` trait:
 
 ```rust
 pub trait Handler<T, Input> {
@@ -280,3 +280,7 @@ let new_routes = old_routes
     // Collect the iterator back into a collection (`Vec` or `TinyMap`)
     .collect();
 ```
+
+### Comparison to Axum
+
+Historically, `smithy-rs` has borrowed from [axum](https://github.com/tokio-rs/axum). Despite various divergences the code bases still have much in common.

From 1db14dd07ab23f4352f9b2c90efb74745af1b7a8 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Fri, 5 Aug 2022 22:12:08 +0000
Subject: [PATCH 04/26] Improve background

---
 design/src/rfcs/rfc0020_service_builder.md | 170 ++++++++++++++++-----
 1 file changed, 129 insertions(+), 41 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 1b32f9400f..0a9cf93e86 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -2,7 +2,7 @@
 
 > Status: RFC
 
-One might characterize `smithy-rs` as a tool for transforming a [Smithy service](https://awslabs.github.io/smithy/1.0/spec/core/model.html#service) into a [tower::Service](https://docs.rs/tower-service/latest/tower_service/trait.Service.html) builder. A Smithy model defines semantics and constrains the behavior of the generated service, but not completely - closures must be passed to the builder to before the `tower::Service` is fully specified. This builder structure is the primary API surface we provide to the customer, as a result, it is important that it meets their needs.
+One might characterize `smithy-rs` as a tool for transforming a [Smithy service](https://awslabs.github.io/smithy/1.0/spec/core/model.html#service) into a [tower::Service](https://docs.rs/tower-service/latest/tower_service/trait.Service.html) builder. A Smithy model defines behavior of the generated service partially - handlers must be passed to the builder before the `tower::Service` is fully specified. This builder structure is the primary API surface we provide to the customer, as a result, it is important that it meets their needs.
 
 This RFC proposes a new builder, deprecating the existing one, which addresses API deficiencies and takes steps to improve performance.
 
@@ -13,6 +13,7 @@ This RFC proposes a new builder, deprecating the existing one, which addresses A
 - **Service**: The `tower::Service` trait is an interface for writing network applications in a modular and reusable way. `Service`s act on requests to produce responses.
 - **Service Builder**: A `tower::Service` builder, generated from a Smithy service, by `smithy-rs`.
 - **Middleware**: Broadly speaking, middleware modify requests and responses. Concretely, these are exist as implementations of [Layer](https://docs.rs/tower/latest/tower/layer/trait.Layer.html)/a `Service` wrapping an inner `Service`.
+- **Handler**: A closure defining the behavior of a particular request after routing. These are provided to the service builder to complete the describe of the service.
 
 ## Background
 
@@ -34,16 +35,40 @@ operation Operation1 {
 service Service {
     operations: [
         Operation0,
-        Operation0,
+        Operation1,
     ]
 }
 ```
 
-We have purposely omitted details from the model that are unimportant to describing the proposal. We also omit distracting details from the Rust snippets.
+We have purposely omitted details from the model that are unimportant to describing the proposal. We also omit distracting details from the Rust snippets. Code generation is linear in the sense that, code snippets can be assumed to extend to multiple operations in a predictable way. In the case where we do want to speak generally about an operation and it's associated types, we use `{Operation}`, for example `{Operation}Input` is the input type of an unspecified operation.
+
+Here is a quick example of what a customer might write when using the service builder:
+
+```rust
+async fn handler0(input: Operation0Input) -> Operation0Output {
+    todo!()
+}
+
+async fn handler1(input: Operation1Input) -> Operation1Output {
+    todo!()
+}
+
+let app: Router = OperationRegistryBuilder::default()
+    // Use the setters
+    .operation0(handler0)
+    .operation1(handler1)
+    // Convert to `OperationRegistry`
+    .build()
+    .unwrap()
+    // Convert to `Router`
+    .into();
+```
+
+During the survey we touch on the major mechanisms used to acheive this API.
 
 ### Handlers
 
-An core concept in the current service builder is the `Handler` trait:
+A core concept in the service builder is the `Handler` trait:
 
 ```rust
 pub trait Handler<T, Input> {
@@ -51,14 +76,14 @@ pub trait Handler<T, Input> {
 }
 ```
 
-Its purpose is to provide an even interface over closures of the form `FnOnce(Input) -> impl Future<Output = Output>` and `FnOnce(Input, State) -> impl Future<Output = Output>`. It's this abstraction which allows the customers to supply both `async fn operation0(input: Operation0Input) -> Operation0Output` and `async fn operation0(input: Operation0Input, state: Extension<S>) -> Operation0Output` to the service builder.
+Its purpose is to provide an even interface over closures of the form `FnOnce({Operation}Input) -> impl Future<Output = {Operation}Output>` and `FnOnce({Operation}Input, State) -> impl Future<Output = {Operation}Output>`. It's this abstraction which allows the customers to supply both `async fn handler(input: {Operation}Input) -> {Operation}Output` and `async fn handler(input: {Operation}Input, state: Extension<S>) -> {Operation}Output` to the service builder.
 
-We generate `Handler` implementations for said closures in [ServerOperationHandlerGenerator.kt](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationHandlerGenerator.kt), for `Operation0` these are:
+We generate `Handler` implementations for said closures in [ServerOperationHandlerGenerator.kt](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationHandlerGenerator.kt):
 
 ```rust
 impl<Fun, Fut> Handler<(), Operation0Input> for Fun
 where
-    Fun: FnOnce(crate::input::Operation0Input) -> Fut,
+    Fun: FnOnce(Operation0Input) -> Fut,
     Fut: Future<Output = Operation0Output>,
 {
     async fn call(self, request: http::Request) -> http::Response {
@@ -90,16 +115,15 @@ where
 }
 ```
 
-Creating `Operation0Input` from a `http::Request` and `http::Response` from a `Operation0Output` involves the HTTP binding traits and protocol aware deserialization. The structure [RuntimeError](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/runtime_error.rs#L53-L5) enumerates internal error cases such as serialization/deserialization failures, `extensions().get::<T>()` failures, etc. We omit error handling in the snippets above, but, in full, they also involve protocol aware conversions from the `RuntimeError` and `http::Response`. The reader should make note of the influence of the model/protocol on the different sections of this procedure.
+Creating `{Operation}Input` from a `http::Request` and `http::Response` from a `{Operation}Output` involves the [HTTP binding traits](https://awslabs.github.io/smithy/1.0/spec/core/http-traits.html) and protocol aware serialization/deserialization. The [RuntimeError](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/runtime_error.rs#L53-L5) enumerates error cases such as serialization/deserialization failures, `extensions().get::<T>()` failures, etc. We omit error handling in the snippets above, but, in full, they also involve protocol aware conversions from the `RuntimeError` and `http::Response`. The reader should make note of the influence of the model on the different sections of this procedure.
 
 The `request.extensions().get::<T>()` present in the `Fun: FnOnce(Operation0Input, Extension<S>) -> Fut` implementation is the current approach to injecting state into handlers. The customer is required to apply a [AddExtensionLayer](https://docs.rs/tower-http/latest/tower_http/add_extension/struct.AddExtensionLayer.html) to the output of the service builder so that, when the request reaches the handler, the `extensions().get::<T>()` will succeed.
 
-To convert the closures described above into a `Service` `OperationHandler` is used:
+To convert the closures described above into a `Service` an `OperationHandler` is used:
 
 ```rust
 pub struct OperationHandler<H, T, Input> {
     handler: H,
-    _marker: PhantomData<(T, Input)>,
 }
 
 impl<H, T, Input> Service<Request<B>> for OperationHandler<H, T, Input>
@@ -120,11 +144,9 @@ where
 }
 ```
 
-The service build uses both `Handler` and `OperationHandler`
-
 ### Builder
 
-The service builder we currently provide to the customer takes the form of the `OperationRegistryBuilder`, generated from [ServerOperationRegistryGenerator.kt](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationRegistryGenerator.kt).
+The service builder we provide to the customer takes the form of the `OperationRegistryBuilder`, generated from [ServerOperationRegistryGenerator.kt](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationRegistryGenerator.kt).
 
 Currently, the reference model would generate the following `OperationRegistryBuilder` and `OperationRegistry`:
 
@@ -132,13 +154,11 @@ Currently, the reference model would generate the following `OperationRegistryBu
 pub struct OperationRegistryBuilder<Op0, In0, Op1, In1> {
     operation1: Option<Op0>,
     operation2: Option<Op1>,
-    _phantom: PhantomData<(In0, In1)>,
 }
 
 pub struct OperationRegistry<Op0, In0, Op1, In1> {
     operation1: Op0,
     operation2: Op1,
-    _phantom: PhantomData<(In0, In1)>,
 }
 ```
 
@@ -160,7 +180,6 @@ impl<Op0, In0, Op1, In1> OperationRegistryBuilder<Op0, In0, Op1, In1> {
         Ok(OperationRegistry {
             operation0: self.operation0.ok_or(/* OperationRegistryBuilderError */)?,
             operation1: self.operation1.ok_or(/* OperationRegistryBuilderError */)?,
-            _phantom: PhantomData,
         })
     }
 }
@@ -199,31 +218,9 @@ where
 }
 ```
 
-A customer using this API would follow this kind of pattern:
-
-```rust
-async fn handler0(input: Operation0Input) -> Operation0Output {
-    todo!()
-}
-
-async fn handler1(input: Operation1Input) -> Operation1Output {
-    todo!()
-}
-
-let app: Router = OperationRegistryBuilder::default()
-    // Use the setters
-    .operation0(handler0)
-    .operation1(handler1)
-    // Convert to `OperationRegistry`
-    .build()
-    .unwrap()
-    // Convert to `Router`
-    .into();
-```
-
 ### Router
 
-The [router today](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/routing/mod.rs#L58-L60) exists as
+The [aws_smithy_http::routing::Router](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/routing/mod.rs#L58-L60) provides the protocol aware routing of requests to their target service, it exists as
 
 ```rust
 pub struct Route {
@@ -268,7 +265,7 @@ impl Service<http::Request> for Router
 }
 ```
 
-Along side the protocol specific constructors, `Router` includes a `layer` method. This provides a way for the customer to apply a `tower::Layer` to all routes. For every protocol, `Router::layer`, has the approximately the same behavior:
+Along side the protocol specific constructors, `Router` includes a `layer` method. This provides a way for the customer to apply a `tower::Layer` to all routes. For every protocol, `Router::layer` has the approximately the same behavior:
 
 ```rust
 let new_routes = old_routes
@@ -283,4 +280,95 @@ let new_routes = old_routes
 
 ### Comparison to Axum
 
-Historically, `smithy-rs` has borrowed from [axum](https://github.com/tokio-rs/axum). Despite various divergences the code bases still have much in common.
+Historically, `smithy-rs` has borrowed from [axum](https://github.com/tokio-rs/axum). Despite various divergences the code bases still have much in common:
+
+* Reliance on `Handler` trait to abstract over different closure signatures:
+  * [axum::handler::Handler](https://docs.rs/axum/latest/axum/handler/trait.Handler.html)
+  * [Handler](#handlers)
+* A mechanism for turning `H: Handler` into a `tower::Service`:
+  * [axum::handler::IntoService](https://docs.rs/axum/latest/axum/handler/struct.IntoService.html)
+  * [OperationHandler](#handlers)
+* A `Router` to route requests to various handlers:
+  * [axum::Router](https://docs.rs/axum/latest/axum/struct.Router.html)
+  * [aws_smithy_http_server::routing::Router](#router)
+
+To identify where the implementations should differ we should classify in what ways the use cases differ. There are two primary areas which we describe below.
+
+#### Extractors and Responses
+
+In `axum` there is a notion of [Extractor](https://docs.rs/axum/latest/axum/extract/index.html), which allows the customer to easily define a decomposition of an incoming `http::Request` by specify the arguments to the handlers. For example,
+
+```rust
+async fn request(Json(payload): Json<Value>, Query(params): Query<HashMap<String, String>>, headers: HeaderMap) {
+    todo!()
+}
+```
+
+is a valid handler - each argument satisfies the [axum::extract::FromRequest](https://docs.rs/axum/latest/axum/extract/trait.FromRequest.html) trait, therefore satisfies one of `axum` blanket `Handler` implementations:
+
+```rust
+macro_rules! impl_handler {
+    ( $($ty:ident),* $(,)? ) => {
+        impl<F, Fut, Res, $($ty,)*> Handler<($($ty,)*)> for F
+        where
+            F: FnOnce($($ty,)*) -> Fut + Clone + Send + 'static,
+            Fut: Future<Output = Res> + Send,
+            Res: IntoResponse,
+            $( $ty: FromRequest + Send,)*
+        {
+            fn call(self, req: http::Request) -> Self::Future {
+                async {
+                    let mut req = RequestParts::new(req);
+
+                    $(
+                        let $ty = match $ty::from_request(&mut req).await {
+                            Ok(value) => value,
+                            Err(rejection) => return rejection.into_response(),
+                        };
+                    )*
+
+                    let res = self($($ty,)*).await;
+
+                    res.into_response()
+                }
+            }
+        }
+    };
+}
+```
+
+The implementations of `Handler` in `axum` and `smithy-rs` follow a similar pattern - convert `http::Request` into the closures input, run the closure, convert the output of the closure to `http::Response`.
+
+In `smithy-rs` we do not need a notion of "extractor", that role is fulfilled by HTTP binding traits. In `smithy-rs` the `http::Request` decomposition is determined by the Smithy model and the service protocol, whereas in `axum` it's defined by the handlers signature. In `smithy-rs` the only remaining degree of freedom in the signature of the handler is whether or not state is included.
+
+Dual to `FromRequest` is the [axum::response::IntoResponse](https://docs.rs/axum/latest/axum/response/trait.IntoResponse.html) trait, this plays the role of converting the output of the handler to `http::Response`. Again, the difference between `axum` and `smithy-rs` is that `smithy-rs` has the conversion from `{Operation}Output` to `http::Response` specified by the Smithy model, whereas `axum` the customer is free to specify a return type which implements `axum::response::IntoResponse`.
+
+#### Routing
+
+The Smithy model not only specifies the `http::Request` decomposition and `http::Response` composition for a given service, it also determines the routing. The `From<OperationRegistry>` implementation, described in [Builder](#builder), yields a fully formed router based on the protocol and [http traits](https://awslabs.github.io/smithy/1.0/spec/core/http-traits.html#http-trait) specified.
+
+This is in contrast to `axum`, where the user decides the routing by various combinators included on the `axum::Router`. In an `axum` application one might encounter the following code:
+
+```rust
+let user_routes = Router::new().route("/:id", /* service */);
+
+let team_routes = Router::new().route("/", /* service */);
+
+let api_routes = Router::new()
+    .nest("/users", user_routes)
+    .nest("/teams", team_routes);
+
+let app = Router::new().nest("/api", api_routes);
+```
+
+Introducing state to handlers in `axum` is done in the same way as `smithy-rs`, described briefly in [Handlers](#handlers) - a layer is used to insert state into incoming `http::Request`s and the `Handler` implementation pops it out of the type map layer. In `axum`, if a customer wanted to scope state to all routes within `/users/` they are able to do the following:
+
+```rust
+async fn handler(Extension(state): Extension</* State */>) -> /* Return Type */ {}
+
+let api_routes = Router::new()
+    .nest("/users", user_routes.layer(Extension(/* state */)))
+    .nest("/teams", team_routes);
+```
+
+In `smithy-rs` a customer is only able to apply a layer to either the `aws_smithy_http::routing::Router` or every route via the [layer method](#router) described above.

From 41f88f195d6ab6deb3aa4d69af26a7fb6bc56278 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Fri, 5 Aug 2022 22:25:19 +0000
Subject: [PATCH 05/26] Fix typo

---
 design/src/rfcs/rfc0020_service_builder.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 0a9cf93e86..bf52a02a79 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -13,7 +13,7 @@ This RFC proposes a new builder, deprecating the existing one, which addresses A
 - **Service**: The `tower::Service` trait is an interface for writing network applications in a modular and reusable way. `Service`s act on requests to produce responses.
 - **Service Builder**: A `tower::Service` builder, generated from a Smithy service, by `smithy-rs`.
 - **Middleware**: Broadly speaking, middleware modify requests and responses. Concretely, these are exist as implementations of [Layer](https://docs.rs/tower/latest/tower/layer/trait.Layer.html)/a `Service` wrapping an inner `Service`.
-- **Handler**: A closure defining the behavior of a particular request after routing. These are provided to the service builder to complete the describe of the service.
+- **Handler**: A closure defining the behavior of a particular request after routing. These are provided to the service builder to complete the description of the service.
 
 ## Background
 

From 817afd8fa25e01a4b0b5ca7f154614356cd8ca8d Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Fri, 5 Aug 2022 23:10:39 +0000
Subject: [PATCH 06/26] Draft proposal

---
 design/src/rfcs/rfc0020_service_builder.md | 75 ++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index bf52a02a79..01a67b7f19 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -372,3 +372,78 @@ let api_routes = Router::new()
 ```
 
 In `smithy-rs` a customer is only able to apply a layer to either the `aws_smithy_http::routing::Router` or every route via the [layer method](#router) described above.
+
+# Proposal
+
+The proposal is presented as a series of transforms to the existing service builder, each paired with a motivation. Most of these can be independently implemented, and where there exists an interdependency it is stated.
+
+Although presented as a mutation to the existing service builder, the actual implementation should exist as a entirely separate builder - reusing code generation from the old builder but exposes a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
+
+## Remove `OperationRegistry`
+
+As described in [Builder](#builder), the customer is required to perform two conversions. One from `OperationRegistryBuilder` via `OperationRegistryBuilder::build`, the second from `OperationRegistryBuilder` to `Router` via the `From<OperationRegistry> for Router` implementation. The intermediary stop at `OperationRegistry` is not required and should be removed.
+
+## Statically check for missing Handlers
+
+As described in [Builder](#builder), the `OperationRegistryBuilder::build` method is fallible - it yields a runtime error when one of the handlers has not been set.
+
+```rust
+    pub fn build(
+        self,
+    ) -> Result<OperationRegistry<Op0, In0, Op1, In1>, OperationRegistryBuilderError> {
+        Ok(OperationRegistry {
+            operation0: self.operation0.ok_or(/* OperationRegistryBuilderError */)?,
+            operation1: self.operation1.ok_or(/* OperationRegistryBuilderError */)?,
+        })
+    }
+```
+
+We can do away with fallibility if we put bounds on `Op0`, `Op1`, etc which are not satisfied by the default type parameters. For example,
+
+```rust
+impl OperationRegistryBuilder<Op0, In0, Op1, In1> {
+    pub fn operation0<NewOp0>(mut self, value: NewOp0) -> OperationRegistryBuilder<NewOp0, In0, Op1, In1> {
+        OperationRegistryBuilder {
+            operation0: value,
+            operation1: self.operation1
+        }
+    }
+    pub fn operation1<NewOp1>(mut self, value: NewOp1) -> OperationRegistryBuilder<Op0, In0, NewOp1, In1> {
+        OperationRegistryBuilder {
+            operation0: self.operation0,
+            operation1: value
+        }
+    }
+}
+
+impl OperationRegistryBuilder<Op0, In0, Op1, In1>
+where
+    Op0: Handler<B, In0, Operation0Input>,
+    Op1: Handler<B, In1, Operation1Input>,
+{
+    pub fn build(self) -> OperationRegistry<Op0, In0, Op1, In1> {
+        OperationRegistry {
+            operation0: self.operation0,
+            operation1: self.operation1,
+        }
+    }
+}
+```
+
+The customer will now get a compile time error rather than a runtime error when they fail to specify a handler.
+
+## Switch `From<OperationRegistry> for Router` to a `OperationRegistry::build` method
+
+## Eagerly convert to Service in the service builder
+
+### Add `layer_all` method to service builder
+
+### Add `{operation}_layer` methods to service builder
+
+## Blanket implementation `Handler`
+
+## Protocol specific routers
+
+## Protocol specific errors
+
+## Type erasure with the name of the Smithy service

From 7d9d89c8b5e453ce71a7191800c0f5d1c54f3847 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Mon, 8 Aug 2022 13:45:49 +0000
Subject: [PATCH 07/26] Proposal draft

---
 design/src/rfcs/rfc0020_service_builder.md | 117 +++++++++++++++++++--
 1 file changed, 108 insertions(+), 9 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 01a67b7f19..46225f1f14 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -347,7 +347,7 @@ Dual to `FromRequest` is the [axum::response::IntoResponse](https://docs.rs/axum
 
 The Smithy model not only specifies the `http::Request` decomposition and `http::Response` composition for a given service, it also determines the routing. The `From<OperationRegistry>` implementation, described in [Builder](#builder), yields a fully formed router based on the protocol and [http traits](https://awslabs.github.io/smithy/1.0/spec/core/http-traits.html#http-trait) specified.
 
-This is in contrast to `axum`, where the user decides the routing by various combinators included on the `axum::Router`. In an `axum` application one might encounter the following code:
+This is in contrast to `axum`, where the user specifies the routing by use of various combinators included on the `axum::Router`, applied to other `tower::Service`s. In an `axum` application one might encounter the following code:
 
 ```rust
 let user_routes = Router::new().route("/:id", /* service */);
@@ -361,6 +361,8 @@ let api_routes = Router::new()
 let app = Router::new().nest("/api", api_routes);
 ```
 
+Note that, in `axum` handlers are quickly converted to `tower::Service` (via `IntoService`) before they are passed into the `Router`. In contrast, in `smithy-rs` handlers are passed into a builder and then the conversion to `tower::Service` is performed (via `OperationHandler`).
+
 Introducing state to handlers in `axum` is done in the same way as `smithy-rs`, described briefly in [Handlers](#handlers) - a layer is used to insert state into incoming `http::Request`s and the `Handler` implementation pops it out of the type map layer. In `axum`, if a customer wanted to scope state to all routes within `/users/` they are able to do the following:
 
 ```rust
@@ -375,13 +377,13 @@ In `smithy-rs` a customer is only able to apply a layer to either the `aws_smith
 
 # Proposal
 
-The proposal is presented as a series of transforms to the existing service builder, each paired with a motivation. Most of these can be independently implemented, and where there exists an interdependency it is stated.
+The proposal is presented as a series of compatible transforms to the existing service builder, each paired with a motivation. Most of these can be independently implemented, but in the case where there exists an interdependency it is stated.
 
 Although presented as a mutation to the existing service builder, the actual implementation should exist as a entirely separate builder - reusing code generation from the old builder but exposes a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
 
-## Remove `OperationRegistry`
+## Remove two-step build procedure
 
-As described in [Builder](#builder), the customer is required to perform two conversions. One from `OperationRegistryBuilder` via `OperationRegistryBuilder::build`, the second from `OperationRegistryBuilder` to `Router` via the `From<OperationRegistry> for Router` implementation. The intermediary stop at `OperationRegistry` is not required and should be removed.
+As described in [Builder](#builder), the customer is required to perform two conversions. One from `OperationRegistryBuilder` via `OperationRegistryBuilder::build`, the second from `OperationRegistryBuilder` to `Router` via the `From<OperationRegistry> for Router` implementation. The intermediary stop at `OperationRegistry` is not required and can be removed.
 
 ## Statically check for missing Handlers
 
@@ -398,7 +400,7 @@ As described in [Builder](#builder), the `OperationRegistryBuilder::build` metho
     }
 ```
 
-We can do away with fallibility if we put bounds on `Op0`, `Op1`, etc which are not satisfied by the default type parameters. For example,
+We can do away with fallibility if we allow for on `Op0`, `Op1`, to switch types during build. For example,
 
 ```rust
 impl OperationRegistryBuilder<Op0, In0, Op1, In1> {
@@ -434,13 +436,110 @@ The customer will now get a compile time error rather than a runtime error when
 
 ## Switch `From<OperationRegistry> for Router` to a `OperationRegistry::build` method
 
-## Eagerly convert to Service in the service builder
+To construct a `Router`, the customer must either give a type ascription
+
+```rust
+let app: Router = /* Service builder */.into();
+```
+
+or be explicit about the `Router` namespace
+
+```rust
+let app = Router::from(/* Service builder */);
+```
+
+If we switch from a `From<OperationRegistry> for Router` to a `build` method on `OperationRegistry` the customer may simply
+
+```rust
+let app = /* Service builder */.build();
+```
+
+## Operations as Middleware
+
+As mentioned in [Comparison to Axum: Routing](#routing) and [Handlers](#handlers), the `smithy-rs` service builder accepts handlers and only converts them into a `tower::Service` during the final conversion into a `Router`. There are downsides to this:
+
+- The customer has no opportunity to apply middleware to a specific operation before they are all collected into `Router`. The `Router` does have a `layer` method, described in [Router](#router), but this applies the middleware uniformly across all operations.
+- The builder has no way to apply middleware around customer applied middleware. A concrete example of where this would be useful is described in the [Middleware Position](rfc0018_logging_sensitive.md#middleware-position) section of [RFC: Logging in the Presence of Sensitive Data](rfc0018_logging_sensitive.md).
+- The customer has no way of expressing readiness of the underlying operation - all handlers are converted to services with [Service::poll_ready](https://docs.rs/tower/latest/tower/trait.Service.html#tymethod.poll_ready) returning `Poll::Ready(Ok(()))`.
+
+There are two points at which the customer might want to apply middleware: around `tower::Service<{Operation}Input, Response = {Operation}Output>` and `tower::Service<http::Response, Response = http::Response>`, that is, before and after the serialization/deserialization is performed.
+
+The three use cases described above are supported by `axum` by virtue of the [Router::route](https://docs.rs/axum/latest/axum/routing/struct.Router.html#method.route) method accepting a `tower::Service`. The reader should consider a similar approach where the service builder setters accept a `tower::Service<http::Request, Response = http::Response>` rather than the `Handler`. In this way the we separate the building of the entire service from the building of individual operations, this leads us to one of the central tenets of this proposal: operations as middleware. More concretely, the customer is provided with the following generated code
+
+```rust
+struct Operation0<S> {
+    inner: S,
+}
+
+impl<S> Service<http::Request> for Operation0<S>
+where
+    S: Service<Operation0Input, Response = Operation0Output, Error = Infallible>
+{
+    type Response = http::Response;
+    type Error = Infallible;
+
+    fn poll_ready(&mut self, cx: &mut Context) -> Poll<Result<(), Self::Error>> {
+        // We defer to the inner service for readiness
+        self.inner.poll_ready(cx)
+    }
+
+    async fn call(&mut self, request: http::Request) -> Result<Self::Response, Self::Error> {
+        let input = /* Create `Operation0Input` from `request: http::Request` */;
+
+        self.inner.call(input).await;
 
-### Add `layer_all` method to service builder
+        let response = /* Create `http::Response` from `output: Operation0Output` */
+        response
+    }
+}
 
-### Add `{operation}_layer` methods to service builder
+fn make_operation_0<S>(inner: S) -> Operation0<S> {
+    Operation0 {
+        inner
+    }
+}
+```
+
+which is concerned with all the same model aware serialization/deserialization we noted in [Handler](#handler). The API usage then becomes
+
+```rust
+async fn handler0(input: Operation0Input) -> Operation0Output {
+    todo!()
+}
 
-## Blanket implementation `Handler`
+async fn handler1(input: Operation1Input, state: State) -> Operation1Output {
+    todo!()
+}
+
+// Model agnostic `into_service` method which transforms a handler `{Operation}Input -> {Operation}Output` to
+// `tower::Service<{Operation}Input, Response = {Operation}Output, Error = Infallible>`
+let svc = OperationHandler::new(handler0);
+
+// Middleware can be applied at this point
+let svc = model_layer.layer(svc);
+
+// Model aware conversion between `tower::Service<{Operation}Input, Response = {Operation}Output, Error = Infallible>`
+// and `tower::Service<http::Request, Response = http::Response, Error = Infallible>`
+let operation0 = make_operation0(svc);
+
+// Middleware can be applied at this point
+let operation0 = http_layer.layer(op1_svc);
+
+OperationRegistryBuilder::default()
+    .operation0(operation0)
+    .operation1(make_operation1(OperationHandler::new(handler1)))
+    /* ... */
+```
+
+Note that this requires that the `OperationRegistryBuilder` stores services, rather than `Handler`s. An unintended and superficial benefit of this is that we are able to drop `In{n}` from the `OperationRegistryBuilder<Op0, In0, Op1, In1>`, only `Op{n}` remains and it parametrizes each operations `tower::Service`.
+
+It is still possible to retain the original API which accepts `Handler` by introducing the following setters
+
+```rust
+fn operation0_handler<H: Handler>(self, handler: H) -> OperationRegistryBuilder<Operation> {
+    self.operation0(make_operation0(handler.into_service()))
+}
+```
 
 ## Protocol specific routers
 

From 2718cd0c36871da3d38e4e02f468a4b2b90be4dd Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Mon, 8 Aug 2022 16:51:55 +0000
Subject: [PATCH 08/26] Proposal draft

---
 design/src/rfcs/rfc0020_service_builder.md | 96 +++++++++++-----------
 1 file changed, 50 insertions(+), 46 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 46225f1f14..9b72db0e15 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -284,7 +284,7 @@ Historically, `smithy-rs` has borrowed from [axum](https://github.com/tokio-rs/a
 
 * Reliance on `Handler` trait to abstract over different closure signatures:
   * [axum::handler::Handler](https://docs.rs/axum/latest/axum/handler/trait.Handler.html)
-  * [Handler](#handlers)
+  * [Handlers](#handlers)
 * A mechanism for turning `H: Handler` into a `tower::Service`:
   * [axum::handler::IntoService](https://docs.rs/axum/latest/axum/handler/struct.IntoService.html)
   * [OperationHandler](#handlers)
@@ -458,13 +458,56 @@ let app = /* Service builder */.build();
 
 As mentioned in [Comparison to Axum: Routing](#routing) and [Handlers](#handlers), the `smithy-rs` service builder accepts handlers and only converts them into a `tower::Service` during the final conversion into a `Router`. There are downsides to this:
 
-- The customer has no opportunity to apply middleware to a specific operation before they are all collected into `Router`. The `Router` does have a `layer` method, described in [Router](#router), but this applies the middleware uniformly across all operations.
-- The builder has no way to apply middleware around customer applied middleware. A concrete example of where this would be useful is described in the [Middleware Position](rfc0018_logging_sensitive.md#middleware-position) section of [RFC: Logging in the Presence of Sensitive Data](rfc0018_logging_sensitive.md).
-- The customer has no way of expressing readiness of the underlying operation - all handlers are converted to services with [Service::poll_ready](https://docs.rs/tower/latest/tower/trait.Service.html#tymethod.poll_ready) returning `Poll::Ready(Ok(()))`.
+1. The customer has no opportunity to apply middleware to a specific operation before they are all collected into `Router`. The `Router` does have a `layer` method, described in [Router](#router), but this applies the middleware uniformly across all operations.
+2. The builder has no way to apply middleware around customer applied middleware. A concrete example of where this would be useful is described in the [Middleware Position](rfc0018_logging_sensitive.md#middleware-position) section of [RFC: Logging in the Presence of Sensitive Data](rfc0018_logging_sensitive.md).
+3. The customer has no way of expressing readiness of the underlying operation - all handlers are converted to services with [Service::poll_ready](https://docs.rs/tower/latest/tower/trait.Service.html#tymethod.poll_ready) returning `Poll::Ready(Ok(()))`.
 
-There are two points at which the customer might want to apply middleware: around `tower::Service<{Operation}Input, Response = {Operation}Output>` and `tower::Service<http::Response, Response = http::Response>`, that is, before and after the serialization/deserialization is performed.
+The three use cases described above are supported by `axum` by virtue of the [Router::route](https://docs.rs/axum/latest/axum/routing/struct.Router.html#method.route) method accepting a `tower::Service`. The reader should consider a similar approach where the service builder setters accept a `tower::Service<http::Request, Response = http::Response>` rather than the `Handler`.
 
-The three use cases described above are supported by `axum` by virtue of the [Router::route](https://docs.rs/axum/latest/axum/routing/struct.Router.html#method.route) method accepting a `tower::Service`. The reader should consider a similar approach where the service builder setters accept a `tower::Service<http::Request, Response = http::Response>` rather than the `Handler`. In this way the we separate the building of the entire service from the building of individual operations, this leads us to one of the central tenets of this proposal: operations as middleware. More concretely, the customer is provided with the following generated code
+The smallest changeset to make progress towards these problems would require the customer eagerly uses `OperationHandler::new` rather than it being applied internally within `From<OperationRegistry> for Router` (see [Handlers](#handlers)). The usage would become
+
+```rust
+async fn handler0(input: Operation0Input) -> Operation0Output {
+    todo!()
+}
+
+async fn handler1(input: Operation1Input, state: State) -> Operation1Output {
+    todo!()
+}
+
+// Create a `Service<http::Request, Response = http::Response, Error = Infallible>` eagerly
+let svc = OperationHandler::new(handler0);
+
+// Middleware can be applied at this point
+let operation0 = http_layer.layer(op1_svc);
+
+let operation1 = OperationHandler::new(handler1);
+
+OperationRegistryBuilder::default()
+    .operation0(operation0)
+    .operation1(operation1)
+    /* ... */
+```
+
+Note that this requires that the `OperationRegistryBuilder` stores services, rather than `Handler`s. An unintended and superficial benefit of this is that we are able to drop `In{n}` from the `OperationRegistryBuilder<Op0, In0, Op1, In1>`, only `Op{n}` remains and it parametrizes each operations `tower::Service`.
+
+It is still possible to retain the original API which accepts `Handler` by introducing the following setters:
+
+```rust
+impl<Op1, Op2> OperationRegistry<Op1, Op2> {
+    fn operation0_handler<H: Handler>(self, handler: H) -> OperationRegistryBuilder<OperationHandler<H>, Op2> {
+        self.operation0(OperationHandler::new(handler))
+    }
+}
+```
+
+There are two points at which the customer might want to apply middleware: around `tower::Service<{Operation}Input, Response = {Operation}Output>` and `tower::Service<http::Response, Response = http::Response>`, that is, before and after the model aware serialization/deserialization is performed. The change described only succeeds in the later, and therefore is only a partial solution to (1).
+
+This solves (2), the service builder may apply additional middleware around the service.
+
+This does not solve (3), as the customer is not permitted to provide a `Service`.
+
+<!-- In this way the we separate the building of the entire service from the building of individual operations, this leads us to one of the central tenets of this proposal: operations as middleware. More concretely, the customer is provided with the following generated code
 
 ```rust
 struct Operation0<S> {
@@ -500,46 +543,7 @@ fn make_operation_0<S>(inner: S) -> Operation0<S> {
 }
 ```
 
-which is concerned with all the same model aware serialization/deserialization we noted in [Handler](#handler). The API usage then becomes
-
-```rust
-async fn handler0(input: Operation0Input) -> Operation0Output {
-    todo!()
-}
-
-async fn handler1(input: Operation1Input, state: State) -> Operation1Output {
-    todo!()
-}
-
-// Model agnostic `into_service` method which transforms a handler `{Operation}Input -> {Operation}Output` to
-// `tower::Service<{Operation}Input, Response = {Operation}Output, Error = Infallible>`
-let svc = OperationHandler::new(handler0);
-
-// Middleware can be applied at this point
-let svc = model_layer.layer(svc);
-
-// Model aware conversion between `tower::Service<{Operation}Input, Response = {Operation}Output, Error = Infallible>`
-// and `tower::Service<http::Request, Response = http::Response, Error = Infallible>`
-let operation0 = make_operation0(svc);
-
-// Middleware can be applied at this point
-let operation0 = http_layer.layer(op1_svc);
-
-OperationRegistryBuilder::default()
-    .operation0(operation0)
-    .operation1(make_operation1(OperationHandler::new(handler1)))
-    /* ... */
-```
-
-Note that this requires that the `OperationRegistryBuilder` stores services, rather than `Handler`s. An unintended and superficial benefit of this is that we are able to drop `In{n}` from the `OperationRegistryBuilder<Op0, In0, Op1, In1>`, only `Op{n}` remains and it parametrizes each operations `tower::Service`.
-
-It is still possible to retain the original API which accepts `Handler` by introducing the following setters
-
-```rust
-fn operation0_handler<H: Handler>(self, handler: H) -> OperationRegistryBuilder<Operation> {
-    self.operation0(make_operation0(handler.into_service()))
-}
-```
+which is concerned with all the same model aware serialization/deserialization we noted in [Handlers](#handlers). -->
 
 ## Protocol specific routers
 

From 20cf454c77658ca96ea0e4b979caffae8dc5a4b0 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Tue, 9 Aug 2022 13:19:17 +0000
Subject: [PATCH 09/26] Proposal draft

---
 design/src/rfcs/rfc0020_service_builder.md | 219 ++++++++++++++++++---
 1 file changed, 194 insertions(+), 25 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 9b72db0e15..a82f5ed486 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -9,7 +9,7 @@ This RFC proposes a new builder, deprecating the existing one, which addresses A
 ## Terminology
 
 - **Model**: A [Smithy Model](https://awslabs.github.io/smithy/1.0/spec/core/model.html), usually pertaining to the one in use by the customer.
-- **Smithy Service**: The entry point of an API that aggregates resources and operations together within a Smithy model. Described in detail [here](https://awslabs.github.io/smithy/1.0/spec/core/model.html#service).
+- **Smithy Service**: The entry point of an API that aggregates [resources](https://awslabs.github.io/smithy/1.0/spec/core/model.html#resource) and [operations](https://awslabs.github.io/smithy/1.0/spec/core/model.html#operation) together within a Smithy model. Described in detail [here](https://awslabs.github.io/smithy/1.0/spec/core/model.html#service).
 - **Service**: The `tower::Service` trait is an interface for writing network applications in a modular and reusable way. `Service`s act on requests to produce responses.
 - **Service Builder**: A `tower::Service` builder, generated from a Smithy service, by `smithy-rs`.
 - **Middleware**: Broadly speaking, middleware modify requests and responses. Concretely, these are exist as implementations of [Layer](https://docs.rs/tower/latest/tower/layer/trait.Layer.html)/a `Service` wrapping an inner `Service`.
@@ -32,7 +32,7 @@ operation Operation1 {
     output: Output1
 }
 
-service Service {
+service Service0 {
     operations: [
         Operation0,
         Operation1,
@@ -304,7 +304,7 @@ async fn request(Json(payload): Json<Value>, Query(params): Query<HashMap<String
 }
 ```
 
-is a valid handler - each argument satisfies the [axum::extract::FromRequest](https://docs.rs/axum/latest/axum/extract/trait.FromRequest.html) trait, therefore satisfies one of `axum` blanket `Handler` implementations:
+is a valid handler - each argument satisfies the [axum::extract::FromRequest](https://docs.rs/axum/latest/axum/extract/trait.FromRequest.html) trait, therefore satisfies one of `axum`s blanket `Handler` implementations:
 
 ```rust
 macro_rules! impl_handler {
@@ -339,7 +339,7 @@ macro_rules! impl_handler {
 
 The implementations of `Handler` in `axum` and `smithy-rs` follow a similar pattern - convert `http::Request` into the closures input, run the closure, convert the output of the closure to `http::Response`.
 
-In `smithy-rs` we do not need a notion of "extractor", that role is fulfilled by HTTP binding traits. In `smithy-rs` the `http::Request` decomposition is determined by the Smithy model and the service protocol, whereas in `axum` it's defined by the handlers signature. In `smithy-rs` the only remaining degree of freedom in the signature of the handler is whether or not state is included.
+In `smithy-rs` we do not need a notion of "extractor", that role is fulfilled by the model, including HTTP binding traits. In `smithy-rs` the `http::Request` decomposition is determined by the Smithy model and the service protocol, whereas in `axum` it's defined by the handlers signature. In `smithy-rs` the only remaining degree of freedom in the signature of the handler is whether or not state is included.
 
 Dual to `FromRequest` is the [axum::response::IntoResponse](https://docs.rs/axum/latest/axum/response/trait.IntoResponse.html) trait, this plays the role of converting the output of the handler to `http::Response`. Again, the difference between `axum` and `smithy-rs` is that `smithy-rs` has the conversion from `{Operation}Output` to `http::Response` specified by the Smithy model, whereas `axum` the customer is free to specify a return type which implements `axum::response::IntoResponse`.
 
@@ -361,7 +361,7 @@ let api_routes = Router::new()
 let app = Router::new().nest("/api", api_routes);
 ```
 
-Note that, in `axum` handlers are quickly converted to `tower::Service` (via `IntoService`) before they are passed into the `Router`. In contrast, in `smithy-rs` handlers are passed into a builder and then the conversion to `tower::Service` is performed (via `OperationHandler`).
+Note that, in `axum` handlers are eagerly converted to a `tower::Service` (via `IntoService`) before they are passed into the `Router`. In contrast, in `smithy-rs` handlers are passed into a builder and then the conversion to `tower::Service` is performed (via `OperationHandler`).
 
 Introducing state to handlers in `axum` is done in the same way as `smithy-rs`, described briefly in [Handlers](#handlers) - a layer is used to insert state into incoming `http::Request`s and the `Handler` implementation pops it out of the type map layer. In `axum`, if a customer wanted to scope state to all routes within `/users/` they are able to do the following:
 
@@ -379,7 +379,7 @@ In `smithy-rs` a customer is only able to apply a layer to either the `aws_smith
 
 The proposal is presented as a series of compatible transforms to the existing service builder, each paired with a motivation. Most of these can be independently implemented, but in the case where there exists an interdependency it is stated.
 
-Although presented as a mutation to the existing service builder, the actual implementation should exist as a entirely separate builder - reusing code generation from the old builder but exposes a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
+Although presented as a mutation to the existing service builder, the actual implementation should exist as a entirely separate builder - reusing code generation from the old builder while exposing a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
 
 ## Remove two-step build procedure
 
@@ -454,7 +454,7 @@ If we switch from a `From<OperationRegistry> for Router` to a `build` method on
 let app = /* Service builder */.build();
 ```
 
-## Operations as Middleware
+## Operations as Middleware Constructors
 
 As mentioned in [Comparison to Axum: Routing](#routing) and [Handlers](#handlers), the `smithy-rs` service builder accepts handlers and only converts them into a `tower::Service` during the final conversion into a `Router`. There are downsides to this:
 
@@ -464,14 +464,31 @@ As mentioned in [Comparison to Axum: Routing](#routing) and [Handlers](#handlers
 
 The three use cases described above are supported by `axum` by virtue of the [Router::route](https://docs.rs/axum/latest/axum/routing/struct.Router.html#method.route) method accepting a `tower::Service`. The reader should consider a similar approach where the service builder setters accept a `tower::Service<http::Request, Response = http::Response>` rather than the `Handler`.
 
-The smallest changeset to make progress towards these problems would require the customer eagerly uses `OperationHandler::new` rather than it being applied internally within `From<OperationRegistry> for Router` (see [Handlers](#handlers)). The usage would become
+Throughout this section we purposely ignore the existence of handlers accepting state alongside the `{Operation}Input`, this class of handlers serve as a distraction and can be handled with small perturbations from each approach.
+
+### Approach A: Customer uses `OperationHandler::new`
+
+It's possible to make progress with a small changeset, by requiring the customer eagerly uses `OperationHandler::new` rather than it being applied internally within `From<OperationRegistry> for Router` (see [Handlers](#handlers)). The setter would then become:
 
 ```rust
-async fn handler0(input: Operation0Input) -> Operation0Output {
-    todo!()
+pub struct OperationRegistryBuilder<Op0, Op1> {
+    operation1: Option<Op0>,
+    operation2: Option<Op1>
+}
+
+impl<Op0, Op1> OperationRegistryBuilder<Op0, Op1> {
+    pub fn operation0(self, value: Op0) -> Self {
+        self.operation1 = Some(value);
+        self
+    }
 }
+```
+
 
-async fn handler1(input: Operation1Input, state: State) -> Operation1Output {
+The API usage would then become
+
+```rust
+async fn handler0(input: Operation0Input) -> Operation0Output {
     todo!()
 }
 
@@ -481,11 +498,8 @@ let svc = OperationHandler::new(handler0);
 // Middleware can be applied at this point
 let operation0 = http_layer.layer(op1_svc);
 
-let operation1 = OperationHandler::new(handler1);
-
 OperationRegistryBuilder::default()
     .operation0(operation0)
-    .operation1(operation1)
     /* ... */
 ```
 
@@ -496,21 +510,26 @@ It is still possible to retain the original API which accepts `Handler` by intro
 ```rust
 impl<Op1, Op2> OperationRegistry<Op1, Op2> {
     fn operation0_handler<H: Handler>(self, handler: H) -> OperationRegistryBuilder<OperationHandler<H>, Op2> {
-        self.operation0(OperationHandler::new(handler))
+        OperationRegistryBuilder {
+            operation0: OperationHandler::new(handler),
+            operation1: self.operation1
+        }
     }
 }
 ```
 
-There are two points at which the customer might want to apply middleware: around `tower::Service<{Operation}Input, Response = {Operation}Output>` and `tower::Service<http::Response, Response = http::Response>`, that is, before and after the model aware serialization/deserialization is performed. The change described only succeeds in the later, and therefore is only a partial solution to (1).
+There are two points at which the customer might want to apply middleware: around `tower::Service<{Operation}Input, Response = {Operation}Output>` and `tower::Service<http::Response, Response = http::Response>`, that is, before and after the serialization/deserialization is performed. The change described only succeeds in the later, and therefore is only a partial solution to (1).
 
 This solves (2), the service builder may apply additional middleware around the service.
 
-This does not solve (3), as the customer is not permitted to provide a `Service`.
+This does not solve (3), as the customer is not able to provide a `tower::Service<{Operation}Input, Response = {Operation}Output>`.
+
+### Approach B: Operations as Middleware
 
-<!-- In this way the we separate the building of the entire service from the building of individual operations, this leads us to one of the central tenets of this proposal: operations as middleware. More concretely, the customer is provided with the following generated code
+In order to achieve all three we model operations as middleware:
 
 ```rust
-struct Operation0<S> {
+pub struct Operation0<S> {
     inner: S,
 }
 
@@ -535,18 +554,168 @@ where
         response
     }
 }
+```
+
+Notice the similarity between this and the `OperationHandler`, the only real difference being that we hold an inner service rather than a closure. In this way we have separated all model aware serialization/deserialization, we noted in [Handlers](#handlers), into this middleware.
+
+A consequence of this is that the user `Operation0` must have two constructors:
+
+- `from_service`, which takes a `tower::Service<Operation0Input, Response = Operation0Output>`.
+- `from_handler`, which takes an async `Operation0Input -> Operation0Output`.
+
+A brief example of how this might look:
+
+```rust
+use tower::util::{ServiceFn, service_fn};
+
+impl<S> Operation0<S> {
+    pub fn from_service(inner: S) -> Self {
+        Self {
+            inner,
+        }
+    }
+}
 
-fn make_operation_0<S>(inner: S) -> Operation0<S> {
-    Operation0 {
-        inner
+impl<F> Operation0<ServiceFn<F>> {
+    pub fn from_handler(inner: F) -> Self {
+        // Using `service_fn` here isn't strictly correct - there is slight misalignment of closure signatures. This
+        // still serves to illustrate the proposal.
+        Operation0::from_service(service_fn(inner))
     }
 }
 ```
 
-which is concerned with all the same model aware serialization/deserialization we noted in [Handlers](#handlers). -->
+The API usage then becomes:
+
+```rust
+async fn handler(input: Operation0Input) -> Operation0Output {
+    todo!()
+}
+
+// These are both `tower::Service` and hence can have middleware applied to them
+let operation_0 = Operation0::from_handler(handler);
+let operation_1 = Operation1::from_service(/* some service */);
+
+OperationRegistryBuilder::default()
+    .operation0(operation_0)
+    .operation1(operation_1)
+    /* ... */
+```
+
+### Approach C: Operations as Middleware Constructors
+
+While [Attempt B](#approach-b-operations-as-middleware) solves all three problems, it fails to adequately model the semantics Smithy. Note that an operation cannot uniquely define a `tower::Service` without reference to a parent Smithy service - information concerning the serialization/deserialization, error modes are all inherited from the Smithy service it used within. In this way, `Operation0` should not be a standalone middleware, but become middleware once accepted by the service builder.
+
+Any solution which provides an `{Operation}` structure and wishes it to be accepted by multiple service builders must deal with this problem. We currently build one library per service and hence have duplicate structures when [service closures](https://awslabs.github.io/smithy/1.0/spec/core/model.html?highlight=closure#service-closure) overlap. This means we wouldn't run into this problem today, but it would be a future obstruction if we wanted to reduce the amount of generated code.
+
+```rust
+use tower::layer::util::{Stack, Identity};
+use tower::util::{ServiceFn, service_fn};
+
+// This takes the same form as `Operation0` defined in the previous attempt. The difference being that this is now
+// private.
+struct Service0Operation0<S> {
+    inner: S
+}
+
+impl<S> Service<http::Request> for ServiceOperation0<S>
+where
+    S: Service<Operation0Input, Response = Operation0Output, Error = Infallible>
+{
+    /* Same as above */
+}
+
+pub struct Operation0<S, L> {
+    inner: S,
+    layer: L
+}
+
+impl<S> Operation0<S, Identity> {
+    pub fn from_service(inner: S) -> Self {
+        Self {
+            inner,
+            layer: Identity
+        }
+    }
+}
+
+impl<F> Operation0<ServiceFn<F>, Identity> {
+    pub fn from_handler(inner: F) -> Self {
+        Operation0::from_service(service_fn(inner))
+    }
+}
+
+impl<S, L> Operation0<S, L> {
+    pub fn layer<NewL>(self, layer: L) -> Operation0<S, Stack<L, NewL>> {
+        Operation0 {
+            inner: self.inner,
+            layer: Stack::new(self.layer, layer)
+        }
+    }
+
+    pub fn logging(self, /* args */) -> Operation0<S, Stack<L, LoggingLayer>> {
+        Operation0 {
+            inner: self.inner,
+            layer: Stack::new(self.layer, LoggingLayer::new(/* args */))
+        }
+    }
+
+    pub fn auth(self, /* args */) -> Operation0<S, Stack<L, AuthLayer>> {
+        Operation0 {
+            inner: self.inner,
+            layer: Stack::new(self.layer, /* Construct auth middleware */)
+        }
+
+    }
+}
+
+impl<Op1, Op2> OperationRegistryBuilder<Op1, Op2> {
+    pub fn operation0<S, L>(self, operation: Operation0<S, L>) -> OperationRegistryBuilder<<L as Layer<Service0Operation0<S>>::Service, Op2>
+    where
+        L: Layer<Service0Operation0<S>>
+    {
+        // Convert `Operation0` to a `tower::Service`.
+        let http_svc = Service0Operation0 { inner: operation.inner };
+        // Apply the layers
+        operation.layer(http_svc)
+    }
+}
+```
+
+Notice that we get some additional type safety here when compared to [Approach A](#approach-a-customer-uses-operationhandlernew) and [Approach B](#approach-b-operations-as-middleware) - `operation0` accepts a `Operation0` rather than a general `tower::Service`. We also get a namespace to include utility methods - notice the `logging` and `auth` methods.
+
+The RFC favours this approach out of all those presented.
+
+### Approach D: Add more methods to the Service Builder
+
+An alternative to [Approach C](#approach-c-operations-as-middleware-constructors) is to simply add more methods to the service builder while internally storing a `tower::Service`:
+
+- `operation0_from_service`, accepts a `tower::Service<Operation0Input, Response = Operation0Output>`.
+- `operation0_from_handler`, accepts an async `Fn(Operation0Input) -> Operation0Output`.
+- `operation0_from_handler_with_state`, accepts an async `Fn(Operation0Input, State) -> Operation0Output` and a `State`.
+- `operation0_layer`, accepts a `tower::Layer<Op0>`.
+
+This is functionally similar to [Attempt C](#approach-c-operations-as-middleware-constructors) except that all composition is done internal to the service builder and the namespace exists in the method name, rather than the `{Operation}` struct.
+
+## Service generic Routers
+
+Currently the `Router` stores `Box<dyn tower::Service<http::Request, Response = http::Response>`. As a result the `Router::layer` method, seen in [Router](#router), must re-box a service after every `tower::Layer` applied. The heap allocation `Box::new` itself is not cause for concern because `Router`s are typically constructed once at startup, however one might expect the indirection to regress performance.
+
+Having the service type parameterized, `Router<S>`, allows us to write:
+
+```rust
+impl<S> Router<S> {
+    fn layer<L>(self, layer: &L) -> Router<L::Service>
+    where
+        L: Layer<S>
+    {
+        /* Same internal implementation without boxing */
+    }
+}
+```
 
-## Protocol specific routers
+## Protocol specific Routers
 
-## Protocol specific errors
+## Protocol specific Errors
 
 ## Type erasure with the name of the Smithy service

From 3c895fcfce2d91a0e6ade809556a768ce6bd3f23 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Tue, 9 Aug 2022 13:42:52 +0000
Subject: [PATCH 10/26] Small fixes

---
 design/src/rfcs/rfc0020_service_builder.md | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index a82f5ed486..2fa5bd9c42 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -484,7 +484,6 @@ impl<Op0, Op1> OperationRegistryBuilder<Op0, Op1> {
 }
 ```
 
-
 The API usage would then become
 
 ```rust
@@ -503,7 +502,7 @@ OperationRegistryBuilder::default()
     /* ... */
 ```
 
-Note that this requires that the `OperationRegistryBuilder` stores services, rather than `Handler`s. An unintended and superficial benefit of this is that we are able to drop `In{n}` from the `OperationRegistryBuilder<Op0, In0, Op1, In1>`, only `Op{n}` remains and it parametrizes each operations `tower::Service`.
+Note that this requires that the `OperationRegistryBuilder` stores services, rather than `Handler`s. An unintended and superficial benefit of this is that we are able to drop `In{n}` from the `OperationRegistryBuilder<Op0, In0, Op1, In1>` - only `Op{n}` remains and it parametrizes each operations `tower::Service`.
 
 It is still possible to retain the original API which accepts `Handler` by introducing the following setters:
 
@@ -604,7 +603,7 @@ OperationRegistryBuilder::default()
 
 ### Approach C: Operations as Middleware Constructors
 
-While [Attempt B](#approach-b-operations-as-middleware) solves all three problems, it fails to adequately model the semantics Smithy. Note that an operation cannot uniquely define a `tower::Service` without reference to a parent Smithy service - information concerning the serialization/deserialization, error modes are all inherited from the Smithy service it used within. In this way, `Operation0` should not be a standalone middleware, but become middleware once accepted by the service builder.
+While [Attempt B](#approach-b-operations-as-middleware) solves all three problems, it fails to adequately model the Smithy semantics. An operation cannot uniquely define a `tower::Service` without reference to a parent Smithy service - information concerning the serialization/deserialization, error modes are all inherited from the Smithy service an operation is used within. In this way, `Operation0` should not be a standalone middleware, but become middleware once accepted by the service builder.
 
 Any solution which provides an `{Operation}` structure and wishes it to be accepted by multiple service builders must deal with this problem. We currently build one library per service and hence have duplicate structures when [service closures](https://awslabs.github.io/smithy/1.0/spec/core/model.html?highlight=closure#service-closure) overlap. This means we wouldn't run into this problem today, but it would be a future obstruction if we wanted to reduce the amount of generated code.
 
@@ -697,11 +696,11 @@ An alternative to [Approach C](#approach-c-operations-as-middleware-constructors
 
 This is functionally similar to [Attempt C](#approach-c-operations-as-middleware-constructors) except that all composition is done internal to the service builder and the namespace exists in the method name, rather than the `{Operation}` struct.
 
-## Service generic Routers
+## Service parameterized Routers
 
 Currently the `Router` stores `Box<dyn tower::Service<http::Request, Response = http::Response>`. As a result the `Router::layer` method, seen in [Router](#router), must re-box a service after every `tower::Layer` applied. The heap allocation `Box::new` itself is not cause for concern because `Router`s are typically constructed once at startup, however one might expect the indirection to regress performance.
 
-Having the service type parameterized, `Router<S>`, allows us to write:
+Having the service type parameterized as `Router<S>`, allows us to write:
 
 ```rust
 impl<S> Router<S> {

From 1a17e52758173f22ddee7a000c5acb3d5a17aca9 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Tue, 9 Aug 2022 13:50:03 +0000
Subject: [PATCH 11/26] Small fixes

---
 design/src/rfcs/rfc0020_service_builder.md | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 2fa5bd9c42..4841c053a2 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -691,14 +691,13 @@ An alternative to [Approach C](#approach-c-operations-as-middleware-constructors
 
 - `operation0_from_service`, accepts a `tower::Service<Operation0Input, Response = Operation0Output>`.
 - `operation0_from_handler`, accepts an async `Fn(Operation0Input) -> Operation0Output`.
-- `operation0_from_handler_with_state`, accepts an async `Fn(Operation0Input, State) -> Operation0Output` and a `State`.
 - `operation0_layer`, accepts a `tower::Layer<Op0>`.
 
 This is functionally similar to [Attempt C](#approach-c-operations-as-middleware-constructors) except that all composition is done internal to the service builder and the namespace exists in the method name, rather than the `{Operation}` struct.
 
 ## Service parameterized Routers
 
-Currently the `Router` stores `Box<dyn tower::Service<http::Request, Response = http::Response>`. As a result the `Router::layer` method, seen in [Router](#router), must re-box a service after every `tower::Layer` applied. The heap allocation `Box::new` itself is not cause for concern because `Router`s are typically constructed once at startup, however one might expect the indirection to regress performance.
+Currently the `Router` stores `Box<dyn tower::Service<http::Request, Response = http::Response>`. As a result the `Router::layer` method, seen in [Router](#router), must re-box a service after every `tower::Layer` applied. The heap allocation `Box::new` itself is not cause for concern because `Router`s are typically constructed once at startup, however one might expect the indirection to regress performance when the server is running.
 
 Having the service type parameterized as `Router<S>`, allows us to write:
 

From 4c935890ed2319d3c08df4f2a115eea87d82addd Mon Sep 17 00:00:00 2001
From: Harry Barber <106155934+hlbarber@users.noreply.github.com>
Date: Tue, 9 Aug 2022 16:41:20 +0100
Subject: [PATCH 12/26] Apply suggestions from code review

Co-authored-by: david-perez <d@vidp.dev>
---
 design/src/rfcs/rfc0020_service_builder.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 4841c053a2..54cd3aa27e 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -40,7 +40,7 @@ service Service0 {
 }
 ```
 
-We have purposely omitted details from the model that are unimportant to describing the proposal. We also omit distracting details from the Rust snippets. Code generation is linear in the sense that, code snippets can be assumed to extend to multiple operations in a predictable way. In the case where we do want to speak generally about an operation and it's associated types, we use `{Operation}`, for example `{Operation}Input` is the input type of an unspecified operation.
+We have purposely omitted details from the model that are unimportant to describing the proposal. We also omit distracting details from the Rust snippets. Code generation is linear in the sense that, code snippets can be assumed to extend to multiple operations in a predictable way. In the case where we do want to speak generally about an operation and its associated types, we use `{Operation}`, for example `{Operation}Input` is the input type of an unspecified operation.
 
 Here is a quick example of what a customer might write when using the service builder:
 
@@ -296,7 +296,7 @@ To identify where the implementations should differ we should classify in what w
 
 #### Extractors and Responses
 
-In `axum` there is a notion of [Extractor](https://docs.rs/axum/latest/axum/extract/index.html), which allows the customer to easily define a decomposition of an incoming `http::Request` by specify the arguments to the handlers. For example,
+In `axum` there is a notion of [Extractor](https://docs.rs/axum/latest/axum/extract/index.html), which allows the customer to easily define a decomposition of an incoming `http::Request` by specifying the arguments to the handlers. For example,
 
 ```rust
 async fn request(Json(payload): Json<Value>, Query(params): Query<HashMap<String, String>>, headers: HeaderMap) {
@@ -337,11 +337,11 @@ macro_rules! impl_handler {
 }
 ```
 
-The implementations of `Handler` in `axum` and `smithy-rs` follow a similar pattern - convert `http::Request` into the closures input, run the closure, convert the output of the closure to `http::Response`.
+The implementations of `Handler` in `axum` and `smithy-rs` follow a similar pattern - convert `http::Request` into the closure's input, run the closure, convert the output of the closure to `http::Response`.
 
 In `smithy-rs` we do not need a notion of "extractor", that role is fulfilled by the model, including HTTP binding traits. In `smithy-rs` the `http::Request` decomposition is determined by the Smithy model and the service protocol, whereas in `axum` it's defined by the handlers signature. In `smithy-rs` the only remaining degree of freedom in the signature of the handler is whether or not state is included.
 
-Dual to `FromRequest` is the [axum::response::IntoResponse](https://docs.rs/axum/latest/axum/response/trait.IntoResponse.html) trait, this plays the role of converting the output of the handler to `http::Response`. Again, the difference between `axum` and `smithy-rs` is that `smithy-rs` has the conversion from `{Operation}Output` to `http::Response` specified by the Smithy model, whereas `axum` the customer is free to specify a return type which implements `axum::response::IntoResponse`.
+Dual to `FromRequest` is the [axum::response::IntoResponse](https://docs.rs/axum/latest/axum/response/trait.IntoResponse.html) trait. This plays the role of converting the output of the handler to `http::Response`. Again, the difference between `axum` and `smithy-rs` is that `smithy-rs` has the conversion from `{Operation}Output` to `http::Response` specified by the Smithy model, whereas in `axum` the customer is free to specify a return type which implements `axum::response::IntoResponse`.
 
 #### Routing
 

From d0f5e1a9e84d42b092d8ffb4d24d0f7d7c6e08dd Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Tue, 9 Aug 2022 16:02:02 +0000
Subject: [PATCH 13/26] Small fixes

---
 design/src/rfcs/rfc0020_service_builder.md | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 54cd3aa27e..d0366e9253 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -339,7 +339,7 @@ macro_rules! impl_handler {
 
 The implementations of `Handler` in `axum` and `smithy-rs` follow a similar pattern - convert `http::Request` into the closure's input, run the closure, convert the output of the closure to `http::Response`.
 
-In `smithy-rs` we do not need a notion of "extractor", that role is fulfilled by the model, including HTTP binding traits. In `smithy-rs` the `http::Request` decomposition is determined by the Smithy model and the service protocol, whereas in `axum` it's defined by the handlers signature. In `smithy-rs` the only remaining degree of freedom in the signature of the handler is whether or not state is included.
+In `smithy-rs` we do not need a notion of "extractor" - the `http::Request` decomposition is specified by the Smithy model, whereas in `axum` it's defined by the handlers signature. In `smithy-rs` the only remaining degree of freedom in the signature of the handler is whether or not state is included.
 
 Dual to `FromRequest` is the [axum::response::IntoResponse](https://docs.rs/axum/latest/axum/response/trait.IntoResponse.html) trait. This plays the role of converting the output of the handler to `http::Response`. Again, the difference between `axum` and `smithy-rs` is that `smithy-rs` has the conversion from `{Operation}Output` to `http::Response` specified by the Smithy model, whereas in `axum` the customer is free to specify a return type which implements `axum::response::IntoResponse`.
 
@@ -379,7 +379,7 @@ In `smithy-rs` a customer is only able to apply a layer to either the `aws_smith
 
 The proposal is presented as a series of compatible transforms to the existing service builder, each paired with a motivation. Most of these can be independently implemented, but in the case where there exists an interdependency it is stated.
 
-Although presented as a mutation to the existing service builder, the actual implementation should exist as a entirely separate builder - reusing code generation from the old builder while exposing a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
+Although presented as a mutation to the existing service builder, the actual implementation should exist as an entirely separate builder - reusing code generation from the old builder while exposing a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
 
 ## Remove two-step build procedure
 
@@ -464,7 +464,7 @@ As mentioned in [Comparison to Axum: Routing](#routing) and [Handlers](#handlers
 
 The three use cases described above are supported by `axum` by virtue of the [Router::route](https://docs.rs/axum/latest/axum/routing/struct.Router.html#method.route) method accepting a `tower::Service`. The reader should consider a similar approach where the service builder setters accept a `tower::Service<http::Request, Response = http::Response>` rather than the `Handler`.
 
-Throughout this section we purposely ignore the existence of handlers accepting state alongside the `{Operation}Input`, this class of handlers serve as a distraction and can be handled with small perturbations from each approach.
+Throughout this section we purposely ignore the existence of handlers accepting state alongside the `{Operation}Input`, this class of handlers serve as a distraction and can be accommodated with small perturbations from each approach.
 
 ### Approach A: Customer uses `OperationHandler::new`
 
@@ -495,19 +495,19 @@ async fn handler0(input: Operation0Input) -> Operation0Output {
 let svc = OperationHandler::new(handler0);
 
 // Middleware can be applied at this point
-let operation0 = http_layer.layer(op1_svc);
+let operation0 = /* A HTTP `tower::Layer` */.layer(op1_svc);
 
 OperationRegistryBuilder::default()
     .operation0(operation0)
     /* ... */
 ```
 
-Note that this requires that the `OperationRegistryBuilder` stores services, rather than `Handler`s. An unintended and superficial benefit of this is that we are able to drop `In{n}` from the `OperationRegistryBuilder<Op0, In0, Op1, In1>` - only `Op{n}` remains and it parametrizes each operations `tower::Service`.
+Note that this requires that the `OperationRegistryBuilder` stores services, rather than `Handler`s. An unintended and superficial benefit of this is that we are able to drop `In{n}` from the `OperationRegistryBuilder<Op0, In0, Op1, In1>` - only `Op{n}` remains and it parametrizes each operation's `tower::Service`.
 
 It is still possible to retain the original API which accepts `Handler` by introducing the following setters:
 
 ```rust
-impl<Op1, Op2> OperationRegistry<Op1, Op2> {
+impl<Op1, Op2> OperationRegistryBuilder<Op1, Op2> {
     fn operation0_handler<H: Handler>(self, handler: H) -> OperationRegistryBuilder<OperationHandler<H>, Op2> {
         OperationRegistryBuilder {
             operation0: OperationHandler::new(handler),

From deef425fc613aaa508974830e0e9a80433214ecf Mon Sep 17 00:00:00 2001
From: Harry Barber <106155934+hlbarber@users.noreply.github.com>
Date: Wed, 10 Aug 2022 15:00:31 +0100
Subject: [PATCH 14/26] Apply suggestions from code review

Co-authored-by: david-perez <d@vidp.dev>
---
 design/src/rfcs/rfc0020_service_builder.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index d0366e9253..5bc4a6693d 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -517,7 +517,7 @@ impl<Op1, Op2> OperationRegistryBuilder<Op1, Op2> {
 }
 ```
 
-There are two points at which the customer might want to apply middleware: around `tower::Service<{Operation}Input, Response = {Operation}Output>` and `tower::Service<http::Response, Response = http::Response>`, that is, before and after the serialization/deserialization is performed. The change described only succeeds in the later, and therefore is only a partial solution to (1).
+There are two points at which the customer might want to apply middleware: around `tower::Service<{Operation}Input, Response = {Operation}Output>` and `tower::Service<http::Request, Response = http::Response>`, that is, before and after the serialization/deserialization is performed. The change described only succeeds in the latter, and therefore is only a partial solution to (1).
 
 This solves (2), the service builder may apply additional middleware around the service.
 
@@ -605,7 +605,7 @@ OperationRegistryBuilder::default()
 
 While [Attempt B](#approach-b-operations-as-middleware) solves all three problems, it fails to adequately model the Smithy semantics. An operation cannot uniquely define a `tower::Service` without reference to a parent Smithy service - information concerning the serialization/deserialization, error modes are all inherited from the Smithy service an operation is used within. In this way, `Operation0` should not be a standalone middleware, but become middleware once accepted by the service builder.
 
-Any solution which provides an `{Operation}` structure and wishes it to be accepted by multiple service builders must deal with this problem. We currently build one library per service and hence have duplicate structures when [service closures](https://awslabs.github.io/smithy/1.0/spec/core/model.html?highlight=closure#service-closure) overlap. This means we wouldn't run into this problem today, but it would be a future obstruction if we wanted to reduce the amount of generated code.
+Any solution which provides an `{Operation}` structure and wishes it to be accepted by multiple service builders must deal with this problem. We currently build one library per service and hence have duplicate structures when [service closures](https://awslabs.github.io/smithy/1.0/spec/core/model.html#service-closure) overlap. This means we wouldn't run into this problem today, but it would be a future obstruction if we wanted to reduce the amount of generated code.
 
 ```rust
 use tower::layer::util::{Stack, Identity};

From cfd257d435443339ce2c71e2bc26a1ed1a2922fa Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Thu, 11 Aug 2022 13:54:14 +0000
Subject: [PATCH 15/26] Small fixes

---
 design/src/rfcs/rfc0020_service_builder.md | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 5bc4a6693d..f4665ad6ff 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -115,7 +115,7 @@ where
 }
 ```
 
-Creating `{Operation}Input` from a `http::Request` and `http::Response` from a `{Operation}Output` involves the [HTTP binding traits](https://awslabs.github.io/smithy/1.0/spec/core/http-traits.html) and protocol aware serialization/deserialization. The [RuntimeError](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/runtime_error.rs#L53-L5) enumerates error cases such as serialization/deserialization failures, `extensions().get::<T>()` failures, etc. We omit error handling in the snippets above, but, in full, they also involve protocol aware conversions from the `RuntimeError` and `http::Response`. The reader should make note of the influence of the model on the different sections of this procedure.
+Creating `{Operation}Input` from a `http::Request` and `http::Response` from a `{Operation}Output` involves protocol aware serialization/deserialization, for example, it can involve the [HTTP binding traits](https://awslabs.github.io/smithy/1.0/spec/core/http-traits.html). The [RuntimeError](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/runtime_error.rs#L53-L5) enumerates error cases such as serialization/deserialization failures, `extensions().get::<T>()` failures, etc. We omit error handling in the snippet above, but, in full, it also involves protocol aware conversions from the `RuntimeError` to `http::Response`. The reader should make note of the influence of the model on the different sections of this procedure.
 
 The `request.extensions().get::<T>()` present in the `Fun: FnOnce(Operation0Input, Extension<S>) -> Fut` implementation is the current approach to injecting state into handlers. The customer is required to apply a [AddExtensionLayer](https://docs.rs/tower-http/latest/tower_http/add_extension/struct.AddExtensionLayer.html) to the output of the service builder so that, when the request reaches the handler, the `extensions().get::<T>()` will succeed.
 
@@ -146,7 +146,7 @@ where
 
 ### Builder
 
-The service builder we provide to the customer takes the form of the `OperationRegistryBuilder`, generated from [ServerOperationRegistryGenerator.kt](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationRegistryGenerator.kt).
+The service builder we provide to the customer is the `OperationRegistryBuilder`, generated from [ServerOperationRegistryGenerator.kt](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationRegistryGenerator.kt).
 
 Currently, the reference model would generate the following `OperationRegistryBuilder` and `OperationRegistry`:
 
@@ -220,11 +220,11 @@ where
 
 ### Router
 
-The [aws_smithy_http::routing::Router](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/routing/mod.rs#L58-L60) provides the protocol aware routing of requests to their target service, it exists as
+The [aws_smithy_http::routing::Router](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/routing/mod.rs#L58-L60) provides the protocol aware routing of requests to their target , it exists as
 
 ```rust
 pub struct Route {
-    service: BoxCloneService<http::Request, http::Response, Infallible>,
+    service: Box<dyn Service<http::Request, Response = http::Response>>,
 }
 
 enum Routes {
@@ -379,7 +379,7 @@ In `smithy-rs` a customer is only able to apply a layer to either the `aws_smith
 
 The proposal is presented as a series of compatible transforms to the existing service builder, each paired with a motivation. Most of these can be independently implemented, but in the case where there exists an interdependency it is stated.
 
-Although presented as a mutation to the existing service builder, the actual implementation should exist as an entirely separate builder - reusing code generation from the old builder while exposing a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
+Although presented as a mutation to the existing service builder, the actual implementation should exist as an entirely separate builder, living in a separate namespace, reusing code generation from the old builder, while exposing a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
 
 ## Remove two-step build procedure
 

From d4fbe9284b94b0231a699d90533e27ef8011f21f Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Tue, 16 Aug 2022 22:35:16 +0000
Subject: [PATCH 16/26] Fill proposal sections

---
 design/src/rfcs/rfc0020_service_builder.md | 58 ++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index f4665ad6ff..9d439ca995 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -714,6 +714,64 @@ impl<S> Router<S> {
 
 ## Protocol specific Routers
 
+Currently there is a single `Router` structure, described in [Router](#router), situated in the `rust-runtime/aws-smithy-http-server` crate, which is output by the service builder. This, roughly, takes the form of an `enum` listing the different protocols.
+
+```rust
+#[derive(Debug)]
+enum Routes {
+    RestXml(/* Container */),
+    RestJson1(/* Container */),
+    AwsJson10(/* Container */),
+    AwsJson11(/* Container */),
+}
+```
+
+A consequence of this is that it obstructs the ability of a third-party to extend `smithy-rs` to additional protocols.
+
+<!-- This needs work. -->
+
 ## Protocol specific Errors
 
+Currently, protocol specific routing errors are either:
+
+- Converted to `RuntimeError`s and then `http::Response` (see [unknown_operation](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/routing/mod.rs#L106-L118)).
+- Converted directly to a `http::Response` (see [method_not_allowed](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/rust-runtime/aws-smithy-http-server/src/routing/mod.rs#L121-L127)). This is an outlier to the common pattern.
+
+The `from_request` functions yield protocol specific errors which are converted to `RequestRejection`s then `RuntimeError`s (see [ServerHttpBoundProtocolGenerator.kt](https://github.com/awslabs/smithy-rs/blob/458eeb63b95e6e1e26de0858457adbc0b39cbe4e/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt#L194-L210)).
+
+In these scenarios protocol specific errors are converted into `RuntimeError` before being converted to a `http::Response` via `into_response` method.
+
+Two consequences of this are:
+
+- `RuntimeError` captures the union of all possible errors across all existing protocols, so is larger than needed when modelling the errors for a specific protocol.
+- If a third-party wanted extend `smithy-rs` to additional protocols with differing failure modes `RuntimeError` would have to be extended.
+
+
+<!-- This needs work. -->
+
 ## Type erasure with the name of the Smithy service
+
+Currently the service builder is named `OperationRegistryBuilder`. Despite the name being model agnostic, the `OperationRegistryBuilder` mutates when the associated service mutates. Renaming `OperationRegistryBuilder` to `{Service}Builder` would reflect the relationship between the builder and the Smithy service and prevent naming conflicts if multiple service builders are to exist in the same namespace.
+
+Similarly, the output of the service builder is `Router`. This ties the output of the service builder to a structure in `rust-runtime`. Introducing a type erasure here around `Router` using a newtype named `{Service}` would:
+
+- Ensure we are free to change the implementation of `{Service}` without changing the `Router` implementation.
+- Allow us to put a `builder` method on `{Service}` which returns `{Service}Builder`.
+
+With both of these changes the API would take the form:
+
+```rust
+let service_0: Service0 = Service0::builder()
+    /* use the setters */
+    .build()
+    .unwrap()
+    .into();
+```
+
+With [Remove two-step build procedure](#remove-two-step-build-procedure), [Switch `From<OperationRegistry> for Router` to a `OperationRegistry::build` method](#switch-fromoperationregistry-for-router-to-a-operationregistrybuild-method), and [Statically check for missing Handlers](#statically-check-for-missing-handlers) we obtain the following API:
+
+```rust
+let service_0: Service0 = Service0::builder()
+    /* use the setters */
+    .build();
+```

From 775b5a91095794585426530c33fee89e93fbce17 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Tue, 16 Aug 2022 22:58:34 +0000
Subject: [PATCH 17/26] Add Type-safe Handler State

---
 design/src/rfcs/rfc0020_service_builder.md | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 9d439ca995..6091bad527 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -775,3 +775,32 @@ let service_0: Service0 = Service0::builder()
     /* use the setters */
     .build();
 ```
+
+### Type-safe Handler State
+
+As described in [Handlers](#handlers), the current method of exposing state to a handler is via the insertion then lookup of items from the `http::Request::extensions` type map. In [Handlers](#handlers) we noted that the retrieval of state from this presents an extra failure case. In [Comparison to Axum](#comparison-to-axum) we noted that, in `smithy-rs`, we no longer have the use case where state is scoped to subtrees of the routing tree.
+
+Removing this runtime insertion/removal and opting for a static alternative would remove a runtime failure case and improve performance.
+
+The `Handler` trait already enjoys a blanket implementation for any `FnOnce({OperationInput}) -> Fut + Clone + Send + 'static` where `Fut: Future<Output = {Operation}Output>`. This includes any closure which captures cloneable state. In this way the customer already has the option to provide state to handlers while building their service.
+
+Requiring that the customer writes all handlers as closures might be cumbersome, providing the following API will make adjoining state to a handler easier for customers:
+
+```rust
+async fn operation_0(input: Operation0Input, state: T) -> Operation0Output {
+    todo!()
+}
+
+let handler = operation_0.with_state(/* something T valued */);
+```
+
+The `with_state` extension method creates the following wrapper
+
+```rust
+struct StatefulWrapper<F, T> {
+    f: F,
+    state: T
+}
+```
+
+which enjoys `Handler` and hence allows it to be provided to the service builder in the same way as a `async fn operation_0(input: Operation0Input) -> Operation0Output` would be.

From 4ba4bc2c3acdaf4a43e60ba24aaad839afe4004e Mon Sep 17 00:00:00 2001
From: Harry Barber <106155934+hlbarber@users.noreply.github.com>
Date: Wed, 17 Aug 2022 00:02:56 +0100
Subject: [PATCH 18/26] Apply spelling suggestion

Co-authored-by: Weihang Lo <weihanglo@users.noreply.github.com>
---
 design/src/rfcs/rfc0020_service_builder.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 6091bad527..a230f2f06a 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -434,7 +434,7 @@ where
 
 The customer will now get a compile time error rather than a runtime error when they fail to specify a handler.
 
-## Switch `From<OperationRegistry> for Router` to a `OperationRegistry::build` method
+## Switch `From<OperationRegistry> for Router` to an `OperationRegistry::build` method
 
 To construct a `Router`, the customer must either give a type ascription
 

From 910efaf588284681cd46ca41eb38a7954e6f15df Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Wed, 17 Aug 2022 09:51:53 +0000
Subject: [PATCH 19/26] Address feedback

---
 design/src/rfcs/rfc0020_service_builder.md | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index a230f2f06a..abebb96a74 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -400,9 +400,14 @@ As described in [Builder](#builder), the `OperationRegistryBuilder::build` metho
     }
 ```
 
-We can do away with fallibility if we allow for on `Op0`, `Op1`, to switch types during build. For example,
+We can do away with fallibility if we allow for on `Op0`, `Op1` to switch types during build and remove the `Option` from around the fields. The `OperationRegistryBuilder` then becomes
 
 ```rust
+struct OperationRegistryBuilder<Op0, Op1> {
+    operation_0: Op0,
+    operation_1: Op1
+}
+
 impl OperationRegistryBuilder<Op0, In0, Op1, In1> {
     pub fn operation0<NewOp0>(mut self, value: NewOp0) -> OperationRegistryBuilder<NewOp0, In0, Op1, In1> {
         OperationRegistryBuilder {

From 849e77497f3a01add32e0b10268ff12ae8158f04 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Wed, 17 Aug 2022 11:10:44 +0000
Subject: [PATCH 20/26] Improve proposal

---
 design/src/rfcs/rfc0020_service_builder.md | 60 +++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index abebb96a74..9f4b5bbd01 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -459,6 +459,8 @@ If we switch from a `From<OperationRegistry> for Router` to a `build` method on
 let app = /* Service builder */.build();
 ```
 
+There already exists a `build` method taking `OperationRegistryBuilder` to `OperationRegistry`, this is removed in [Remove two-step build procedure](#remove-two-step-build-procedure). These two transforms pair well together for this reason.
+
 ## Operations as Middleware Constructors
 
 As mentioned in [Comparison to Axum: Routing](#routing) and [Handlers](#handlers), the `smithy-rs` service builder accepts handlers and only converts them into a `tower::Service` during the final conversion into a `Router`. There are downsides to this:
@@ -781,7 +783,7 @@ let service_0: Service0 = Service0::builder()
     .build();
 ```
 
-### Type-safe Handler State
+## Type-safe Handler State
 
 As described in [Handlers](#handlers), the current method of exposing state to a handler is via the insertion then lookup of items from the `http::Request::extensions` type map. In [Handlers](#handlers) we noted that the retrieval of state from this presents an extra failure case. In [Comparison to Axum](#comparison-to-axum) we noted that, in `smithy-rs`, we no longer have the use case where state is scoped to subtrees of the routing tree.
 
@@ -809,3 +811,59 @@ struct StatefulWrapper<F, T> {
 ```
 
 which enjoys `Handler` and hence allows it to be provided to the service builder in the same way as a `async fn operation_0(input: Operation0Input) -> Operation0Output` would be.
+
+### Combined Proposal
+
+A combination of all the proposed transformations result in the following API:
+
+```rust
+struct State {
+    /* fields */
+}
+
+async fn handler(input: Operation0Input) -> Operation0Output {
+    todo!()
+}
+
+async fn stateful_handler(input: Operation0Input, state: State) -> Operation0Output {
+    todo!()
+}
+
+struct Operation1Service {
+    /* fields */
+}
+
+impl Service<Operation1Input> for Operation1Service {
+    type Response = Operation1Output;
+
+    /* implementation */
+}
+
+// Create an operation from a handler
+let operation_0 = Operation0::from_handler(handler);
+
+// Create an operation from a handler with state
+let operation_0 = Operation::from_handler(stateful_handler.with_state(State { /* initialize */ }));
+
+// Create an operation from a `tower::Service`
+let operation_1_svc = Operation1Service { /* initialize */ };
+let operation_1 = Operation::from_service(operation_1_svc);
+
+// Apply a layer
+let operation_0 = operation_0.layer(/* layer */);
+
+// Use the service builder
+let service_0 = Service0::builder()
+    .operation_0(operation_0)
+    .operation_1(operation_1)
+    .build();
+```
+
+A toy implementation of the combined proposal presented in the [this PR](https://github.com/hlbarber/service-builder/pull/1).
+
+## Changes Checklist
+
+- [ ] Add protocol specific routers to `rust-runtime/aws-smithy-http-server`.
+- [ ] Add middleware primitives and error types to `rust-runtime/aws-smithy-http-server`.
+- [ ] Add code generation which outputs new service builder.
+- [ ] Deprecate `OperationRegistryBuilder`, `OperationRegistry` and `Router`.

From 5cd2899e66e1476b3843f881d3afac724de619fc Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Wed, 17 Aug 2022 12:25:06 +0000
Subject: [PATCH 21/26] Improve protocol specific sections

---
 design/src/rfcs/rfc0020_service_builder.md | 23 +++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 9f4b5bbd01..a322b29180 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -32,6 +32,7 @@ operation Operation1 {
     output: Output1
 }
 
+@restJson1
 service Service0 {
     operations: [
         Operation0,
@@ -733,9 +734,14 @@ enum Routes {
 }
 ```
 
-A consequence of this is that it obstructs the ability of a third-party to extend `smithy-rs` to additional protocols.
+Recall the form of the `Service::call` method, given in [Router](#router), which involved matching on the protocol and then performing protocol specific logic.
 
-<!-- This needs work. -->
+Two downsides of modelling `Router` in this way are:
+
+- `Router` is larger and has more branches than a protocol specific implementation.
+- If a third-party wanted to extend `smithy-rs` to additional protocols `Routes` would have to be extended. A synopsis of this obstruction is presented in [Should we generate the `Router` type](https://github.com/awslabs/smithy-rs/issues/1606) issue.
+
+After taking the [Switch `From<OperationRegistry> for Router` to an `OperationRegistry::build` method](#switch-fromoperationregistry-for-router-to-an-operationregistrybuild-method) transform, code generation is free to switch between return types based on the model. This allows for a scenario where a `@restJson1` causes the service builder to output a specific `RestRouter1`.
 
 ## Protocol specific Errors
 
@@ -748,13 +754,10 @@ The `from_request` functions yield protocol specific errors which are converted
 
 In these scenarios protocol specific errors are converted into `RuntimeError` before being converted to a `http::Response` via `into_response` method.
 
-Two consequences of this are:
-
-- `RuntimeError` captures the union of all possible errors across all existing protocols, so is larger than needed when modelling the errors for a specific protocol.
-- If a third-party wanted extend `smithy-rs` to additional protocols with differing failure modes `RuntimeError` would have to be extended.
+Two downsides of this are:
 
-
-<!-- This needs work. -->
+- `RuntimeError` enumerates all possible errors across all existing protocols, so is larger than modelling the errors for a specific protocol.
+- If a third-party wanted to extend `smithy-rs` to additional protocols with differing failure modes `RuntimeError` would have to be extended. As in [Protocol specific Errors](#protocol-specific-errors), a synopsis of this obstruction is presented in [Should we generate the `Router` type](https://github.com/awslabs/smithy-rs/issues/1606) issue.
 
 ## Type erasure with the name of the Smithy service
 
@@ -765,6 +768,8 @@ Similarly, the output of the service builder is `Router`. This ties the output o
 - Ensure we are free to change the implementation of `{Service}` without changing the `Router` implementation.
 - Allow us to put a `builder` method on `{Service}` which returns `{Service}Builder`.
 
+This is compatible with [Protocol specific Routers](#protocol-specific-routers), we simply newtype the protocol specific router rather than `Router`.
+
 With both of these changes the API would take the form:
 
 ```rust
@@ -789,7 +794,7 @@ As described in [Handlers](#handlers), the current method of exposing state to a
 
 Removing this runtime insertion/removal and opting for a static alternative would remove a runtime failure case and improve performance.
 
-The `Handler` trait already enjoys a blanket implementation for any `FnOnce({OperationInput}) -> Fut + Clone + Send + 'static` where `Fut: Future<Output = {Operation}Output>`. This includes any closure which captures cloneable state. In this way the customer already has the option to provide state to handlers while building their service.
+The `Handler` trait already enjoys a blanket implementation for any `FnOnce({OperationInput}) -> Fut + Clone + Send + 'static` where `Fut: Future<Output = {Operation}Output>`. This includes any closure which captures cloneable state. In this way the customer already has the option to provide type safe state to handlers while building their service.
 
 Requiring that the customer writes all handlers as closures might be cumbersome, providing the following API will make adjoining state to a handler easier for customers:
 

From 774c2bf3adb35c6508877df5ba460edff4a1a62c Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Fri, 19 Aug 2022 12:43:35 +0000
Subject: [PATCH 22/26] Tweaks

---
 design/src/rfcs/rfc0020_service_builder.md | 87 +++++++++-------------
 1 file changed, 37 insertions(+), 50 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index a322b29180..2ec38377de 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -65,7 +65,7 @@ let app: Router = OperationRegistryBuilder::default()
     .into();
 ```
 
-During the survey we touch on the major mechanisms used to acheive this API.
+During the survey we touch on the major mechanisms used to achieve this API.
 
 ### Handlers
 
@@ -340,7 +340,7 @@ macro_rules! impl_handler {
 
 The implementations of `Handler` in `axum` and `smithy-rs` follow a similar pattern - convert `http::Request` into the closure's input, run the closure, convert the output of the closure to `http::Response`.
 
-In `smithy-rs` we do not need a notion of "extractor" - the `http::Request` decomposition is specified by the Smithy model, whereas in `axum` it's defined by the handlers signature. In `smithy-rs` the only remaining degree of freedom in the signature of the handler is whether or not state is included.
+In `smithy-rs` we do not need a general notion of "extractor" - the `http::Request` decomposition is specified by the Smithy model, whereas in `axum` it's defined by the handlers signature. Despite the Smithy specification the customer may still want an "escape hatch" to allow them access to data outside of the Smithy service inputs, for this reason we should continue to support a restricted notion of extractor. This will help support use cases such as passing [lambda_http::Context](https://docs.rs/lambda_http/latest/lambda_http/struct.Context.html) through to the handler despite it not being modeled in the Smithy model.
 
 Dual to `FromRequest` is the [axum::response::IntoResponse](https://docs.rs/axum/latest/axum/response/trait.IntoResponse.html) trait. This plays the role of converting the output of the handler to `http::Response`. Again, the difference between `axum` and `smithy-rs` is that `smithy-rs` has the conversion from `{Operation}Output` to `http::Response` specified by the Smithy model, whereas in `axum` the customer is free to specify a return type which implements `axum::response::IntoResponse`.
 
@@ -376,17 +376,17 @@ let api_routes = Router::new()
 
 In `smithy-rs` a customer is only able to apply a layer to either the `aws_smithy_http::routing::Router` or every route via the [layer method](#router) described above.
 
-# Proposal
+## Proposal
 
 The proposal is presented as a series of compatible transforms to the existing service builder, each paired with a motivation. Most of these can be independently implemented, but in the case where there exists an interdependency it is stated.
 
 Although presented as a mutation to the existing service builder, the actual implementation should exist as an entirely separate builder, living in a separate namespace, reusing code generation from the old builder, while exposing a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
 
-## Remove two-step build procedure
+### Remove two-step build procedure
 
 As described in [Builder](#builder), the customer is required to perform two conversions. One from `OperationRegistryBuilder` via `OperationRegistryBuilder::build`, the second from `OperationRegistryBuilder` to `Router` via the `From<OperationRegistry> for Router` implementation. The intermediary stop at `OperationRegistry` is not required and can be removed.
 
-## Statically check for missing Handlers
+### Statically check for missing Handlers
 
 As described in [Builder](#builder), the `OperationRegistryBuilder::build` method is fallible - it yields a runtime error when one of the handlers has not been set.
 
@@ -440,7 +440,7 @@ where
 
 The customer will now get a compile time error rather than a runtime error when they fail to specify a handler.
 
-## Switch `From<OperationRegistry> for Router` to an `OperationRegistry::build` method
+### Switch `From<OperationRegistry> for Router` to an `OperationRegistry::build` method
 
 To construct a `Router`, the customer must either give a type ascription
 
@@ -462,7 +462,7 @@ let app = /* Service builder */.build();
 
 There already exists a `build` method taking `OperationRegistryBuilder` to `OperationRegistry`, this is removed in [Remove two-step build procedure](#remove-two-step-build-procedure). These two transforms pair well together for this reason.
 
-## Operations as Middleware Constructors
+### Operations as Middleware Constructors
 
 As mentioned in [Comparison to Axum: Routing](#routing) and [Handlers](#handlers), the `smithy-rs` service builder accepts handlers and only converts them into a `tower::Service` during the final conversion into a `Router`. There are downsides to this:
 
@@ -474,7 +474,7 @@ The three use cases described above are supported by `axum` by virtue of the [Ro
 
 Throughout this section we purposely ignore the existence of handlers accepting state alongside the `{Operation}Input`, this class of handlers serve as a distraction and can be accommodated with small perturbations from each approach.
 
-### Approach A: Customer uses `OperationHandler::new`
+#### Approach A: Customer uses `OperationHandler::new`
 
 It's possible to make progress with a small changeset, by requiring the customer eagerly uses `OperationHandler::new` rather than it being applied internally within `From<OperationRegistry> for Router` (see [Handlers](#handlers)). The setter would then become:
 
@@ -531,7 +531,7 @@ This solves (2), the service builder may apply additional middleware around the
 
 This does not solve (3), as the customer is not able to provide a `tower::Service<{Operation}Input, Response = {Operation}Output>`.
 
-### Approach B: Operations as Middleware
+#### Approach B: Operations as Middleware
 
 In order to achieve all three we model operations as middleware:
 
@@ -609,7 +609,7 @@ OperationRegistryBuilder::default()
     /* ... */
 ```
 
-### Approach C: Operations as Middleware Constructors
+#### Approach C: Operations as Middleware Constructors
 
 While [Attempt B](#approach-b-operations-as-middleware) solves all three problems, it fails to adequately model the Smithy semantics. An operation cannot uniquely define a `tower::Service` without reference to a parent Smithy service - information concerning the serialization/deserialization, error modes are all inherited from the Smithy service an operation is used within. In this way, `Operation0` should not be a standalone middleware, but become middleware once accepted by the service builder.
 
@@ -693,7 +693,7 @@ Notice that we get some additional type safety here when compared to [Approach A
 
 The RFC favours this approach out of all those presented.
 
-### Approach D: Add more methods to the Service Builder
+#### Approach D: Add more methods to the Service Builder
 
 An alternative to [Approach C](#approach-c-operations-as-middleware-constructors) is to simply add more methods to the service builder while internally storing a `tower::Service`:
 
@@ -703,7 +703,7 @@ An alternative to [Approach C](#approach-c-operations-as-middleware-constructors
 
 This is functionally similar to [Attempt C](#approach-c-operations-as-middleware-constructors) except that all composition is done internal to the service builder and the namespace exists in the method name, rather than the `{Operation}` struct.
 
-## Service parameterized Routers
+### Service parameterized Routers
 
 Currently the `Router` stores `Box<dyn tower::Service<http::Request, Response = http::Response>`. As a result the `Router::layer` method, seen in [Router](#router), must re-box a service after every `tower::Layer` applied. The heap allocation `Box::new` itself is not cause for concern because `Router`s are typically constructed once at startup, however one might expect the indirection to regress performance when the server is running.
 
@@ -720,7 +720,7 @@ impl<S> Router<S> {
 }
 ```
 
-## Protocol specific Routers
+### Protocol specific Routers
 
 Currently there is a single `Router` structure, described in [Router](#router), situated in the `rust-runtime/aws-smithy-http-server` crate, which is output by the service builder. This, roughly, takes the form of an `enum` listing the different protocols.
 
@@ -741,9 +741,9 @@ Two downsides of modelling `Router` in this way are:
 - `Router` is larger and has more branches than a protocol specific implementation.
 - If a third-party wanted to extend `smithy-rs` to additional protocols `Routes` would have to be extended. A synopsis of this obstruction is presented in [Should we generate the `Router` type](https://github.com/awslabs/smithy-rs/issues/1606) issue.
 
-After taking the [Switch `From<OperationRegistry> for Router` to an `OperationRegistry::build` method](#switch-fromoperationregistry-for-router-to-an-operationregistrybuild-method) transform, code generation is free to switch between return types based on the model. This allows for a scenario where a `@restJson1` causes the service builder to output a specific `RestRouter1`.
+After taking the [Switch `From<OperationRegistry> for Router` to an `OperationRegistry::build` method](#switch-fromoperationregistry-for-router-to-an-operationregistrybuild-method) transform, code generation is free to switch between return types based on the model. This allows for a scenario where a `@restJson1` causes the service builder to output a specific `RestJson1Router`.
 
-## Protocol specific Errors
+### Protocol specific Errors
 
 Currently, protocol specific routing errors are either:
 
@@ -759,7 +759,9 @@ Two downsides of this are:
 - `RuntimeError` enumerates all possible errors across all existing protocols, so is larger than modelling the errors for a specific protocol.
 - If a third-party wanted to extend `smithy-rs` to additional protocols with differing failure modes `RuntimeError` would have to be extended. As in [Protocol specific Errors](#protocol-specific-errors), a synopsis of this obstruction is presented in [Should we generate the `Router` type](https://github.com/awslabs/smithy-rs/issues/1606) issue.
 
-## Type erasure with the name of the Smithy service
+Switching from using `RuntimeError` to protocol specific errors which satisfy a common interface, `IntoResponse`, would resolve these problem.
+
+### Type erasure with the name of the Smithy service
 
 Currently the service builder is named `OperationRegistryBuilder`. Despite the name being model agnostic, the `OperationRegistryBuilder` mutates when the associated service mutates. Renaming `OperationRegistryBuilder` to `{Service}Builder` would reflect the relationship between the builder and the Smithy service and prevent naming conflicts if multiple service builders are to exist in the same namespace.
 
@@ -788,41 +790,12 @@ let service_0: Service0 = Service0::builder()
     .build();
 ```
 
-## Type-safe Handler State
-
-As described in [Handlers](#handlers), the current method of exposing state to a handler is via the insertion then lookup of items from the `http::Request::extensions` type map. In [Handlers](#handlers) we noted that the retrieval of state from this presents an extra failure case. In [Comparison to Axum](#comparison-to-axum) we noted that, in `smithy-rs`, we no longer have the use case where state is scoped to subtrees of the routing tree.
-
-Removing this runtime insertion/removal and opting for a static alternative would remove a runtime failure case and improve performance.
-
-The `Handler` trait already enjoys a blanket implementation for any `FnOnce({OperationInput}) -> Fut + Clone + Send + 'static` where `Fut: Future<Output = {Operation}Output>`. This includes any closure which captures cloneable state. In this way the customer already has the option to provide type safe state to handlers while building their service.
-
-Requiring that the customer writes all handlers as closures might be cumbersome, providing the following API will make adjoining state to a handler easier for customers:
-
-```rust
-async fn operation_0(input: Operation0Input, state: T) -> Operation0Output {
-    todo!()
-}
-
-let handler = operation_0.with_state(/* something T valued */);
-```
-
-The `with_state` extension method creates the following wrapper
-
-```rust
-struct StatefulWrapper<F, T> {
-    f: F,
-    state: T
-}
-```
-
-which enjoys `Handler` and hence allows it to be provided to the service builder in the same way as a `async fn operation_0(input: Operation0Input) -> Operation0Output` would be.
-
 ### Combined Proposal
 
-A combination of all the proposed transformations result in the following API:
+A combination of all the proposed transformations results in the following API:
 
 ```rust
-struct State {
+struct Context {
     /* fields */
 }
 
@@ -830,7 +803,7 @@ async fn handler(input: Operation0Input) -> Operation0Output {
     todo!()
 }
 
-async fn stateful_handler(input: Operation0Input, state: State) -> Operation0Output {
+async fn handler_with_ext(input: Operation0Input, extension: Extension<Context>) -> Operation0Output {
     todo!()
 }
 
@@ -844,16 +817,30 @@ impl Service<Operation1Input> for Operation1Service {
     /* implementation */
 }
 
+struct Operation1ServiceWithExt {
+    /* fields */
+}
+
+impl Service<(Operation1Input, Extension<Context>)> for Operation1Service {
+    type Response = Operation1Output;
+
+    /* implementation */
+}
+
 // Create an operation from a handler
 let operation_0 = Operation0::from_handler(handler);
 
-// Create an operation from a handler with state
-let operation_0 = Operation::from_handler(stateful_handler.with_state(State { /* initialize */ }));
+// Create an operation from a handler with extension
+let operation_0 = Operation::from_handler(handler_with_ext);
 
 // Create an operation from a `tower::Service`
 let operation_1_svc = Operation1Service { /* initialize */ };
 let operation_1 = Operation::from_service(operation_1_svc);
 
+// Create an operation from a `tower::Service` with extension
+let operation_1_svc = Operation1ServiceWithExtension { /* initialize */ };
+let operation_1 = Operation::from_service(operation_1_svc);
+
 // Apply a layer
 let operation_0 = operation_0.layer(/* layer */);
 

From 8304be582a8b000b590f9017ef97541043e9a12f Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Wed, 24 Aug 2022 10:34:07 +0000
Subject: [PATCH 23/26] Tweaks

---
 design/src/rfcs/rfc0020_service_builder.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 2ec38377de..98b9aebc7c 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -362,7 +362,7 @@ let api_routes = Router::new()
 let app = Router::new().nest("/api", api_routes);
 ```
 
-Note that, in `axum` handlers are eagerly converted to a `tower::Service` (via `IntoService`) before they are passed into the `Router`. In contrast, in `smithy-rs` handlers are passed into a builder and then the conversion to `tower::Service` is performed (via `OperationHandler`).
+Note that, in `axum` handlers are eagerly converted to a `tower::Service` (via `IntoService`) before they are passed into the `Router`. In contrast, in `smithy-rs`, handlers are passed into a builder and then the conversion to `tower::Service` is performed (via `OperationHandler`).
 
 Introducing state to handlers in `axum` is done in the same way as `smithy-rs`, described briefly in [Handlers](#handlers) - a layer is used to insert state into incoming `http::Request`s and the `Handler` implementation pops it out of the type map layer. In `axum`, if a customer wanted to scope state to all routes within `/users/` they are able to do the following:
 
@@ -374,11 +374,11 @@ let api_routes = Router::new()
     .nest("/teams", team_routes);
 ```
 
-In `smithy-rs` a customer is only able to apply a layer to either the `aws_smithy_http::routing::Router` or every route via the [layer method](#router) described above.
+In `smithy-rs` a customer is only able to apply a layer around the `aws_smithy_http::routing::Router` or around every route via the [layer method](#router) described above.
 
 ## Proposal
 
-The proposal is presented as a series of compatible transforms to the existing service builder, each paired with a motivation. Most of these can be independently implemented, but in the case where there exists an interdependency it is stated.
+The proposal is presented as a series of compatible transforms to the existing service builder, each paired with a motivation. Most of these can be independently implemented, and it is stated in the cases where an interdependency exists.
 
 Although presented as a mutation to the existing service builder, the actual implementation should exist as an entirely separate builder, living in a separate namespace, reusing code generation from the old builder, while exposing a new Rust API. Preserving the old API surface will prevent breakage and make it easier to perform comparative benchmarks and testing.
 
@@ -851,7 +851,7 @@ let service_0 = Service0::builder()
     .build();
 ```
 
-A toy implementation of the combined proposal presented in the [this PR](https://github.com/hlbarber/service-builder/pull/1).
+A toy implementation of the combined proposal is presented in [this PR](https://github.com/hlbarber/service-builder/pull/1).
 
 ## Changes Checklist
 

From 9f1cd0f153705ffb622b31c2b354dec682c89644 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Fri, 26 Aug 2022 23:36:22 +0000
Subject: [PATCH 24/26] Update checklist

---
 design/src/rfcs/rfc0020_service_builder.md | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 98b9aebc7c..813f1ffdf0 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -855,7 +855,9 @@ A toy implementation of the combined proposal is presented in [this PR](https://
 
 ## Changes Checklist
 
-- [ ] Add protocol specific routers to `rust-runtime/aws-smithy-http-server`.
-- [ ] Add middleware primitives and error types to `rust-runtime/aws-smithy-http-server`.
+- [x] Add protocol specific routers to `rust-runtime/aws-smithy-http-server`.
+  - https://github.com/awslabs/smithy-rs/pull/1666
+- [x] Add middleware primitives and error types to `rust-runtime/aws-smithy-http-server`.
+  - https://github.com/awslabs/smithy-rs/pull/1679
 - [ ] Add code generation which outputs new service builder.
 - [ ] Deprecate `OperationRegistryBuilder`, `OperationRegistry` and `Router`.

From 14b053181f96e9895b3aaabf17200a5a410955e1 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Tue, 30 Aug 2022 14:02:45 +0000
Subject: [PATCH 25/26] Fix lints

---
 design/src/rfcs/rfc0020_service_builder.md | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index 813f1ffdf0..f6435a5031 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -283,15 +283,15 @@ let new_routes = old_routes
 
 Historically, `smithy-rs` has borrowed from [axum](https://github.com/tokio-rs/axum). Despite various divergences the code bases still have much in common:
 
-* Reliance on `Handler` trait to abstract over different closure signatures:
-  * [axum::handler::Handler](https://docs.rs/axum/latest/axum/handler/trait.Handler.html)
-  * [Handlers](#handlers)
-* A mechanism for turning `H: Handler` into a `tower::Service`:
-  * [axum::handler::IntoService](https://docs.rs/axum/latest/axum/handler/struct.IntoService.html)
-  * [OperationHandler](#handlers)
-* A `Router` to route requests to various handlers:
-  * [axum::Router](https://docs.rs/axum/latest/axum/struct.Router.html)
-  * [aws_smithy_http_server::routing::Router](#router)
+- Reliance on `Handler` trait to abstract over different closure signatures:
+  - [axum::handler::Handler](https://docs.rs/axum/latest/axum/handler/trait.Handler.html)
+  - [Handlers](#handlers)
+- A mechanism for turning `H: Handler` into a `tower::Service`:
+  - [axum::handler::IntoService](https://docs.rs/axum/latest/axum/handler/struct.IntoService.html)
+  - [OperationHandler](#handlers)
+- A `Router` to route requests to various handlers:
+  - [axum::Router](https://docs.rs/axum/latest/axum/struct.Router.html)
+  - [aws_smithy_http_server::routing::Router](#router)
 
 To identify where the implementations should differ we should classify in what ways the use cases differ. There are two primary areas which we describe below.
 
@@ -782,7 +782,7 @@ let service_0: Service0 = Service0::builder()
     .into();
 ```
 
-With [Remove two-step build procedure](#remove-two-step-build-procedure), [Switch `From<OperationRegistry> for Router` to a `OperationRegistry::build` method](#switch-fromoperationregistry-for-router-to-a-operationregistrybuild-method), and [Statically check for missing Handlers](#statically-check-for-missing-handlers) we obtain the following API:
+With [Remove two-step build procedure](#remove-two-step-build-procedure), [Switch `From<OperationRegistry> for Router` to a `OperationRegistry::build` method](#switch-fromoperationregistry-for-router-to-an-operationregistrybuild-method), and [Statically check for missing Handlers](#statically-check-for-missing-handlers) we obtain the following API:
 
 ```rust
 let service_0: Service0 = Service0::builder()
@@ -856,8 +856,8 @@ A toy implementation of the combined proposal is presented in [this PR](https://
 ## Changes Checklist
 
 - [x] Add protocol specific routers to `rust-runtime/aws-smithy-http-server`.
-  - https://github.com/awslabs/smithy-rs/pull/1666
+  - <https://github.com/awslabs/smithy-rs/pull/1666>
 - [x] Add middleware primitives and error types to `rust-runtime/aws-smithy-http-server`.
-  - https://github.com/awslabs/smithy-rs/pull/1679
+  - <https://github.com/awslabs/smithy-rs/pull/1679>
 - [ ] Add code generation which outputs new service builder.
 - [ ] Deprecate `OperationRegistryBuilder`, `OperationRegistry` and `Router`.

From e8622cfa2702f7bff8211ce0c6da8376e1b04cb1 Mon Sep 17 00:00:00 2001
From: Harry Barber <hlbarber@amazon.co.uk>
Date: Tue, 30 Aug 2022 14:26:08 +0000
Subject: [PATCH 26/26] Address feedback

---
 design/src/rfcs/rfc0020_service_builder.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/design/src/rfcs/rfc0020_service_builder.md b/design/src/rfcs/rfc0020_service_builder.md
index f6435a5031..f2842569e7 100644
--- a/design/src/rfcs/rfc0020_service_builder.md
+++ b/design/src/rfcs/rfc0020_service_builder.md
@@ -768,6 +768,7 @@ Currently the service builder is named `OperationRegistryBuilder`. Despite the n
 Similarly, the output of the service builder is `Router`. This ties the output of the service builder to a structure in `rust-runtime`. Introducing a type erasure here around `Router` using a newtype named `{Service}` would:
 
 - Ensure we are free to change the implementation of `{Service}` without changing the `Router` implementation.
+- Hide the router type, which is determined by the protocol specified in the model.
 - Allow us to put a `builder` method on `{Service}` which returns `{Service}Builder`.
 
 This is compatible with [Protocol specific Routers](#protocol-specific-routers), we simply newtype the protocol specific router rather than `Router`.