From 8439f2ae43e02b93f4aecd42a48673c1490b919a Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Thu, 12 Oct 2023 16:49:26 -0500 Subject: [PATCH] Move `aws_smithy_http::operation::error::BuildError` into `aws-smithy-types` (#3032) ## Motivation and Context Takes care of the first part of https://github.com/awslabs/smithy-rs/issues/3054 (the remaining part is denoted as `TODO(runtimeCratesVersioningCleanup)` and until that's done the issue will not be closed). ## Description This PR moves from `aws-smithy-http::operation::error::{BuildError, SerializationError}` to `aws-smithy-types::error::operation::{BuildError, SerializationError}`. Within the origin crate, we leave "breadcrumbs" (i.e. reexports) for existing customers of the items above so they are not broken by the move. The breadcrumps will be removed by the part two of the issue. As part of the move, `aws_smithy_http::endpoint::EndpointPrefix::new` now returns its `crate::endpoint::error::InvalidEndpointError` instead of `crate::operation::error::BuildError` for two reasons: - `BuildError` is now in a stable crate and for an `InvalidUri` variant to be created, it needed to take a `http::uri::InvalidUri` from a unstable crate, which we cannot expose in public API. - The new error is more closely related to the endpoint business. ## Testing Relied on the tests in CI. ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: John DiSanti --- CHANGELOG.next.toml | 8 +++++ .../EndpointTraitBindingGenerator.kt | 10 +++---- .../generators/EndpointTraitBindingsTest.kt | 10 +++---- rust-runtime/aws-smithy-http/src/endpoint.rs | 21 +++++++------- .../aws-smithy-http/src/endpoint/error.rs | 25 +++++++++++----- rust-runtime/aws-smithy-http/src/operation.rs | 7 ++++- rust-runtime/aws-smithy-types/src/error.rs | 1 + .../src/error/operation.rs} | 29 ++----------------- 8 files changed, 52 insertions(+), 59 deletions(-) rename rust-runtime/{aws-smithy-http/src/operation/error.rs => aws-smithy-types/src/error/operation.rs} (84%) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 072b073745..c6be15ef79 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -335,3 +335,11 @@ message = "The future return types on traits `EndpointResolver` and `IdentityRes references = ["smithy-rs#3055"] meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" } author = "jdisanti" + +[[smithy-rs]] +message = """ +[`EndpointPrefix::new`](https://docs.rs/aws-smithy-http/latest/aws_smithy_http/endpoint/struct.EndpointPrefix.html#method.new) no longer returns `crate::operation::error::BuildError` for an Err variant, instead returns a more specific [`InvalidEndpointError`](https://docs.rs/aws-smithy-http/latest/aws_smithy_http/endpoint/error/struct.InvalidEndpointError.html). +""" +references = ["smithy-rs#3032"] +meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" } +author = "ysaito1001" diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingGenerator.kt index 41e76cc498..fe373a13bb 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingGenerator.kt @@ -16,7 +16,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider -import software.amazon.smithy.rust.codegen.core.smithy.generators.OperationBuildError import software.amazon.smithy.rust.codegen.core.smithy.isOptional import software.amazon.smithy.rust.codegen.core.util.inputShape @@ -72,18 +71,17 @@ class EndpointTraitBindings( rust("let $field = &$input.$field;") } if (generateValidation) { + val errorString = "$field was unset or empty but must be set as part of the endpoint prefix" val contents = """ if $field.is_empty() { - return Err(#{invalidFieldError:W}.into()) + return Err(#{InvalidEndpointError}::failed_to_construct_uri("$errorString").into()); } """ rustTemplate( contents, - "invalidFieldError" to OperationBuildError(runtimeConfig).invalidField( - field, - "$field was unset or empty but must be set as part of the endpoint prefix", - ), + "InvalidEndpointError" to RuntimeType.smithyHttp(runtimeConfig) + .resolve("endpoint::error::InvalidEndpointError"), ) } "${label.content} = $field" diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingsTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingsTest.kt index 3d1dcd009d..a6f699dae0 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingsTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingsTest.kt @@ -16,10 +16,9 @@ import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency import software.amazon.smithy.rust.codegen.core.rustlang.RustModule import software.amazon.smithy.rust.codegen.core.rustlang.implBlock import software.amazon.smithy.rust.codegen.core.rustlang.rust -import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType -import software.amazon.smithy.rust.codegen.core.smithy.generators.operationBuildError import software.amazon.smithy.rust.codegen.core.testutil.TestRuntimeConfig import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel @@ -70,10 +69,9 @@ internal class EndpointTraitBindingsTest { """, ) implBlock(symbolProvider.toSymbol(model.lookup("test#GetStatusInput"))) { - rustBlock( - "fn endpoint_prefix(&self) -> std::result::Result<#T::endpoint::EndpointPrefix, #T>", - RuntimeType.smithyHttp(TestRuntimeConfig), - TestRuntimeConfig.operationBuildError(), + rustBlockTemplate( + "fn endpoint_prefix(&self) -> std::result::Result<#{endpoint}::EndpointPrefix, #{endpoint}::error::InvalidEndpointError>", + "endpoint" to RuntimeType.smithyHttp(TestRuntimeConfig).resolve("endpoint"), ) { endpointBindingGenerator.render(this, "self") } diff --git a/rust-runtime/aws-smithy-http/src/endpoint.rs b/rust-runtime/aws-smithy-http/src/endpoint.rs index 8496239787..03759cc531 100644 --- a/rust-runtime/aws-smithy-http/src/endpoint.rs +++ b/rust-runtime/aws-smithy-http/src/endpoint.rs @@ -6,7 +6,6 @@ //! Code for resolving an endpoint (URI) that a request should be sent to use crate::endpoint::error::InvalidEndpointError; -use crate::operation::error::BuildError; use aws_smithy_types::config_bag::{Storable, StoreReplace}; use http::uri::{Authority, Uri}; use std::borrow::Cow; @@ -104,15 +103,13 @@ impl ResolveEndpoint for Endpoint { pub struct EndpointPrefix(String); impl EndpointPrefix { /// Create a new endpoint prefix from an `impl Into`. If the prefix argument is invalid, - /// a [`BuildError`] will be returned. - pub fn new(prefix: impl Into) -> StdResult { + /// a [`InvalidEndpointError`] will be returned. + pub fn new(prefix: impl Into) -> StdResult { let prefix = prefix.into(); match Authority::from_str(&prefix) { Ok(_) => Ok(EndpointPrefix(prefix)), - Err(err) => Err(BuildError::invalid_uri( - prefix, - "invalid prefix".into(), - err, + Err(err) => Err(InvalidEndpointError::failed_to_construct_authority( + prefix, err, )), } } @@ -142,11 +139,13 @@ pub fn apply_endpoint( .map(|auth| auth.as_str()) .unwrap_or(""); let authority = if !prefix.is_empty() { - Authority::from_str(&format!("{}{}", prefix, authority)) + Cow::Owned(format!("{}{}", prefix, authority)) } else { - Authority::from_str(authority) - } - .map_err(InvalidEndpointError::failed_to_construct_authority)?; + Cow::Borrowed(authority) + }; + let authority = Authority::from_str(&authority).map_err(|err| { + InvalidEndpointError::failed_to_construct_authority(authority.into_owned(), err) + })?; let scheme = *endpoint .scheme() .as_ref() diff --git a/rust-runtime/aws-smithy-http/src/endpoint/error.rs b/rust-runtime/aws-smithy-http/src/endpoint/error.rs index bab0f93ac6..eeb703523c 100644 --- a/rust-runtime/aws-smithy-http/src/endpoint/error.rs +++ b/rust-runtime/aws-smithy-http/src/endpoint/error.rs @@ -54,6 +54,7 @@ impl Error for ResolveEndpointError { pub(super) enum InvalidEndpointErrorKind { EndpointMustHaveScheme, FailedToConstructAuthority { + authority: String, source: Box, }, FailedToConstructUri { @@ -69,23 +70,28 @@ pub struct InvalidEndpointError { } impl InvalidEndpointError { - pub(super) fn endpoint_must_have_scheme() -> Self { + /// Construct a build error for a missing scheme + pub fn endpoint_must_have_scheme() -> Self { Self { kind: InvalidEndpointErrorKind::EndpointMustHaveScheme, } } - pub(super) fn failed_to_construct_authority( + /// Construct a build error for an invalid authority + pub fn failed_to_construct_authority( + authority: impl Into, source: impl Into>, ) -> Self { Self { kind: InvalidEndpointErrorKind::FailedToConstructAuthority { + authority: authority.into(), source: source.into(), }, } } - pub(super) fn failed_to_construct_uri( + /// Construct a build error for an invalid URI + pub fn failed_to_construct_uri( source: impl Into>, ) -> Self { Self { @@ -105,11 +111,11 @@ impl From for InvalidEndpointError { impl fmt::Display for InvalidEndpointError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use InvalidEndpointErrorKind as ErrorKind; - match self.kind { + match &self.kind { ErrorKind::EndpointMustHaveScheme => write!(f, "endpoint must contain a valid scheme"), - ErrorKind::FailedToConstructAuthority { .. } => write!( + ErrorKind::FailedToConstructAuthority { authority, source: _ } => write!( f, - "endpoint must contain a valid authority when combined with endpoint prefix" + "endpoint must contain a valid authority when combined with endpoint prefix: {authority}" ), ErrorKind::FailedToConstructUri { .. } => write!(f, "failed to construct URI"), } @@ -120,8 +126,11 @@ impl Error for InvalidEndpointError { fn source(&self) -> Option<&(dyn Error + 'static)> { use InvalidEndpointErrorKind as ErrorKind; match &self.kind { - ErrorKind::FailedToConstructUri { source } - | ErrorKind::FailedToConstructAuthority { source } => Some(source.as_ref()), + ErrorKind::FailedToConstructUri { source } => Some(source.as_ref()), + ErrorKind::FailedToConstructAuthority { + authority: _, + source, + } => Some(source.as_ref()), ErrorKind::EndpointMustHaveScheme => None, } } diff --git a/rust-runtime/aws-smithy-http/src/operation.rs b/rust-runtime/aws-smithy-http/src/operation.rs index d31997c85e..0fed557b79 100644 --- a/rust-runtime/aws-smithy-http/src/operation.rs +++ b/rust-runtime/aws-smithy-http/src/operation.rs @@ -9,7 +9,12 @@ use aws_smithy_types::config_bag::{Storable, StoreReplace}; use std::borrow::Cow; -pub mod error; +//TODO(runtimeCratesVersioningCleanup): Re-point those who use the following reexport to +// directly depend on `aws_smithy_types` and remove the reexport below. +/// Errors for operations +pub mod error { + pub use aws_smithy_types::error::operation::{BuildError, SerializationError}; +} /// Metadata added to the [`ConfigBag`](aws_smithy_types::config_bag::ConfigBag) that identifies the API being called. #[derive(Clone, Debug)] diff --git a/rust-runtime/aws-smithy-types/src/error.rs b/rust-runtime/aws-smithy-types/src/error.rs index b83d6ffe07..96a48441e0 100644 --- a/rust-runtime/aws-smithy-types/src/error.rs +++ b/rust-runtime/aws-smithy-types/src/error.rs @@ -9,6 +9,7 @@ use std::fmt; pub mod display; pub mod metadata; +pub mod operation; mod unhandled; pub use metadata::ErrorMetadata; diff --git a/rust-runtime/aws-smithy-http/src/operation/error.rs b/rust-runtime/aws-smithy-types/src/error/operation.rs similarity index 84% rename from rust-runtime/aws-smithy-http/src/operation/error.rs rename to rust-runtime/aws-smithy-types/src/error/operation.rs index 5b2024396f..5b61f1acfe 100644 --- a/rust-runtime/aws-smithy-http/src/operation/error.rs +++ b/rust-runtime/aws-smithy-types/src/error/operation.rs @@ -5,9 +5,7 @@ //! Errors for operations -use aws_smithy_types::date_time::DateTimeFormatError; -use http::uri::InvalidUri; -use std::borrow::Cow; +use crate::date_time::DateTimeFormatError; use std::error::Error; use std::fmt::{Display, Formatter}; @@ -80,15 +78,6 @@ enum BuildErrorKind { /// The serializer could not serialize the input SerializationError(SerializationError), - /// The serializer did not produce a valid URI - /// - /// This typically indicates that a field contained invalid characters. - InvalidUri { - uri: String, - message: Cow<'static, str>, - source: InvalidUri, - }, - /// An error occurred request construction Other(Box), } @@ -104,16 +93,6 @@ pub struct BuildError { } impl BuildError { - pub(crate) fn invalid_uri(uri: String, message: Cow<'static, str>, source: InvalidUri) -> Self { - Self { - kind: BuildErrorKind::InvalidUri { - uri, - message, - source, - }, - } - } - /// Construct a build error for a missing field pub fn missing_field(field: &'static str, details: &'static str) -> Self { Self { @@ -121,7 +100,7 @@ impl BuildError { } } - /// Construct a build error for a missing field + /// Construct a build error for an invalid field pub fn invalid_field(field: &'static str, details: impl Into) -> Self { Self { kind: BuildErrorKind::InvalidField { @@ -165,9 +144,6 @@ impl Display for BuildError { BuildErrorKind::SerializationError(_) => { write!(f, "failed to serialize input") } - BuildErrorKind::InvalidUri { uri, message, .. } => { - write!(f, "generated URI `{uri}` was not a valid URI: {message}") - } BuildErrorKind::Other(_) => { write!(f, "error during request construction") } @@ -180,7 +156,6 @@ impl Error for BuildError { match &self.kind { BuildErrorKind::SerializationError(source) => Some(source as _), BuildErrorKind::Other(source) => Some(source.as_ref()), - BuildErrorKind::InvalidUri { source, .. } => Some(source as _), BuildErrorKind::InvalidField { .. } | BuildErrorKind::MissingField { .. } => None, } }