Introduce generic fluent client generator to non-sdk codegen

[jonhoo]

This change adds a FluentClientGenerator to codegen. The generated client uses a new crate, smithy_client, which now contains most of what used to be in aws_hyper. aws_hyper, in turn, is now a thin wrapper around smithy_client that mainly exposes type aliases with generic parameters already filled in. It's public interface is mostly unchanged (notice that aws-hyper/tests/e2e_test.rs has not changed.

smithy_client makes the client generic over three type parameters:

• Connector, which encapsulates the behavior exposed by the conn submodule previously. This is essentially a

  Service<http::Request<SdkBody>, Response = http::Response<SdkBody>>

• Middleware, which encapsulates the layers of the service stack that will vary between different Smithy deployments, such as request signing, adding a user agent, etc. This type is essentially a

  Layer<Service = Service<smithy_http::operation::Request, Response = http::Response<SdkBody>>>

  The Layer type is there so that the Middleware can be injected in the existing request stack (alongside Connector, DispatchLayer, and friends). It is instantiated to a new AwsMiddleware type in aws_hyper that encapsulates the behavior that used to be hard-coded in https://github.com/awslabs/smithy-rs/blob/5ef157dc49f1f36611b365f8eb34407f9ff942e1/aws/rust-runtime/aws-hyper/src/lib.rs#L130-L137
• RetryPolicy, which encapsulates the retry policy for the client. It defaults to smithy_client::retry::Standard, which is exactly equivalent to the old aws_hyper::RetryHandleFactory, but can be swapped out for anything that can instantiate new values of a type that implements tower::Policy.

The exact requirements of each of these generic types are represented in the new smithy_client::bounds module, which is used to make the trait bounds on methods easier to read (rather than listing out fifteen different bounds). Consumers should never implemented the traits in bounds::, but should instead implement tower::Service, tower::Layer, and others as appropriate.

So, given smithy_client::Client<Connector, Middleware, RetryPolicy>, aws_hyper::Client now becomes:

pub type Client<C> = smithy_client::Client<C, AwsMiddleware, smithy_client::retry::Standard;

which takes care of hiding most of the generic bounds. The remaining one, C, is useful to allow consumers to specify their own connection backend in a zero-overhead way. To continue to support the convenient type-erased API that existed previously, smithy_client also exposes DynConnector (and DynMiddleware) which represents a dynamically-dispatched version of a Connector. This is used as the default type for Client's C parameter, so users will not need to specify it unless they specifically want to use a custom type there. Client::https now returns a Client<DynConnector> (which is what StandardClient now holds) instead of a Client<conn::Standard>, but this is unlikely to make a difference to consumers in practice.

smithy_client also introduces a builder-style constructor with a twist. Rather than take values, the builder takes types (and values of those types). Builder is generic over <C, M, R> just like Client, and each method consumes self and returns a new Builder type with the appropriate type parameter changed depending on the method (e.g., fn connector<C2>(self, C2) -> Builder<C2, M, R>). Builder::build gives a Client. Builder also comes with convenience methods like Builder::rustls for setting the connector (C) to a Rustls hyper connector, Builder::hyper for using a custom hyper::Client, and Builder::connector_fn for using a async fn(http::Request) -> http::Response as the connector (handy for testing). It also exposes map_connector and map_middleware for wrapping an already-set connector or middleware.

aws_hyper also exposes a Builder through a simple type alias to smithy_client::Builder with M and R set and C defaulting to DynConnector.

The fluent generator for codegen generates a fluent client that is itself generic over C, M, and R. It implements From<smithy_client::Client<C, M, R>> so that smithy_client::Builder can be used to full effect, as well as methods similar to the existing ones (from_conf_and_conn) except that they take smithy_client::Client instead. The fluent generator for the sdk now generates a Client that is generic over C, but with a default of DynConnector. Its constructors remain the same, except that the type for conn in the relevant methods is now C instead of conn::Standard.

Oh, yeah, and smithy_client makes hyper an optional dependency, since all it's used for is if consumers specifically want to use a hyper connector for C.

With this all in place, aws_hyper is now trivial enough that it could (and should) probably be inlined into the generated client directly rather than be its own crate. Furthermore, the fluent client generator for the sdk could likely just be a slight customization to the one generated by codegen rather than a re-implementation of all the generation bits. But let's leave that for follow-on work.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[jonhoo]

Hmm, the documentation test failure is a little awkward. Builder::native_tls is the right value, it's just that it's not defined if the native_tls feature isn't enabled, which it isn't by default.. Not sure what to do about that. I guess I can just make it not be a link, even if that leads to slightly worse documentation.

[jonhoo]

The error about "documentation test in private item" is a known compiler error: rust-lang/rust#72081

I'll ignore that warning for now.

[rcoh]

main-generated...generic-client-generated

[carllerche]

This is a pretty awesome PR and I'm excited to see more smithy work happening. I do think the tower components would be a good fit here, especially for a smithy client.

I found reviewing the PR challenging but not because of the code itself, which is clean and well documented. The generics and bounds make it pretty hard to jump in and understand the code. Going "full tower" has pros / cons. Pros: being able to use off the shelf tower components + enable users to customize the middleware stack. Cons: very intense generics.

In the end, the trade off calculus depends on who will be interacting with smithy-client. Will it be common for services engineers to have to use smithy-client directly or is smithy-client more for sharing code between projects that use smithy-client to expose specialized client libs (e.g. the AWS Rust SDK).

As long as engineers that are new to Rust and want to build something with smithy don't have to see these generics as part of the getting started experience, then the trade off might make sense.

[jonhoo]

Yeah, it's definitely a step up in complexity. Though in some sense it's fundamental — if we want the non-sdk "fluent" client to have swappable middleware (for authn/z), connectors (for hyper vs testing), and retry policies, those will all need to have some bound to represent the behavior those swappable pieces must exhibit. The most onerous bound in here, I think, is the fact that the middleware is a Layer that in turn must produce a Service with the right request/response.

But at the same time, I think developers will only actually face those bounds if they are looking to implement their own middleware, in which case implementing Layer in the right way isn't all that onerous or error-prone (see aws_hyper's AwsMiddleware type). I think we may want to provide (either here or more likely in tower) a good guide to Layer, and to how to extend an existing stack for those who just want to amend an existing layer with some functionality (e.g., add tracing to AwsMiddleware), but even as-is that's pretty clear how to do with an example.

Honestly, I think the biggest ergonomics pain with the current implementation is a) all the repeated Send + Sync + 'static bounds, which add noise that developers need to know to ignore, and b) the lack of true type aliases, which means we need to introduce the extra traits in bounds. Also, something something about error messages for trait bounds, though this may prove fertile ground for finding new good test-cases for Esteban to improve the error messages for.

Back to your original query:

In the end, the trade off calculus depends on who will be interacting with smithy-client. Will it be common for services engineers to have to use smithy-client directly or is smithy-client more for sharing code between projects that use smithy-client to expose specialized client libs (e.g. the AWS Rust SDK).

As long as engineers that are new to Rust and want to build something with smithy don't have to see these generics as part of the getting started experience, then the trade off might make sense.

This, I think, is exactly the right question. I think it is very unlikely that service engineers will work with smithy-client directly. Instead, they will be using the generated "fluent" client, which while also generic in the same way as smithy_client::Client, has concrete types for nearly all generic type parameters, which should both make error messages better and signatures clearer (if more verbose). Even then though, I think there's a further breakdown of "instantiating the client" and "using an instantiated client".

Concretely, when someone generates a client for some Smithy service foo, what they'll end up with is a crate foo that exposes a type foo::Client<C, M, R>. foo::Client is constructed using a smithy_client::Builder. Someone has to write the code in the crate that wants to use foo::Client that selects which connector, middleware, and retry policy the foo::Client should be instantiated with. Basically, someone has to select (and construct) the arguments to the Builder. That work is a little hairy, mostly because of the middleware. I'm imagining it'll mean writing something like:

let mw = AwsMiddleware;
let mw = mw.layer(TracingMiddleware);
let client = Builder::new().https().middleware(mw).build();

That's not trivial, but it's also not too onerous, and we can provide good docs for that (and already do provide some in this PR). Specifically, what helps is that I'm expecting most consumers will want to take some existing middleware that provides authn/z, and then just add some custom bits for their particular use-case. Which guards them from most of the complexity. Nevertheless, writing this code does take a bit of understanding of tower (which hopefully the coming guides will help with), so a complete Rust newcomer will probably struggle a bit with it, as it does require understanding what Service<smithy_http_operation::Request, Response = http::Response<SdkBody>> means, and also being able to read the Layer trait.

But, once someone has written that code to construct the client, and you have a foo::Client, using that client is (or at least should be) trivial. All the bounds should hold more or less by definition, and you can simply issue requests and await their responses. I imagine even relative Rust newcomers should be able to use a client. At least that's been my goal here. They may see some bounds if they look at the docs for a particular method, but those bounds should basically not ever bother them.

---

An aside about trait objects — I don't think trait objects help simplify the API here. They remove the generic type parameter, true, but the same bounds would still need to be there on the Builder methods that take a middleware or connector or whatnot. I don't think that the mere fact of the trait bounds being hidden from docs (but still ultimately required) actually helps for ergonomics, and I especially don't think it'd be worth the reduction in flexibility and possibly performance that "forcibly removing" the trait bounds (and static dispatch with it) would bring.

[rcoh]

LGTM! I'm very excited for this!

[jonhoo]

Replaced by #496