Revamp errors in aws-smithy-http
#1884
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jdisanti
force-pushed
the
jdisanti-errors-smithy-eventstream
branch
from
c450949
to
54bcd03
Compare
jdisanti
force-pushed
the
jdisanti-errors-smithy-http
branch
from
5ecd801
to
33787d6
Compare
jdisanti
force-pushed
the
jdisanti-errors-smithy-eventstream
branch
from
54bcd03
to
56a2475
Compare
jdisanti
force-pushed
the
jdisanti-errors-smithy-http
branch
from
33787d6
to
7a043e5
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jdisanti
force-pushed
the
jdisanti-errors-smithy-eventstream
branch
from
56a2475
to
3afc0db
Compare
jdisanti
force-pushed
the
jdisanti-errors-smithy-http
branch
from
7a043e5
to
6459079
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jdisanti
force-pushed
the
jdisanti-errors-smithy-http
branch
from
6459079
to
c419c76
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Velfi
reviewed
Nov 2, 2022
Comment on lines
+769
to
+782
| macro_rules! assert_full_error_contains { | ||
| ($err:expr, $contains:expr) => { | ||
| let err = $err; | ||
| let message = format!( | ||
| "{}", | ||
| aws_smithy_types::error::display::DisplayErrorContext(&err) | ||
| ); | ||
| assert!( | ||
| message.contains($contains), | ||
| "Error message '{message}' didn't contain text '{}'", | ||
| $contains | ||
| ); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Should we extract this to a shared utils crate? It seems useful.
There was a problem hiding this comment.
I definitely considered it, but I didn't want to create a whole crate just for this macro when there are only a few places it would be used outside of aws-smithy-http.
It almost seems like we should replace the aws-smithy-protocol-test crate with a general purpose testing crate.
Velfi
reviewed
Nov 2, 2022
| assert_eq!(format!("{:?}", err), "TimeoutError(RequestTimeoutError { kind: \"operation timeout (all attempts including retries)\", duration: 500ms })"); | ||
| assert_eq!("TimeoutError(TimeoutError { source: RequestTimeoutError { kind: \"operation timeout (all attempts including retries)\", duration: 500ms } })", format!("{:?}", err)); |
There was a problem hiding this comment.
I expect assertions to be in the format assert_eq!(expected, actual) so thanks for doing this!
Velfi
reviewed
Nov 2, 2022
| @@ -289,7 +289,9 @@ class RequestBindingGeneratorTest { | |||
| .key(ts.clone()) | |||
| .build().unwrap(); | |||
| let err = inp.test_request_builder_base().expect_err("can't build request with bucket unset"); | |||
| assert!(matches!(err, ${format(TestRuntimeConfig.operationBuildError())}::MissingField { .. })) | |||
| let message = format!("{err}"); | |||
There was a problem hiding this comment.
This comment is very minor
Calling to_string would be more idiomatic, no?
Velfi
reviewed
Nov 2, 2022
| assert_eq!( | ||
| format!("{}", resp), | ||
| "timeout: error trying to connect: HTTP connect timeout occurred after 1s" | ||
| let message = format!("{}", DisplayErrorContext(&resp)); |
Velfi
reviewed
Nov 2, 2022
Comment on lines
+12
to
+15
| #[cfg(feature = "rt-tokio")] | ||
| OffsetLargerThanFileSize, | ||
| #[cfg(feature = "rt-tokio")] | ||
| LengthLargerThanFileSizeMinusReadOffset, |
There was a problem hiding this comment.
I didn't even realize you could have feature-gated variants like this, very neat
Velfi
approved these changes
Nov 2, 2022
jdisanti
force-pushed
the
jdisanti-errors-smithy-eventstream
branch
from
3afc0db
to
aa06082
Compare
jdisanti
force-pushed
the
jdisanti-errors-smithy-http
branch
from
916eda8
to
4f71d15
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jdisanti
force-pushed
the
jdisanti-errors-smithy-eventstream
branch
from
aa06082
to
fbb4c21
Compare
jdisanti
force-pushed
the
jdisanti-errors-smithy-http
branch
from
b8d09de
to
0658ee8
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jdisanti
force-pushed
the
jdisanti-errors-smithy-eventstream
branch
from
fbb4c21
to
6141e74
Compare
drganjoo
reviewed
Nov 12, 2022
| err: CredentialsError::ProviderError { .. }, | ||
| .. | ||
| } | ||
| SdkError::ServiceError(ref context) if matches!(context.err(), CredentialsError::ProviderError { .. }) |
There was a problem hiding this comment.
I guess all of CredentialsError:: are not part of this refactoring? Because CredentialsError::ProviderError looks like a struct variant.
There was a problem hiding this comment.
CredentialsError is refactored in #1922
| raw: RawMessage::Decoded(message), | ||
| }), | ||
| Err(err) => Err(SdkError::response_error( | ||
| Box::new(err), |
There was a problem hiding this comment.
I've seen some places where you've skipped writing Box::new(....) and just passed err since the response_error takes Into<BoxError>. Only for ergonomics, If we can remove this Box::new it will make it more consistent across the code.
There was a problem hiding this comment.
Good catch, it looks like I missed cleaning up some of these.
| raw: RawMessage::Invalid(None), // the buffer has been consumed | ||
| .map_err(|err| { | ||
| SdkError::response_error( | ||
| Box::new(err), |
There was a problem hiding this comment.
Only for ergonomics if we can remove Box::new and if that still works, that would be awesome.
jdisanti
force-pushed
the
jdisanti-errors-smithy-http
branch
from
ebcf384
to
07c63b2
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
drganjoo
approved these changes
Nov 15, 2022
jdisanti
force-pushed
the
jdisanti-errors-smithy-http
branch
from
07c63b2
to
c29accf
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
thomas-k-cameron
added a commit
to thomas-k-cameron/smithy-rs
that referenced
this pull request
Apr 9, 2023
* Add error refactor changelog entries and re-export `DisplayErrorContext` (#1951) * Re-export `DisplayErrorContext` * Update changelog * Do not use deprecated from_timestamp from chrono (#1980) * Do not use deprecated from_timestamp from chrono CI is failing because [from_timestamp](https://docs.rs/chrono/latest/chrono/naive/struct.NaiveDateTime.html#method.from_timestamp) is now deprecated. Signed-off-by: Daniele Ahmed <[email protected]> * Update changelog Signed-off-by: Daniele Ahmed <[email protected]> * Fix error Signed-off-by: Daniele Ahmed <[email protected]> * Use with_ymd_and_hms Signed-off-by: Daniele Ahmed <[email protected]> Signed-off-by: Daniele Ahmed <[email protected]> * Remove left behind trailing whitespace from instrumentation closure (#1975) * Fix `update-sdk-next` GitHub Actions workflow (#1979) * Feature: configurable connectors for AWS clients (#1918) * feature: make HTTP connectors configurable * add: test for HTTP connector configuration customization add: impl<B> From<TestConnection<B>> for HttpConnector add: impl From<CaptureRequestHandler> for HttpConnector add: impl From<NeverConnector> for HttpConnector add: impl From<ReplayingConnection> for HttpConnector * add: to_vec method to AggregatedBytes update: method param names of FluentClientGenerics.sendBounds to be more explicit update: restructure s3/s3control tests to be uniform in structure * update: CHANGELOG.next.toml update: codegen `impl From<&SdkConfig> for Builder` to support HTTP connectors * update: CHANGELOG entry references * add: missing copyright header * fix: clippy lint * format: run cargo fmt * format: run cargo fmt on aws_smithy_client::dvr modules * format: run ktlintFormat * refactor: use from_conf instead of from_conf_conn remove: from_conf_conn * update: impl From<SmithyConnector> for HttpConnector remove: other From<T> for HttpConnector impls update: HttpConnector config setter examples * update: CHANGELOG.next.toml * Update CHANGELOG.next.toml Co-authored-by: John DiSanti <[email protected]> * update: CHANGELOG.next.toml remove: obsolete test update: `ConfigLoader::http_connector` setter method * Update CHANGELOG.next.toml Co-authored-by: John DiSanti <[email protected]> * Update CHANGELOG.next.toml Co-authored-by: John DiSanti <[email protected]> * Update CHANGELOG.next.toml Co-authored-by: John DiSanti <[email protected]> * Apply suggestions from code review Co-authored-by: John DiSanti <[email protected]> * update: aws_config::loader::ConfigLoader doc comments update: CHANGELOG.next.toml examples * fix: doc issues add: reëxport aws_smithy_types::endpoint module to aws-config * format: run rustfmt in the weird CI way to get it to actually format. * fix: incorrect reëxport * add: "aws_smithy_http::endpoint" to allowed external types for aws-config * update: move `hyper-rustls` to deps so that it doesn't break exotic arch CI check * remove: `hyper-rustls` dep because it's not actually needed * fix: aws-types dep issue blocking exotic arch CI check * fix: broken doc comment Co-authored-by: John DiSanti <[email protected]> * Service with ConnectInfo (#1955) * Service with ConnectInfo Signed-off-by: Daniele Ahmed <[email protected]> * Use `escape` on the body in `@httpRequestTest` protocol test generation (#1949) * Builders of builders (#1342) This patchset, affectionately called "Builders of builders", lays the groundwork for fully implementing [Constraint traits] in the server SDK generator. [The RFC] illustrates what the end goal looks like, and is recommended prerrequisite reading to understanding this cover letter. This commit makes the sever deserializers work with _unconstrained_ types during request parsing, and only after the entire request is parsed are constraints enforced. Values for a constrained shape are stored in the correspondingly unconstrained shape, and right before the operation input is built, the values are constrained via a `TryFrom<UnconstrainedShape> for ConstrainedShape` implementation that all unconstrained types enjoy. The service owner only interacts with constrained types, the unconstrained ones are `pub(crate)` and for use by the framework only. In the case of structure shapes, the corresponding unconstrained shape is their builders. This is what gives this commit its title: during request deserialization, arbitrarily nested structures are parsed into _builders that hold builders_. Builders keep track of whether their members are constrained or not by storing its members in a `MaybeConstrained` [Cow](https://doc.rust-lang.org/std/borrow/enum.Cow.html)-like `enum` type: ```rust pub(crate) trait Constrained { type Unconstrained; } #[derive(Debug, Clone)] pub(crate) enum MaybeConstrained<T: Constrained> { Constrained(T), Unconstrained(T::Unconstrained), } ``` Consult the documentation for the generator in `ServerBuilderGenerator.kt` for more implementation details and for the differences with the builder types the server has been using, generated by `BuilderGenerator.kt`, which after this commit are exclusively used by clients. Other shape types, when they are constrained, get generated with their correspondingly unconstrained counterparts. Their Rust types are essentially wrapper newtypes, and similarly enjoy `TryFrom` converters to constrain them. See the documentation in `UnconstrainedShapeSymbolProvider.kt` for details and an example. When constraints are not met, the converters raise _constraint violations_. These are currently `enum`s holding the _first_ encountered violation. When a shape is _transitively but not directly_ constrained, newtype wrappers are also generated to hold the nested constrained values. To illustrate their need, consider for example a list of `@length` strings. Upon request parsing, the server deserializers need a way to hold a vector of unconstrained regular `String`s, and a vector of the constrained newtyped `LengthString`s. The former requirement is already satisfied by the generated unconstrained types, but for the latter we need to generate an intermediate constrained `ListUnconstrained(Vec<LengthString>)` newtype that will eventually be unwrapped into the `Vec<LengthString>` the user is handed. This is the purpose of the `PubCrate*` generators: consult the documentation in `PubCrateConstrainedShapeSymbolProvider.kt`, `PubCrateConstrainedCollectionGenerator.kt`, and `PubCrateConstrainedMapGenerator.kt` for more details. As their name implies, all of these types are `pub(crate)`, and the user never interacts with them. For users that would not like their application code to make use of constrained newtypes for their modeled constrained shapes, a `codegenConfig` setting `publicConstrainedTypes` has been added. They opt out of these by setting it to `false`, and use the inner types directly: the framework will still enforce constraints upon request deserialization, but once execution enters an application handler, the user is on their own to honor (or not) the modeled constraints. No user interest has been expressed for this feature, but I expect we will see demand for it. Moreover, it's a good stepping stone for users that want their services to honor constraints, but are not ready to migrate their application code to constrained newtypes. As for how it's implemented, several parts of the codebase inspect the setting and toggle or tweak generators based on its value. Perhaps the only detail worth mentioning in this commit message is that the structure shape builder types are generated by a much simpler and entirely different generator, in `ServerBuilderGeneratorWithoutPublicConstrainedTypes.kt`. Note that this builder _does not_ enforce constraints, except for `required` and `enum`, which are always (and already) baked into the type system. When `publicConstrainedTypes` is disabled, this is the builder that end users interact with, while the one that enforces all constraints, `ServerBuilderGenerator`, is now generated as `pub(crate)` and left for exclusive use by the deserializers. See the relevant documentation for the details and differences among the builder types. As proof that these foundations are sound, this commit also implements the `length` constraint trait on Smithy map and string shapes. Likewise, the `required` and `enum` traits, which were already baked in the generated types as non-`Option`al and `enum` Rust types, respectively, are now also treated like the rest of constraint traits upon request deserialization. See the documentation in `ConstrainedMapGenerator.kt` and `ConstrainedStringGenerator.kt` for details. The rest of the constraint traits and target shapes are left as an exercise to the reader, but hopefully the reader has been convinced that all of them can be enforced within this framework, paving the way for straightforward implementations. The diff is already large as it is. Any reamining work is being tracked in #1401; this and other issues are referenced in the code as TODOs. So as to not give users the impression that the server SDK plugin _fully_ honors constraints as per the Smithy specification, a validator in `ValidateUnsupportedConstraintsAreNotUsed.kt` has been added. This traverses the model and detects yet-unsupported parts of the spec, aborting code generation and printing informative warnings referencing the relevant tracking issues. This is a regression in that models that used constraint traits previously built fine (even though the constraint traits were silently not being honored), and now they will break. To unblock generation of these models, this commit adds another `codegenConfig` setting, `ignoreUnsupportedConstraints`, that users can opt into. Closes #1714. Testing ------- Several Kotlin unit test classes exercising the finer details of the added generators and symbol providers have been added. However, the best way to test is to generate server SDKs from models making use of constraint traits. The biggest assurances come from the newly added `constraints.smithy` model, an "academic" service that _heavily_ exercises constraint traits. It's a `restJson1` service that also tests binding of constrained shapes to different parts of the HTTP message. Deeply nested hierarchies and recursive shapes are also featured. ```sh ./gradlew -P modules='constraints' codegen-server-test:build ``` This model is _additionally_ generated in CI with the `publicConstrainedTypes` setting disabled: ```sh ./gradlew -P modules='constraints_without_public_constrained_types' codegen-server-test:build `````` Similarly, models using currently unsupported constraints are now being generated with the `ignoreUnsupportedConstraints` setting enabled. See `codegen-server-test/build.gradle.kts` for more details. [Constraint traits]: https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html [The RFC]: https://github.com/awslabs/smithy-rs/pull/1199 * Revamp errors in `aws-smithy-http` (#1884) * Revamp errors in `aws-smithy-json` (#1888) * Revamp errors in `aws-smithy-types` (#1893) * Revamp errors in `aws-smithy-types-convert` and `aws-smithy-xml` (#1917) * Revamp errors in AWS runtime crates (#1922) Revamp errors in: * `aws-types` * `aws-endpoint` * `aws-http` * `aws-sig-auth` * `aws-inlineable` * Revamp errors in `aws-config` (#1934) * Make `SdkError::into_service_error` infallible (#1974) * Implement RFC23 - Evolve the new service builder API (#1954) * Implement RFC 23, with the exception of PluginBuilder * Update documentation. * Elide implementation details in `Upgradable`. * Update wording in docs to avoid personal pronouns. * Update `Upgradable`'s documentation. * Template the service builder name. * Template MissingOperationsError directly. * Code-generate an array directly. * Update design/src/server/anatomy.md Co-authored-by: david-perez <[email protected]> * Use `rust` where we do not need templating. * Remove unused variable. * Add `expect` message to point users at our issue board in case a panic slips through. * Typo. * Update `expect` error message. * Refactor the `for` loop in ``requestSpecMap` into an `associateWith` call. * Fix new pokemon bin example. * Fix new pokemon bin example. * Fix unresolved symbolProvider. * Assign the `expect` error message to a variable. * Omit additional generic parameters in Upgradable when it's first introduced. Co-authored-by: david-perez <[email protected]> * Fix simple.smithy (#1991) Signed-off-by: Daniele Ahmed <[email protected]> * Add support for Endpoints 2.0 Parameters (#1953) * Add support for Endpoints 2.0 Parameters This commit adds `EndpointDecorator` which injects Endpoint parameters in to the operation property bag. These can come from an ordered list of sources—this wires them all up. To facilitate testing, this diff also writes the parameters into the property bag during operation generation. * remove println * CR Feedback * Implement RFC 23, Part 2 - Plugin pipeline (#1971) * Implement RFC 23, with the exception of PluginBuilder * Update documentation. * Avoid star re-exports. * Elide implementation details in `Upgradable`. * Update wording in docs to avoid personal pronouns. * Update `Upgradable`'s documentation. * Template the service builder name. * Template MissingOperationsError directly. * Code-generate an array directly. * Sketch out the implementation of `PluginPipeline`. Remove `PluginExt`. Add a public constructor for `FilterByOperationName`. * Ask for a `PluginPipeline` as input for the generated builder. * Rename `add` to `push`. * Remove Pluggable. Rename `composer` module to `pipeline`. * Remove all mentions of `Pluggable` from docs and examples. * Fix punctuation. * Rename variable from `composer` to `pipeline` in doc examples. * Add a free-standing function for filtering. * Rename. * Rename. * Update design/src/server/anatomy.md Co-authored-by: david-perez <[email protected]> * Use `rust` where we do not need templating. * Remove unused variable. * Add `expect` message to point users at our issue board in case a panic slips through. * Typo. * Update `expect` error message. * Refactor the `for` loop in ``requestSpecMap` into an `associateWith` call. * Remove unnecessary bound - since `new` is private, the condition is already enforced via `filter_by_operation_name`. * Use `Self` in `new`. * Rename `inner` to `into_inner` * Rename `concat` to `append` to correctly mirror Vec's API terminology. * Fix codegen to use renamed method. * Cut down the public API surface to key methods for a sequence-like interface. * Add a note about ordering. * Add method docs. * Add Default implementation. * Fix new pokemon bin example. * Fix new pokemon bin example. * Fix code-generated builder. * Fix unresolved symbolProvider. * Assign the `expect` error message to a variable. * Do not require a PluginPipeline as input to `builder_with_plugins`. * Reverse plugin application order. * Upgrade documentation. * Add a test to verify that plugin layers are executed in registration order. * Add license header. * Update middleware.md * Typo. * Fix more builder() calls. Co-authored-by: david-perez <[email protected]> * Improve error doc comments around logging errors (#1990) * Rework enum for forward-compatibility (#1945) * Make enum forward-compatible This commit implements the suggested approach described in https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092 The idea is that once the user writes a match expression against an enum and assumes that an execution path comes to a particular match arm, we should guarantee that when the user upgrades a version of SDK, the execution path should come to the same match arm as before. * Add unit test to ensure enums are forward-compatible The test first mimics the user's interaction with the enum generated from a model where the user writes a match expression on the enum, wishing to hit the match arm corresponding to Variant3, which is not yet supported by the model. The test then simulates a scenario where the user now has access to the updated enum generated from the next version of the model that does support Variant3. The user's code should compile in both of the use cases. * Generate rustdoc for enum's forward-compatibility This commits generates rustdoc explaining to the users how they might write a match expression against a generated enum so their code is forward-compatible. While it is explained in https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092, it can be helpful if the users can read it in rustdoc right below where the enum's definition is shown. * Make snippet in rustdoc text This commit updates a rustdoc code snippet generated for an enum from `rust,no_run` to `text`. We observed that the snippet fails to compile in CI because the name of the enum was not fully qualified; code in rustdoc is treated in a similar way that code in integration tests is treated as being outside of a crate, but not part of the crate, which means the name of the crate needs to be spelled out for a `use` statement if we want to import the enum to the code in rustdoc. However, the name of the crate is usually one-off, generated on the fly for testing, making it not obvious to hard-code it or obtain it programmatically from within Kotlin code. * Suppress missing doc lint for UnknownVariantValue This commit marks UnknownVariantValue as `#[allow(missing_docs)]` because failing to do so will cause tests in CI to fail. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: Russell Cohen <[email protected]> * Generate UnknownVariantValue via forInlineFun This commit attempts to create UnknownVariantValue using RuntimeType.forInlineFun. Prior to this commit, it was generated per enum but that caused multiple definitions of UnknownVariantValue in a single file as there can be multiple enums in the file. By using RuntimeType.forInlineFun, we can generate a single UnknownVariantValue per module and the enums within the module can refer to the same instance. This commit, however, fails to pass the unit tests in EnumGeneratorTest. The issue seems to be that a RustWriter used in the tests does not end up calling the finalize method, failing to render inline functions. * Replace "target == CodegenTarget.CLIENT" with a helper This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1014390811 but uses an existing helper CodegenTarget.renderUnknownVariant. * Update EnumGeneratorTests to use TestWorkspace This commit addresses failures for unit tests in EnumGeneratorTests. Now that we use RuntimeType.forInlineFcn in EnumGenerator, we need to ensure that the inline functions should be rendered to a Rust source file during testing. RustWriter, used in EnumGeneratorTests prior to this commit, was not capable of doing so. We thus update EnumGeneratorTests to use TestWorkspace by which we ensure that the inline functions are rendered during testing. * Make sure to use the passed-in variable for shapeId This commit fixes a copy-and-paste error introduced in 712c983. The function should use the passed-in variable rather than the hard-coded literal "test#SomeEnum". * Address https://github.com/awslabs/smithy-rs/pull/1945\#discussion_r1014389852 * Update CHANGELOG.next.toml * Update CHANGELOG.next.toml This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017235315 * Avoid potential name collisions by UnknownVariantValue This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017237745. It changes the module in which the `UnknownVariantValue` is rendered. Before the commit, it was emitted to the `model` module but that might cause name collisions in the future when a Smithy model has a shape named `UnknownVariantValue`. After this commit, we render it to the `types` module. * Move re-exports from lib.rs to types.rs This commit moves re-export statements in client crates from `lib.rs` to `types.rs`. The motivation is that now that we render the struct `UnknownVariantValue` in a separate file for the `types` module, we can no longer have a `pub mod types {...}` block in `lib.rs` as it cannot coexist with `pub mod types;`. * Add docs on UnknownVariantValue This commit adds public docs on `UnknownVariantValue`. Since it is rendered in `types.rs`, the users may not find the `Unknown` variant of an enum in the same file and may wonder what this struct is for when seeing it in isolation. * Update CHANGELOG.next.toml This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1020414459 * Update the module documentation for types This commit updates rustdoc for the types module to reflect that fact that the module now contains not only re-exports but also an auxiliary struct `UnknownVariantValue`. * Add extensions to run code block depending on the target This commit introduces extensions on CodegenTarget that execute the block of code depending on the target is for client or for server. These extensions are intended to replace if-else expressions throughout the codebase explicitly checking whether the target is for client or for server. We do not update such if-else expressions all at once in this commit. Rather, we are planning to make incremental changes to update them opportunistically. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: John DiSanti <[email protected]> * Move extensions into CodegenTarget as methods This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1021952411. By moving extensions into the CodegenTarget enum, no separate import is required to use them. Co-authored-by: Saito <[email protected]> Co-authored-by: Russell Cohen <[email protected]> Co-authored-by: Yuki Saito <[email protected]> Co-authored-by: John DiSanti <[email protected]> * fix: use idiomatic method of setting smithy gradle plugin version (#1995) * formatting: run code cleanup on entire project (#1993) * formatting: run code cleanup on entire project * revert: deletion of SdkSettings.defaultsConfigPath * Improve `Endpoint` panic-safety and ergonomics (#1984) - `Endpoint::set_endpoint` no longer panics when called on an endpoint without a scheme - `Endpoint::mutable` and `Endpoint::immutable` now both return a result so that constructing an endpoint without a scheme is an error - `Endpoint::mutable` and `Endpoint::immutable` both now take a string instead of a `Uri` as a convenience - `Endpoint::mutable_uri` and `Endpoint::immutable_uri` were added to construct an endpoint directly from a `Uri` * Remove `fromCoreProtocol` in favour of `as` downcasting (#2000) * Render full shape IDs in server error responses (#1982) RestJson1 implementations can denote errors in responses in several ways. New server-side protocol implementations MUST use a header field named `X-Amzn-Errortype`. Note that the spec says that implementations SHOULD strip the error shape ID's namespace. However, our server implementation renders the full shape ID (including namespace), since some existing clients rely on it to deserialize the error shape and fail if only the shape name is present. This is compliant with the spec, see https://github.com/awslabs/smithy/pull/1493. See https://github.com/awslabs/smithy/issues/1494 too. * fix: use new version of deprecated fn (#1994) update: crate-hasher sha256 dep to 1.1 * update: CargoDependency companion fn names for smithy runtime crates (#1996) * update: CargoDependency companion fn names for smithy runtime crates rename: CargoDependency.asType to CargoDependency.toType fix: errors in InlineDependency doc comment formatting: run import optimizer for all kotlin files fix: gradle issue with incorrectly named tasks * fix: server test broken by removal of rustName * Improve `Plugin` toolkit (#2003) Co-authored-by: Julian Antonielli <[email protected]> Co-authored-by: Luca Palmieri <[email protected]> * Multiple FromParts in handlers (#1999) * Multiple FromParts in handlers Handlers can have up to 8 extensions: ``` pub async fn handler( input: input::Input, ext0: Extension<...>, ext1: Extension<...>, ext2: Extension<...>, ext3: Extension<...>, ext4: Extension<...>, ext5: Extension<...>, ext6: Extension<...>, ext7: Extension<...>, ) -> Result<output::Output, error::Error> { ... } ``` Signed-off-by: Daniele Ahmed <[email protected]> Co-authored-by: Harry Barber <[email protected]> * Tidy up PR template slightly (#2006) * Re-export service from root of the generated crate (#2010) * Always write required query param keys, even if value is unset or empty (#1973) * fix: issues when sending multipart-upload-related operations with no upload ID by making upload ID required add: Mandatory trait add: S3 customization to make Upload Ids mandatory * add: CHANGELOG.next.toml entry * update: strategy for tagging shapes as mandatory * remove: Mandatory trait undo: S3Decorator changes add: codegen to generate "else" branch after required query string serialization `if let Some` check add: missing tokio feature to `TokioTest` * update: mandatory-query-param.rs tests * format: run cargo fmt * update: return BuildError for missing required query params update: required-query-params.rs tests * formatting: make RustWriter.paramList more readable fix: code broken by merge from `main` * fix: handling of enum strings * add: codegen tests for required query params update: required enum query param codegen add: smithy-rs CHANGELOG.next.toml entry * fix: presigning.rs test * fix: use `toType` instead of `asType` fix: indentation in test * Detect if the `uniqueItems` trait is used (#2001) And act like in the rest of the unsupported constraint traits cases. I missed this in the implementation of #1342. * fix: missing Tokio feature in S3 integration test (#2017) fix: remove extra "the" in CDK readme * Collect extractors into `request` module (#2008) * Add const to generated enum values() (#2011) * Add const to generated enum values() Enum values() functions return static arrays of static strings and could be of compile time use. Make callable in const contexts. * Add reference to PR in changelog next for const enum values() * Correct changelog next target to all for const enum values() Co-authored-by: Luca Palmieri <[email protected]> * Improve code generated documentation (#2019) Co-authored-by: Luca Palmieri <[email protected]> * Implement `@pattern` on `string` shapes (#1998) * Refactor `ConstrainedStringGenerator` to extract length checking * Extract `TryFrom` rendering into its own function * Extract `ConstraintViolation` definition to separate function * Add pattern check mock for `@pattern` trait * Add `regex` to list of server dependencies * Use real regex check in `check_pattern` * Add `@pattern` to traits that make `string` shapes directly constrained * Remove unsupported validation for `@pattern` on strings * Fix test cases in `ConstraintsTest` * Remove test disallowing `@pattern` on strings * Add first test case for `@pattern` on strings * Fix codegen in `ConstraintViolation::as_validation_exception_field` * Use `OnceCell` to store `@pattern`'s regex * Tidy up `ConstrainedStringGenerator` to work with many constraints * Rename `handleTrait` -> `fromTrait` and make it a static method * Add docs for `ConstraintViolation` variants * Fix unescaped slashes in regexes * Quick hack to get tests compiling * Fix `sensitive_string_display_implementation` test * Use new fn name: `asType` -> `toType` * Refactor `ConstrainedStringGenerator` to not pass in `constraints` * Add `@pattern` test * Use `Lazy` instead of `OnceCell` * Store `&'static str` in `Pattern` error instead of `String` * Move `TraitInfo` to the bottom * Extract branches in `TraitInfo.fromTrait` into separate functions * Add `PatternTrait.validationErrorMessage` * Add more tests for `@pattern` * Add entry to `CHANGELOG.next.toml` * Fix a `@length` + `@pattern` test case * Remove unused binding in `ConstrainedStringGeneratorTest` * Remove calls to `trimIndent` * Use raw Rust strings instead of `escapedPattern` * Require `regex 1.5.5` instead of `1.7.0` * Use `Writable`s in `TraitInfo` * Use single `rust` call for `impl $name` block * Move `supportedStringConstraintTraits` to `Constraints.kt` * Fix error message mentioning wrong `ignoreUnsupportedConstraintTraits` key * Fix usage of `:W` in `rustTemplate` and raw Rust strings * Use shorthand form for `PatternTrait.validationErrorMessage` * Fix protocol tests by adjusting the validation message * Add uses of `@pattern` in `constraints.smithy` model * Fix broken `RestJsonMalformedPatternReDOSString` test * Move `TraitInfo` to its own module and separate string-specific details * Update codegen-core/common-test-models/constraints.smithy Co-authored-by: david-perez <[email protected]> * Fix nits in `constraints.smithy` * Add license to `TraitInfo.kt` * Make `StringTraitInfo.fromTrait` not return a nullable * Remove overzealous formatting * Add some padding to json in `fixRestJsonMalformedPatternReDOSString` * Add `compile_regex` function for `@pattern` strings * Add helpful error message when regexes fail to compile * Add missing coverage in `constraints.smithy` * Fix examples in `constraints.smithy` * Fix failing test * Combine writables in `ConstrainedStringGenerator` * Fix uses of `Writable.join` * Use `expect` over `unwrap_or_else` * Use `once_cell::sync::Lazy` over `once_cell::sync::OnceCell` Co-authored-by: david-perez <[email protected]> * Overhaul RustModule system to support nested modules (#1992) * Overhaul RustModule system to support nested modules * Cleanups / CR feedback * Get server unit tests passing * Fix compilation * Remove unused import * CR Feedback II * customize isn't actually a reserved word—move it to the reserved member section * Improve service documentation (#2020) Co-authored-by: Luca Palmieri <[email protected]> * Add module documentation to `plugin.rs`. * Remove reference to `FromRequest` in `FromParts`. * Improve `Route` documentation. * @range implementation for integer shapes (#2005) * Draft of @range implementation for integer shapes * Green tests. * Fix serialization of constrained integers. * Fix tagger to include Integer simple shapes. Fix de/serialization codegen. * Add range trait to the entry about constraint traits in our changelog. * Bind a range-constrained integer to various HTTP parts to test our implementation. * Rename ConstrainedIntGeneratorTest to ConstrainedIntegerGeneratorTest for consistency. * Remove AsRef implementation. * Fix constraints models. * Fix conversion. * Use ReturnSymbolToParse to dry up. * Fix builder when constrained types should not be exposed. * Add customisation to unwrap constrained integers. * Getting closer - collections need to be handled differently to make everything fall into place. * Refactor `renderHeaders` * Fix constraints test. * Fix ebs. * Rename for clarity. * Remove unnecessary From implementation. * Rename `Size` variant to `Range`. * Remove stray comments. * Rename for clarity * Rename for consistency * Add test to guard against types for which we do not support range yet * DRY up branches and the relevant tests. * Fix header name. * Fix serialization bug for default values in headers. * Fix serialization issue for primitive headers. * Remove SetOfRangeInteger * Fix pubcrateconstrained unit test. * Remove duplication * Revert "Remove SetOfRangeInteger" This reverts commit f37a560bd0233ad65512c4916292f0f7f8c571e1. * Reintroduce `SetOfRangeInteger`, but keep them commented out. * Ignore leading whitespace. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt Co-authored-by: david-perez <[email protected]> * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/parse/JsonParserGenerator.kt Co-authored-by: david-perez <[email protected]> * Update codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt Co-authored-by: david-perez <[email protected]> * Update codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedIntegerGeneratorTest.kt Co-authored-by: david-perez <[email protected]> * Unify with a single rustTemplate invocation. * Rename `Type` to `NumberType` * Update codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProviderTest.kt Co-authored-by: david-perez <[email protected]> * Formatting issue. * Move and rename test helper. * Dry up the logic for default serialization. * Ooops, I swapped the two branches. * Add a wrapper block * Fix support detection. * Fix CHANGELOG. * Add (failing) protocol tests for @range on byte/integer/long. * Fix validation message. Fix test definitions. * Mark some tests as expected to fail. Rename service. * Use ValueExpression everywhere for more granular control of the ownership component. * Use the new enhanced module facilities * Fixes. * Fix formatting * Remove unnecessary when. * Update codegen-core/common-test-models/constraints.smithy Co-authored-by: david-perez <[email protected]> * Remove unused shapes. * Avoid mixing concerns in our test shapes for integer constraints. * Reuse the trait info machinery * Update stale comment * Fix unused attribute. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt Co-authored-by: Zelda Hessler <[email protected]> * Avoid unnecessary bindings by manipulating the serialization context directly in the customisations. * Avoid unnecessary bindings in header serialization by introducing and manipulating the header value serialization context directly in the customisations. * Add code examples. * Move `safeName` call into the if branch. Co-authored-by: david-perez <[email protected]> Co-authored-by: Zelda Hessler <[email protected]> * Fix Canary (#2016) * add: "release-2022-10-26" to canary runner PINNED_SMITHY_RS_VERSIONS * update: canary-runner README.md * Implement @range on short (#2034) * Extract a ConstrainedNumberGenerator from ConstrainedIntegerGenerator * Update tests' expectations to assume that we support @range for short * Add ShortShape to all the key places to get @range support * Generalize generator tests. * Add tests for the @range trait on short to our constraints.smithy model. * Update changelog * Fix type in constraints.smithy * Introduce an `aws-lambda` feature. (#2035) * Introduce an `aws-lambda` feature. * Add CHANGELOG * Use the new and shiny feature syntax to avoid creating an implicit `lambda_http` feature. * Add `aws-lambda` feature to the Python server. Enable the `aws-lambda` feature in our example. Be explicit in providing an implementation of request rejection for Box<dyn Error>. * Add an `aws-lambda` feature flag to the generated Python-specific crate. We enable it by default to make sure the Lambda method ends up visible in the final Python wrapper library. * Implement @range on long and byte shapes (#2036) * Group all override-related test cases together. * Add @range examples for both byte and long in constraints.smithy. * Implement @range for long and byte shapes. * Update CHANGELOG * Fix unit tests. * Address comments * Dry up further. * Upgrade cargo tools (#2032) * Upgrade `cargo-check-external-types` to 0.1.6 * Upgrade `cargo-deny` to 0.13.5 * Upgrade `cargo-udeps` to 0.1.35 * Upgrade `cargo-hack` to 0.5.23 * Upgrade `cargo-minimal-versions` to 0.1.8 * `CODEOWNERS`: Move Python related folders to `@awslabs/smithy-rs-python-server` team (#2033) * Move Python related folders to `@awslabs/smithy-rs-python-server` team * Add `@awslabs/smithy-rs-server` team to Python related folders Co-authored-by: Zelda Hessler <[email protected]> * Restore `into_make_service_with_connect_info` module (#2039) * RFC: RequestId for services (#1942) * RFC 24 RequestId Signed-off-by: Daniele Ahmed <[email protected]> * Add lambda extractors (#2038) * Implement `@length` on collections (#2028) * Refactor `ConstrainedMapGenerator` slightly * Add `@length` list operation in `constraints.smithy` * Add more setup required for rendering constraint `list`s * Add initial draft of `ConstrainedCollectionGenerator` * Add license header to `LengthTraitCommon` * Add draft of collection constraint violation generation Copy `MapCostraintViolationGenerator` into `CollectionConstraintViolationGenerator` for now. * Add `Visibility.toRustQualifier` * Implement `CollectionConstraintViolationGenerator` * Use `TraitInfo` on `CollectionConstraintViolationGenerator` * Update docs on `ConstrainedCollectionGenerator` * Improve formatting on rust code * Don't generate `ConstraintViolation` enum twice * Make symbol provider work with constrained lists * Fix call to `ConstraintViolation::Member` * Render constraint validation functions for lists * Fix call to `usize::to_string` * Use map json customisation for collections as well * Update to use overhauled module system * Add constraint traits check for collection shapes * Remove test checking that `@length` is not used in `list`s * Refactor use of `shape.isDirectlyConstrained` * Fix failing `UnconstrainedCollectionGeneratorTest` test * Fix test * Actually fix the test * Update changelog * Fix `constrainedCollectionCheck` * Add tests for `ConstrainedCollectionGenerator` * Make tests work for when sets are implemented * Remove mention of `set` in changelog Co-authored-by: david-perez <[email protected]> * Fix styling in `constraints.smithy` Co-authored-by: david-perez <[email protected]> * Use `check` instead of `assert` * Grammar fix in comment about `Option<Box<_>>` * Rename unsupported length data class for blobs - `UnsupportedLengthTraitOnCollectionOrOnBlobShape` -> `UnsupportedLengthTraitOnBlobShape` * Add TODO about `uniqueItems` not being implemented yet * Change error message: `unconstrained` -> `not constrained` * Add imports to fix docs * Remove unused `modelsModuleWriter` parameter * Use `filterIsInstance` and coalesce `filter + map` * Add `check` in json customization * Use `Set` instead of `List` for supported contraints * Use `toMutableList` to avoid creating an extra list * Add `@length` list to `ConA` * Add `@httpQuery`-bound `@length` list example * Add `@httpHeader`-bound `@length` list * Fix `UnconstrainedCollectionGeneratorTest` test * Fix rendering of constrained lists as header values * Split very long line * Add docs for `ConstraintViolation::Member` for lists * Pass `length` variable name to `LengthTrait.rustCondition` * Refactor long conditional * Homogenise conditional Co-authored-by: david-perez <[email protected]> * Improve extractor errors (#2041) Co-authored-by: Luca Palmieri <[email protected]> * Document the new service builder (#2021) * Document the new service builder Signed-off-by: Daniele Ahmed <[email protected]> * The TODO comment should not be visible in the public docs. (#2045) * Add server SDK constraint traits RFC (#1199) * Add Endpoint Resolver Implementation (#2030) * Add Endpoint Resolver Implementation This commit adds `EndpointDecorator`, standard libary + tests that can be used to add endpoints 2.0 to the AWS SDK. It _does not_ actually wire these things up. We'll follow up with a PR that actually integrates everything. * CR Feedback * CR feedback II * Add default for the Smithy test workspace (#1981) * Add default for the Smithy test workspace * CR feedback * Upgrade test SDK models to IDLv2 (#2049) * Various small corrections to server documentation (#2050) * Link to super from Handler and OperationService * Note that structure documentation is from model * Don't refer to ZSTs in operation_shape.rs opener * Re-export everything from service to root * Add reference to root documentation on service * Fix spelling of MissingOperationsError * #[doc(hidden)] for opaque_future * Rename from-parts.md to from_parts.md * Fix Connected link * Use S type parameter and service as variable name * Remove MissingOperation dead code * Remove Operation header from operation.rs * Remove reference to closures * Document ConnectInfo<T> .0 * Add BoxBody documentation * Rephrase operation.rs documentation * Reference PluginPipeline from PluginStack * Move the BoxBody documentation * Document Plugin associated types * Add example implementation for Plugin * Link to plugin module from Plugin * Improve FromParts/FromRequest documentation * Remove links to doc(hidden) RuntimeError * Add link from Upgradable to operation module * Fix service builder docs (#2046) * Fix doc prefix in service generator * Move documentation to root module * Documentation improvements * Expose handler imports Signed-off-by: Daniele Ahmed <[email protected]> Co-authored-by: Harry Barber <[email protected]> * Unhide new service builder and deprecate the prior (#1886) Co-authored-by: david-perez <[email protected]> * Normalize URI paths during signing (except for S3) (#2018) * De-dupe multiple leading forward slashes in request This commit fixes a bug reported in https://github.com/awslabs/aws-sdk-rust/issues/661. Note that this de-duping is not part of URI path normalization because it must happen to a request for any service, including S3. * Normalize URI paths for canonical requests This commit enables URI path normalization in aws-sigv4. It follows https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html to create the canonical URI that is part of the canonical request. * Avoid using OS specific path separator This commit updates the implementation of normalize_path_segment so that it does not use an OS specific path separator, specifically "\\" on Windows and always uses a forward slash "/". It addresses a test failure on Windows: https://github.com/awslabs/smithy-rs/actions/runs/3518627877/jobs/5897764889. * Fix the rustdoc::bare_urls warning from cargo doc * Fix a typo Normalzie -> Normalize * Limit de-duping multiple leading forward slashes to S3 This commit addresses https://github.com/awslabs/smithy-rs/pull/2018#issuecomment-1325283254. There is a case where an empty string between leading double forward slashes is meaningful so blindly de-duping them breaks the use case. Therefore, handling multiple leading forward slashes in this manner should be applied only to S3. * Add an integration test per https://github.com/awslabs/aws-sdk-rust/issues/661 This commit adds an integration test verifying the special rule for URI path normalization applies to S3. That is, leading forward slashes should be de-deduped and normalizing URI paths as prescribed in RFC 3986 should be ignored. * Update CHANGELOG.next.toml * Revert 87b749c This commit reverts 87b749c, the reason being that we need to better understand the requirement for de-duplicating leading forward slashes in requests, i.e. do we even need to do it or if so where should we perform the de-dupe operation? * Revert naming of UriPathNormalizationMode variants This commit reverts the naming of variants in UriPathNormalizationMode. They were PerRfc3986 and ForS3, but they should be more generic like Enabled and Disabled as we had before. * Update CHANGELOG.next.toml * Simplify integration test using capture_request This commit simplifies the integration test normalize-uri-path for s3. Previously, the test used ReplayingConnection but it was an overkill to test something as simple as whether the generated request contained the expected URI and signature. * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <[email protected]> * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <[email protected]> * Update aws/rust-runtime/aws-sigv4/src/http_request/uri_path_normalization.rs Co-authored-by: John DiSanti <[email protected]> * Address review feedback on normalize_path_segment This commit addresses the following code review feedback: https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034192229 https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034194621 https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034195625 * Update CHANGELOG.next.toml This commit addresses https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034198861 * Preallocate the Vec capacity for normalize_path_segment This commit addresses https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034195625. It counts the number of slashes to preallocate the capacity of a Vec used in normalize_path_segment. * Address https://github.com/awslabs/smithy-rs/pull/2018\#discussion_r1034188849 * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <[email protected]> * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <[email protected]> * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <[email protected]> Co-authored-by: Yuki Saito <[email protected]> Co-authored-by: John DiSanti <[email protected]> * Fix inability to create service clients when `default-features = false` (#2055) * add: tests for config/client construction when default features are disabled fix: enable users to create service clients when default features are disabled update: missing HTTP connector panic messages * Apply suggestions from code review Co-authored-by: John DiSanti <[email protected]> * Fix server book documentation links (#2059) * Respect the sensitive trait on container shapes (#1983) * Make enum forward-compatible This commit implements the suggested approach described in https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092 The idea is that once the user writes a match expression against an enum and assumes that an execution path comes to a particular match arm, we should guarantee that when the user upgrades a version of SDK, the execution path should come to the same match arm as before. * Add unit test to ensure enums are forward-compatible The test first mimics the user's interaction with the enum generated from a model where the user writes a match expression on the enum, wishing to hit the match arm corresponding to Variant3, which is not yet supported by the model. The test then simulates a scenario where the user now has access to the updated enum generated from the next version of the model that does support Variant3. The user's code should compile in both of the use cases. * Generate rustdoc for enum's forward-compatibility This commits generates rustdoc explaining to the users how they might write a match expression against a generated enum so their code is forward-compatible. While it is explained in https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092, it can be helpful if the users can read it in rustdoc right below where the enum's definition is shown. * Make snippet in rustdoc text This commit updates a rustdoc code snippet generated for an enum from `rust,no_run` to `text`. We observed that the snippet fails to compile in CI because the name of the enum was not fully qualified; code in rustdoc is treated in a similar way that code in integration tests is treated as being outside of a crate, but not part of the crate, which means the name of the crate needs to be spelled out for a `use` statement if we want to import the enum to the code in rustdoc. However, the name of the crate is usually one-off, generated on the fly for testing, making it not obvious to hard-code it or obtain it programmatically from within Kotlin code. * Suppress missing doc lint for UnknownVariantValue This commit marks UnknownVariantValue as `#[allow(missing_docs)]` because failing to do so will cause tests in CI to fail. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: Russell Cohen <[email protected]> * Generate UnknownVariantValue via forInlineFun This commit attempts to create UnknownVariantValue using RuntimeType.forInlineFun. Prior to this commit, it was generated per enum but that caused multiple definitions of UnknownVariantValue in a single file as there can be multiple enums in the file. By using RuntimeType.forInlineFun, we can generate a single UnknownVariantValue per module and the enums within the module can refer to the same instance. This commit, however, fails to pass the unit tests in EnumGeneratorTest. The issue seems to be that a RustWriter used in the tests does not end up calling the finalize method, failing to render inline functions. * Replace "target == CodegenTarget.CLIENT" with a helper This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1014390811 but uses an existing helper CodegenTarget.renderUnknownVariant. * Update EnumGeneratorTests to use TestWorkspace This commit addresses failures for unit tests in EnumGeneratorTests. Now that we use RuntimeType.forInlineFcn in EnumGenerator, we need to ensure that the inline functions should be rendered to a Rust source file during testing. RustWriter, used in EnumGeneratorTests prior to this commit, was not capable of doing so. We thus update EnumGeneratorTests to use TestWorkspace by which we ensure that the inline functions are rendered during testing. * Make sure to use the passed-in variable for shapeId This commit fixes a copy-and-paste error introduced in 712c983. The function should use the passed-in variable rather than the hard-coded literal "test#SomeEnum". * Address https://github.com/awslabs/smithy-rs/pull/1945\#discussion_r1014389852 * Update CHANGELOG.next.toml * Update CHANGELOG.next.toml This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017235315 * Avoid potential name collisions by UnknownVariantValue This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017237745. It changes the module in which the `UnknownVariantValue` is rendered. Before the commit, it was emitted to the `model` module but that might cause name collisions in the future when a Smithy model has a shape named `UnknownVariantValue`. After this commit, we render it to the `types` module. * Move re-exports from lib.rs to types.rs This commit moves re-export statements in client crates from `lib.rs` to `types.rs`. The motivation is that now that we render the struct `UnknownVariantValue` in a separate file for the `types` module, we can no longer have a `pub mod types {...}` block in `lib.rs` as it cannot coexist with `pub mod types;`. * Add docs on UnknownVariantValue This commit adds public docs on `UnknownVariantValue`. Since it is rendered in `types.rs`, the users may not find the `Unknown` variant of an enum in the same file and may wonder what this struct is for when seeing it in isolation. * Update CHANGELOG.next.toml This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1020414459 * Update the module documentation for types This commit updates rustdoc for the types module to reflect that fact that the module now contains not only re-exports but also an auxiliary struct `UnknownVariantValue`. * Add extensions to run code block depending on the target This commit introduces extensions on CodegenTarget that execute the block of code depending on the target is for client or for server. These extensions are intended to replace if-else expressions throughout the codebase explicitly checking whether the target is for client or for server. We do not update such if-else expressions all at once in this commit. Rather, we are planning to make incremental changes to update them opportunistically. * Respect the sensitive trait on enums This commit fixes #1745. It allows enums to respect the sensitive trait. When annotated as such, an enum will not display its data and instead will show the redacted text upon debug print. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: John DiSanti <[email protected]> * Move extensions into CodegenTarget as methods This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1021952411. By moving extensions into the CodegenTarget enum, no separate import is required to use them. * Update codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt Co-authored-by: david-perez <[email protected]> * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: david-perez <[email protected]> * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: david-perez <[email protected]> * Update CHANGELOG.next.toml * Configure the (in|ex)clusion of the Debug trait for containers per members' sensitive trait (#2029) * Removes Debug for shape with sensitive trait This commit centralizes a place where the codegen excludes the Debug trait if a shape has the sensitive trait. Previously the exclusion was handled locally in each shape, e.g. StructureGenerator and EnumGenerator. However, that approach may overlook a certain shape we also need to treat as such. We now handle the exclusion of the Debug trait in one place, BaseSymbolMetadataProvider. * Stop excluding the Debug trait locally This commit updates EnumGenerator and StructureGenerator based on the change made to BaseSymbolMetadataProvider in the previous commit. Now that the exclusion of the Debug trait was centralized, those classes in question no longer need to do so individually. * Implement a custom Debug trait in BuilderGenerator This commit implements a custom Debug trait in BuilderGenerator now that the derived Debug trait is excluded from BaseSymbolMetadataProvider for the structure container. The implementation of the custom Debug trait pretty much follows that of StructureGenerator. * Implement a custom Debug trait in UnionGenerator This commit implements a custom Debug trait in BuilderGenerator now that the derived Debug trait is excluded from BaseSymbolMetadataProvider for the union container. The implementation of the custom Debug trait pretty much follows that of EnumGenerator. * Implement a custom Debug trait in ServerBuilderGenerator This commit implements a custom Debug trait in ServerBuilderGenerator now that the derived Debug trait is excluded from BaseSymbolMetadataProvider for the structure container. The implementation of the custom Debug trait pretty much follows that of StructureGenerator. * Add the Copyright header * Update CHANGELOG.next.toml * Update Debug impl for UnionGenerator This commit updates the implementation of a custom Debug trait impl for UnionGenerator. Turns out that in a Union, a member target can be marked as sensitive separately outside the Union. Therefore, the implementation of a custom Debug trait has two cases depending on where the sensitive trait appears, either it is applied to the whole Union or to a member target. * Peek at member sensitivity for Debug trait (in|ex)clusion This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1034205685. With this change, structure shapes no longer need to exclude the Debug trait unconditionally. The upshot is that we may be able to avoid a custom Debug impl for a structure where the derived Debug will do, i.e. when there is no sensitive trait either at a container level or at a member level. * Remove statement that does not seem to take effect This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036288146 * Rename renderDebugImplForUnion -> renderFullyRedactedDebugImpl This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036290722 * Rename renderDebugImplForUnionMemberWise -> renderDebugImpl This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036291209 Co-authored-by: Yuki Saito <[email protected]> Co-authored-by: Saito <[email protected]> Co-authored-by: Russell Cohen <[email protected]> Co-authored-by: Yuki Saito <[email protected]> Co-authored-by: John DiSanti <[email protected]> Co-authored-by: david-perez <[email protected]> * Tracing output json (#2060) * Tracing output json Signed-off-by: Daniele Ahmed <[email protected]> * Remove client body callbacks (#2065) * Implement FromParts for Option, Result (#2068) * Implement FromParts for Option, Result Signed-off-by: Daniele Ahmed <[email protected]> * Endpoints 2.0 Integration pre-work (#2063) * Split endpoint resolution middleware into two parts & refactor endpoint generation * Endpoints 2.0 Integration pre-work This PR does a 3 bits of pre-work ahead of ep2 integration: 1. Split endpoint resolution into two separate middlewares: 1. A smithy native middleware that applies URI and headers 2. An AWS middleware that applies the auth schemes 2. Add vendorParams support to the ProtocolTestGenerator so that protocol tests can insert a region. 3. Simplify endpoint resolution logic by allowing `make_operation` to fail when an endpoint cannot be resolved. * Back out previous change to insert endpoint directly into the bag * backout changes to property bag * Update changelog & add more docs * Fix AWS test * Fix test * allow parse_url functions to be unused (#2071) * Establish default max idle connections on default connectors (#2064) * Render a Union member of type Unit to an enum variant without inner data (#1989) * Avoid explicitly emitting Unit type within Union This commit addresses #1546. Previously, the Unit type in a Union was rendered as an enum variant whose inner data was crate::model::Unit. The way such a variant appears in Rust code feels a bit odd as it usually does not carry inner data for `()`. We now render a Union member of type Unit to an enum variant without inner data. * Address test failures washed out in CI This commit updates places that need to be aware of the Unit type attached to a Union member. Those places will render the Union member in a way that the generated Rust code does not include inner data of type Unit. It should help pass `codegen-client-test:test` and `codegen-server-test:test`. Further refactoring is required because each place we updated performs an explicit if-else check for special casing, which we should avoid. * Update codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt Co-authored-by: John DiSanti <[email protected]> * Remove commented-out code * Add a helper for comparing against ShapeId for Unit This commit adds a helper for checking whether MemberShape targets the ShapeId of the Unit type. The benefit of the helper is that each call site does not have to construct a ShapeId for the Unit type every time comparison is made. * Move Unit type bifurcation logic to jsonObjectWriter This commit moves the if-else expression hard-coded in the StructureShape matching case within RustWriter.serializeMemberValue to jsonObjectWriter. The previous approach of inlining the if-else expression out of jsonObjectWriter to the StructureShape case and tailoring it to the call site violated the DRY principle. * Make QuerySerializerGenerator in sync with the change This commit brings the Unit type related change we've made so far to QuerySerializerGenerator. All tests in CI passed even without this commit, but noticing how similar QuerySerializerGenerator is to JsonSerializerGenerator, we should make the former in sync with the latter. * Update CHANGELOG.next.toml * Refactor ofTypeUnit -> isTargetUnit This commit addresses https://github.com/awslabs/smithy-rs/pull/1989#discussion_r1035372417. The extension should be renamed to isTargetUnit and moved to Smithy.kt. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt Co-authored-by: John DiSanti <[email protected]> * Simplify if-else in jsonObjectWriter This commit addresses https://github.com/awslabs/smithy-rs/pull/1989#discussion_r1037651893 * Avoid the union member's reference name being empty This commit addresses https://github.com/awslabs/smithy-rs/pull/1989#discussion_r1037654601 Even if member's reference name "inner" is present, it will not be an issue when the member is the Unit type where the reference name "inner" cannot be extracted. The reason is jsonObjectWriter won't render the serialization function if the member is the Unit type. That said, the same change in QuerySerializerGenerator may not be the case so we'll run the tests in CI and see what breaks. * Ensure Union with Unit target can be serialized This commit updates serialization of a Union with a Unit target in different types of serializers. We first updated protocol tests by adding a new field of type Unit and some of them failed as expected. We worked our way back from those failed tests and fixed the said implementation for serialization accordingly. * Ensure Union with Unit target can be parsed This commit handles deserialization of a Union with a Unit target in XmlBindingTraitParserGenerator. Prior to the commit, we already handled the said deserialization correctly in JsonParserGenerator but missed it in XmlBindingTraitParserGenerator. We added a new field of type Unit to a Union in parser protocols tests, ran them, and worked our way back from the failed tests. * Ensure match arm for Unit works in custom Debug impl This commit handles a use case that came up as a result of combining two updates, implementing a custom Debug impl for a Union and avoid rendering the Unit type in a Union. In the custom Debug impl, we now make sure that if the target is of type Unit, we render the corresponding match arm without the inner data. We add a new unit test in UnionGeneratorTest. * Fix unused variables warnings in CI This commit adds the #[allow(unused_variables)] annotation to a block of code generated for a member being the Unit type. The annotation is put before we call the handleOptional function so that it covers the whole block that the function generates. * Fix E0658 on unused_variables This commit fixes an error where attributes on expressions are experimental. It does so by rearranging code in a way that achieves the same effect but without #[allow(unused_variables)] on an expression. Co-authored-by: Yuki Saito <[email protected]> Co-authored-by: John DiSanti <[email protected]> * Improve client tracing spans (#2044) * Emit spans for implementers of map request middleware traits * Instrument dispatch with its own span * Fix trace span hierarchy * Partially flatten the middleware span hierarchy * Make `MapRequest::name` required * Add sub-spans to the `load_response` span * Add `tracing` events to signing an…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
This PR revamps errors in
aws-smithy-httpto adhere to RFC-0022: Error Context and Compatibility.This PR is part of a series that will fully implement the RFC, tracked in #1926.
Changelog entries added in #1951.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.