From 817ca7acada1423bf1a1fc260a3a22bbfdf97204 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Thu, 5 Oct 2023 16:30:27 -0500 Subject: [PATCH 1/8] Move operation's BuildError to smithy-types --- rust-runtime/aws-smithy-http/src/endpoint.rs | 21 +- .../aws-smithy-http/src/endpoint/error.rs | 16 +- rust-runtime/aws-smithy-http/src/operation.rs | 5 +- .../aws-smithy-http/src/operation/error.rs | 187 ------------------ rust-runtime/aws-smithy-types/src/lib.rs | 1 + .../aws-smithy-types/src/operation.rs | 166 ++++++++++++++++ 6 files changed, 192 insertions(+), 204 deletions(-) delete mode 100644 rust-runtime/aws-smithy-http/src/operation/error.rs create mode 100644 rust-runtime/aws-smithy-types/src/operation.rs diff --git a/rust-runtime/aws-smithy-http/src/endpoint.rs b/rust-runtime/aws-smithy-http/src/endpoint.rs index 3a4939434e..5bf63e1a99 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; @@ -105,15 +104,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, )), } } @@ -143,11 +140,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..9ea5442463 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 { @@ -76,10 +77,12 @@ impl InvalidEndpointError { } pub(super) fn failed_to_construct_authority( + authority: String, source: impl Into>, ) -> Self { Self { kind: InvalidEndpointErrorKind::FailedToConstructAuthority { + authority, source: source.into(), }, } @@ -105,11 +108,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 +123,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 2ddbd28070..57bf3f6455 100644 --- a/rust-runtime/aws-smithy-http/src/operation.rs +++ b/rust-runtime/aws-smithy-http/src/operation.rs @@ -13,7 +13,10 @@ use aws_smithy_types::config_bag::{Storable, StoreReplace}; use std::borrow::Cow; use std::ops::{Deref, DerefMut}; -pub mod error; +/// Errors for operations +pub mod error { + pub use aws_smithy_types::operation::error::{BuildError, SerializationError}; +} /// Metadata attached to an [`Operation`] that identifies the API being called. #[derive(Clone, Debug)] diff --git a/rust-runtime/aws-smithy-http/src/operation/error.rs b/rust-runtime/aws-smithy-http/src/operation/error.rs deleted file mode 100644 index 5b2024396f..0000000000 --- a/rust-runtime/aws-smithy-http/src/operation/error.rs +++ /dev/null @@ -1,187 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -//! Errors for operations - -use aws_smithy_types::date_time::DateTimeFormatError; -use http::uri::InvalidUri; -use std::borrow::Cow; -use std::error::Error; -use std::fmt::{Display, Formatter}; - -#[derive(Debug)] -enum SerializationErrorKind { - CannotSerializeUnknownVariant { union: &'static str }, - DateTimeFormatError { cause: DateTimeFormatError }, -} - -/// An error that occurs when serialization of an operation fails. -#[derive(Debug)] -pub struct SerializationError { - kind: SerializationErrorKind, -} - -impl SerializationError { - /// An error that occurs when serialization of an operation fails for an unknown reason. - pub fn unknown_variant(union: &'static str) -> Self { - Self { - kind: SerializationErrorKind::CannotSerializeUnknownVariant { union }, - } - } -} - -impl Display for SerializationError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self.kind { - SerializationErrorKind::CannotSerializeUnknownVariant { union } => write!( - f, - "Cannot serialize `{union}::Unknown`. Unknown union variants cannot be serialized. \ - This can occur when round-tripping a response from the server that was not \ - recognized by the SDK. Consider upgrading to the latest version of the SDK.", - ), - SerializationErrorKind::DateTimeFormatError { .. } => { - write!(f, "failed to serialize timestamp") - } - } - } -} - -impl Error for SerializationError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match &self.kind { - SerializationErrorKind::CannotSerializeUnknownVariant { .. } => None, - SerializationErrorKind::DateTimeFormatError { cause } => Some(cause as _), - } - } -} - -impl From for SerializationError { - fn from(err: DateTimeFormatError) -> SerializationError { - Self { - kind: SerializationErrorKind::DateTimeFormatError { cause: err }, - } - } -} - -#[derive(Debug)] -enum BuildErrorKind { - /// A field contained an invalid value - InvalidField { - field: &'static str, - details: String, - }, - /// A field was missing - MissingField { - field: &'static str, - details: &'static str, - }, - /// 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), -} - -/// An error occurred attempting to build an `Operation` from an input -/// -/// These are almost always due to user error caused by limitations of specific fields due to -/// protocol serialization (e.g. fields that can only be a subset ASCII because they are serialized -/// as the name of an HTTP header) -#[derive(Debug)] -pub struct BuildError { - kind: BuildErrorKind, -} - -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 { - kind: BuildErrorKind::MissingField { field, details }, - } - } - - /// Construct a build error for a missing field - pub fn invalid_field(field: &'static str, details: impl Into) -> Self { - Self { - kind: BuildErrorKind::InvalidField { - field, - details: details.into(), - }, - } - } - - /// Construct a build error from another underlying error - pub fn other(source: impl Into>) -> Self { - Self { - kind: BuildErrorKind::Other(source.into()), - } - } -} - -impl From for BuildError { - fn from(err: SerializationError) -> Self { - Self { - kind: BuildErrorKind::SerializationError(err), - } - } -} - -impl From for BuildError { - fn from(err: DateTimeFormatError) -> Self { - Self::from(SerializationError::from(err)) - } -} - -impl Display for BuildError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match &self.kind { - BuildErrorKind::InvalidField { field, details } => { - write!(f, "invalid field in input: {field} (details: {details})") - } - BuildErrorKind::MissingField { field, details } => { - write!(f, "{field} was missing: {details}") - } - 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") - } - } - } -} - -impl Error for BuildError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - 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, - } - } -} diff --git a/rust-runtime/aws-smithy-types/src/lib.rs b/rust-runtime/aws-smithy-types/src/lib.rs index 76fb9010c3..2cc17827d5 100644 --- a/rust-runtime/aws-smithy-types/src/lib.rs +++ b/rust-runtime/aws-smithy-types/src/lib.rs @@ -19,6 +19,7 @@ pub mod config_bag; pub mod date_time; pub mod endpoint; pub mod error; +pub mod operation; pub mod primitive; pub mod retry; pub mod timeout; diff --git a/rust-runtime/aws-smithy-types/src/operation.rs b/rust-runtime/aws-smithy-types/src/operation.rs new file mode 100644 index 0000000000..1e45438cd2 --- /dev/null +++ b/rust-runtime/aws-smithy-types/src/operation.rs @@ -0,0 +1,166 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +//! Types for representing the interaction between a service an a client, referred to as an "operation" in smithy. +//! Clients "send" operations to services, which are composed of 1 or more HTTP requests. + +/// Errors for operations +pub mod error { + use crate::date_time::DateTimeFormatError; + use std::error::Error; + use std::fmt::{Display, Formatter}; + + #[derive(Debug)] + enum SerializationErrorKind { + CannotSerializeUnknownVariant { union: &'static str }, + DateTimeFormatError { cause: DateTimeFormatError }, + } + + /// An error that occurs when serialization of an operation fails. + #[derive(Debug)] + pub struct SerializationError { + kind: SerializationErrorKind, + } + + impl SerializationError { + /// An error that occurs when serialization of an operation fails for an unknown reason. + pub fn unknown_variant(union: &'static str) -> Self { + Self { + kind: SerializationErrorKind::CannotSerializeUnknownVariant { union }, + } + } + } + + impl Display for SerializationError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self.kind { + SerializationErrorKind::CannotSerializeUnknownVariant { union } => write!( + f, + "Cannot serialize `{union}::Unknown`. Unknown union variants cannot be serialized. \ + This can occur when round-tripping a response from the server that was not \ + recognized by the SDK. Consider upgrading to the latest version of the SDK.", + ), + SerializationErrorKind::DateTimeFormatError { .. } => { + write!(f, "failed to serialize timestamp") + } + } + } + } + + impl Error for SerializationError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match &self.kind { + SerializationErrorKind::CannotSerializeUnknownVariant { .. } => None, + SerializationErrorKind::DateTimeFormatError { cause } => Some(cause as _), + } + } + } + + impl From for SerializationError { + fn from(err: DateTimeFormatError) -> SerializationError { + Self { + kind: SerializationErrorKind::DateTimeFormatError { cause: err }, + } + } + } + + #[derive(Debug)] + enum BuildErrorKind { + /// A field contained an invalid value + InvalidField { + field: &'static str, + details: String, + }, + /// A field was missing + MissingField { + field: &'static str, + details: &'static str, + }, + /// The serializer could not serialize the input + SerializationError(SerializationError), + + /// An error occurred request construction + Other(Box), + } + + /// An error occurred attempting to build an `Operation` from an input + /// + /// These are almost always due to user error caused by limitations of specific fields due to + /// protocol serialization (e.g. fields that can only be a subset ASCII because they are serialized + /// as the name of an HTTP header) + #[derive(Debug)] + pub struct BuildError { + kind: BuildErrorKind, + } + + impl BuildError { + /// Construct a build error for a missing field + pub fn missing_field(field: &'static str, details: &'static str) -> Self { + Self { + kind: BuildErrorKind::MissingField { field, details }, + } + } + + /// Construct a build error for a missing field + pub fn invalid_field(field: &'static str, details: impl Into) -> Self { + Self { + kind: BuildErrorKind::InvalidField { + field, + details: details.into(), + }, + } + } + + /// Construct a build error from another underlying error + pub fn other(source: impl Into>) -> Self { + Self { + kind: BuildErrorKind::Other(source.into()), + } + } + } + + impl From for BuildError { + fn from(err: SerializationError) -> Self { + Self { + kind: BuildErrorKind::SerializationError(err), + } + } + } + + impl From for BuildError { + fn from(err: DateTimeFormatError) -> Self { + Self::from(SerializationError::from(err)) + } + } + + impl Display for BuildError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match &self.kind { + BuildErrorKind::InvalidField { field, details } => { + write!(f, "invalid field in input: {field} (details: {details})") + } + BuildErrorKind::MissingField { field, details } => { + write!(f, "{field} was missing: {details}") + } + BuildErrorKind::SerializationError(_) => { + write!(f, "failed to serialize input") + } + BuildErrorKind::Other(_) => { + write!(f, "error during request construction") + } + } + } + } + + impl Error for BuildError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match &self.kind { + BuildErrorKind::SerializationError(source) => Some(source as _), + BuildErrorKind::Other(source) => Some(source.as_ref()), + BuildErrorKind::InvalidField { .. } | BuildErrorKind::MissingField { .. } => None, + } + } + } +} From 1487a6061c2ba7869149b8cc70152c0bb96baa66 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Fri, 6 Oct 2023 10:20:55 -0500 Subject: [PATCH 2/8] Fix EndpointTraitBindingTest --- .../generators/EndpointTraitBindingGenerator.kt | 11 ++++------- .../smithy/generators/EndpointTraitBindingsTest.kt | 10 ++++------ rust-runtime/aws-smithy-http/src/endpoint/error.rs | 9 ++++++--- rust-runtime/aws-smithy-types/src/operation.rs | 2 +- 4 files changed, 15 insertions(+), 17 deletions(-) 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 99c0228c1b..6bfec25095 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,20 +71,18 @@ 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 = // TODO(enableNewSmithyRuntimeCleanup): Remove the allow attribute once all places need .into method """ if $field.is_empty() { - ##[allow(clippy::useless_conversion)] - 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 430ff4737e..60d50eec77 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/error.rs b/rust-runtime/aws-smithy-http/src/endpoint/error.rs index 9ea5442463..06581a933a 100644 --- a/rust-runtime/aws-smithy-http/src/endpoint/error.rs +++ b/rust-runtime/aws-smithy-http/src/endpoint/error.rs @@ -70,13 +70,15 @@ 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: String, source: impl Into>, ) -> Self { @@ -88,7 +90,8 @@ impl InvalidEndpointError { } } - 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 { diff --git a/rust-runtime/aws-smithy-types/src/operation.rs b/rust-runtime/aws-smithy-types/src/operation.rs index 1e45438cd2..de9cced09a 100644 --- a/rust-runtime/aws-smithy-types/src/operation.rs +++ b/rust-runtime/aws-smithy-types/src/operation.rs @@ -103,7 +103,7 @@ pub mod error { } } - /// 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 { From f5faa896c8585a9fb918795ff31db27e0610e221 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Fri, 6 Oct 2023 12:26:43 -0500 Subject: [PATCH 3/8] Reexport `operation::error` in its entirety --- rust-runtime/aws-smithy-http/src/operation.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rust-runtime/aws-smithy-http/src/operation.rs b/rust-runtime/aws-smithy-http/src/operation.rs index 57bf3f6455..ef5f3ddc57 100644 --- a/rust-runtime/aws-smithy-http/src/operation.rs +++ b/rust-runtime/aws-smithy-http/src/operation.rs @@ -13,10 +13,10 @@ use aws_smithy_types::config_bag::{Storable, StoreReplace}; use std::borrow::Cow; use std::ops::{Deref, DerefMut}; +//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::operation::error::{BuildError, SerializationError}; -} +pub use aws_smithy_types::operation::error; /// Metadata attached to an [`Operation`] that identifies the API being called. #[derive(Clone, Debug)] From f6bb4ba46bb0a2ee696281f9a104ecb4c21dcb71 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Mon, 9 Oct 2023 22:41:30 -0500 Subject: [PATCH 4/8] Make `BuildError` under `error::operation` in `smithy-types` This commit addresses https://github.com/awslabs/smithy-rs/pull/3032#discussion_r1349138255 --- rust-runtime/aws-smithy-http/src/operation.rs | 4 +- rust-runtime/aws-smithy-types/src/error.rs | 1 + .../aws-smithy-types/src/error/operation.rs | 162 +++++++++++++++++ rust-runtime/aws-smithy-types/src/lib.rs | 1 - .../aws-smithy-types/src/operation.rs | 166 ------------------ 5 files changed, 166 insertions(+), 168 deletions(-) create mode 100644 rust-runtime/aws-smithy-types/src/error/operation.rs delete mode 100644 rust-runtime/aws-smithy-types/src/operation.rs diff --git a/rust-runtime/aws-smithy-http/src/operation.rs b/rust-runtime/aws-smithy-http/src/operation.rs index ef5f3ddc57..246cb09195 100644 --- a/rust-runtime/aws-smithy-http/src/operation.rs +++ b/rust-runtime/aws-smithy-http/src/operation.rs @@ -16,7 +16,9 @@ use std::ops::{Deref, DerefMut}; //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 use aws_smithy_types::operation::error; +pub mod error { + pub use aws_smithy_types::error::operation::{BuildError, SerializationError}; +} /// Metadata attached to an [`Operation`] 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-types/src/error/operation.rs b/rust-runtime/aws-smithy-types/src/error/operation.rs new file mode 100644 index 0000000000..5b61f1acfe --- /dev/null +++ b/rust-runtime/aws-smithy-types/src/error/operation.rs @@ -0,0 +1,162 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +//! Errors for operations + +use crate::date_time::DateTimeFormatError; +use std::error::Error; +use std::fmt::{Display, Formatter}; + +#[derive(Debug)] +enum SerializationErrorKind { + CannotSerializeUnknownVariant { union: &'static str }, + DateTimeFormatError { cause: DateTimeFormatError }, +} + +/// An error that occurs when serialization of an operation fails. +#[derive(Debug)] +pub struct SerializationError { + kind: SerializationErrorKind, +} + +impl SerializationError { + /// An error that occurs when serialization of an operation fails for an unknown reason. + pub fn unknown_variant(union: &'static str) -> Self { + Self { + kind: SerializationErrorKind::CannotSerializeUnknownVariant { union }, + } + } +} + +impl Display for SerializationError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self.kind { + SerializationErrorKind::CannotSerializeUnknownVariant { union } => write!( + f, + "Cannot serialize `{union}::Unknown`. Unknown union variants cannot be serialized. \ + This can occur when round-tripping a response from the server that was not \ + recognized by the SDK. Consider upgrading to the latest version of the SDK.", + ), + SerializationErrorKind::DateTimeFormatError { .. } => { + write!(f, "failed to serialize timestamp") + } + } + } +} + +impl Error for SerializationError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match &self.kind { + SerializationErrorKind::CannotSerializeUnknownVariant { .. } => None, + SerializationErrorKind::DateTimeFormatError { cause } => Some(cause as _), + } + } +} + +impl From for SerializationError { + fn from(err: DateTimeFormatError) -> SerializationError { + Self { + kind: SerializationErrorKind::DateTimeFormatError { cause: err }, + } + } +} + +#[derive(Debug)] +enum BuildErrorKind { + /// A field contained an invalid value + InvalidField { + field: &'static str, + details: String, + }, + /// A field was missing + MissingField { + field: &'static str, + details: &'static str, + }, + /// The serializer could not serialize the input + SerializationError(SerializationError), + + /// An error occurred request construction + Other(Box), +} + +/// An error occurred attempting to build an `Operation` from an input +/// +/// These are almost always due to user error caused by limitations of specific fields due to +/// protocol serialization (e.g. fields that can only be a subset ASCII because they are serialized +/// as the name of an HTTP header) +#[derive(Debug)] +pub struct BuildError { + kind: BuildErrorKind, +} + +impl BuildError { + /// Construct a build error for a missing field + pub fn missing_field(field: &'static str, details: &'static str) -> Self { + Self { + kind: BuildErrorKind::MissingField { field, details }, + } + } + + /// Construct a build error for an invalid field + pub fn invalid_field(field: &'static str, details: impl Into) -> Self { + Self { + kind: BuildErrorKind::InvalidField { + field, + details: details.into(), + }, + } + } + + /// Construct a build error from another underlying error + pub fn other(source: impl Into>) -> Self { + Self { + kind: BuildErrorKind::Other(source.into()), + } + } +} + +impl From for BuildError { + fn from(err: SerializationError) -> Self { + Self { + kind: BuildErrorKind::SerializationError(err), + } + } +} + +impl From for BuildError { + fn from(err: DateTimeFormatError) -> Self { + Self::from(SerializationError::from(err)) + } +} + +impl Display for BuildError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match &self.kind { + BuildErrorKind::InvalidField { field, details } => { + write!(f, "invalid field in input: {field} (details: {details})") + } + BuildErrorKind::MissingField { field, details } => { + write!(f, "{field} was missing: {details}") + } + BuildErrorKind::SerializationError(_) => { + write!(f, "failed to serialize input") + } + BuildErrorKind::Other(_) => { + write!(f, "error during request construction") + } + } + } +} + +impl Error for BuildError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match &self.kind { + BuildErrorKind::SerializationError(source) => Some(source as _), + BuildErrorKind::Other(source) => Some(source.as_ref()), + BuildErrorKind::InvalidField { .. } | BuildErrorKind::MissingField { .. } => None, + } + } +} diff --git a/rust-runtime/aws-smithy-types/src/lib.rs b/rust-runtime/aws-smithy-types/src/lib.rs index 2cc17827d5..76fb9010c3 100644 --- a/rust-runtime/aws-smithy-types/src/lib.rs +++ b/rust-runtime/aws-smithy-types/src/lib.rs @@ -19,7 +19,6 @@ pub mod config_bag; pub mod date_time; pub mod endpoint; pub mod error; -pub mod operation; pub mod primitive; pub mod retry; pub mod timeout; diff --git a/rust-runtime/aws-smithy-types/src/operation.rs b/rust-runtime/aws-smithy-types/src/operation.rs deleted file mode 100644 index de9cced09a..0000000000 --- a/rust-runtime/aws-smithy-types/src/operation.rs +++ /dev/null @@ -1,166 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -//! Types for representing the interaction between a service an a client, referred to as an "operation" in smithy. -//! Clients "send" operations to services, which are composed of 1 or more HTTP requests. - -/// Errors for operations -pub mod error { - use crate::date_time::DateTimeFormatError; - use std::error::Error; - use std::fmt::{Display, Formatter}; - - #[derive(Debug)] - enum SerializationErrorKind { - CannotSerializeUnknownVariant { union: &'static str }, - DateTimeFormatError { cause: DateTimeFormatError }, - } - - /// An error that occurs when serialization of an operation fails. - #[derive(Debug)] - pub struct SerializationError { - kind: SerializationErrorKind, - } - - impl SerializationError { - /// An error that occurs when serialization of an operation fails for an unknown reason. - pub fn unknown_variant(union: &'static str) -> Self { - Self { - kind: SerializationErrorKind::CannotSerializeUnknownVariant { union }, - } - } - } - - impl Display for SerializationError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self.kind { - SerializationErrorKind::CannotSerializeUnknownVariant { union } => write!( - f, - "Cannot serialize `{union}::Unknown`. Unknown union variants cannot be serialized. \ - This can occur when round-tripping a response from the server that was not \ - recognized by the SDK. Consider upgrading to the latest version of the SDK.", - ), - SerializationErrorKind::DateTimeFormatError { .. } => { - write!(f, "failed to serialize timestamp") - } - } - } - } - - impl Error for SerializationError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match &self.kind { - SerializationErrorKind::CannotSerializeUnknownVariant { .. } => None, - SerializationErrorKind::DateTimeFormatError { cause } => Some(cause as _), - } - } - } - - impl From for SerializationError { - fn from(err: DateTimeFormatError) -> SerializationError { - Self { - kind: SerializationErrorKind::DateTimeFormatError { cause: err }, - } - } - } - - #[derive(Debug)] - enum BuildErrorKind { - /// A field contained an invalid value - InvalidField { - field: &'static str, - details: String, - }, - /// A field was missing - MissingField { - field: &'static str, - details: &'static str, - }, - /// The serializer could not serialize the input - SerializationError(SerializationError), - - /// An error occurred request construction - Other(Box), - } - - /// An error occurred attempting to build an `Operation` from an input - /// - /// These are almost always due to user error caused by limitations of specific fields due to - /// protocol serialization (e.g. fields that can only be a subset ASCII because they are serialized - /// as the name of an HTTP header) - #[derive(Debug)] - pub struct BuildError { - kind: BuildErrorKind, - } - - impl BuildError { - /// Construct a build error for a missing field - pub fn missing_field(field: &'static str, details: &'static str) -> Self { - Self { - kind: BuildErrorKind::MissingField { field, details }, - } - } - - /// Construct a build error for an invalid field - pub fn invalid_field(field: &'static str, details: impl Into) -> Self { - Self { - kind: BuildErrorKind::InvalidField { - field, - details: details.into(), - }, - } - } - - /// Construct a build error from another underlying error - pub fn other(source: impl Into>) -> Self { - Self { - kind: BuildErrorKind::Other(source.into()), - } - } - } - - impl From for BuildError { - fn from(err: SerializationError) -> Self { - Self { - kind: BuildErrorKind::SerializationError(err), - } - } - } - - impl From for BuildError { - fn from(err: DateTimeFormatError) -> Self { - Self::from(SerializationError::from(err)) - } - } - - impl Display for BuildError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match &self.kind { - BuildErrorKind::InvalidField { field, details } => { - write!(f, "invalid field in input: {field} (details: {details})") - } - BuildErrorKind::MissingField { field, details } => { - write!(f, "{field} was missing: {details}") - } - BuildErrorKind::SerializationError(_) => { - write!(f, "failed to serialize input") - } - BuildErrorKind::Other(_) => { - write!(f, "error during request construction") - } - } - } - } - - impl Error for BuildError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match &self.kind { - BuildErrorKind::SerializationError(source) => Some(source as _), - BuildErrorKind::Other(source) => Some(source.as_ref()), - BuildErrorKind::InvalidField { .. } | BuildErrorKind::MissingField { .. } => None, - } - } - } -} From eb1b2f8708a34e0be2926f8c9af6135587c1c718 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Tue, 10 Oct 2023 14:26:15 -0500 Subject: [PATCH 5/8] Update CHANGELOG.next.toml --- CHANGELOG.next.toml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index d9ed616628..64b0878322 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -267,3 +267,11 @@ message = "STS and SSO-based credential providers will now respect both `use_fip references = ["aws-sdk-rust#882", "smithy-rs#3007"] meta = { "breaking" = true, "tada" = true, "bug" = true } author = "Velfi" + +[[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" From b26a495d14b35dab5d70dbab5957cc9ab5859b7c Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Tue, 10 Oct 2023 14:27:22 -0500 Subject: [PATCH 6/8] Remove TODO that has been done --- .../client/smithy/generators/EndpointTraitBindingGenerator.kt | 1 - 1 file changed, 1 deletion(-) 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 6bfec25095..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 @@ -73,7 +73,6 @@ class EndpointTraitBindings( if (generateValidation) { val errorString = "$field was unset or empty but must be set as part of the endpoint prefix" val contents = - // TODO(enableNewSmithyRuntimeCleanup): Remove the allow attribute once all places need .into method """ if $field.is_empty() { return Err(#{InvalidEndpointError}::failed_to_construct_uri("$errorString").into()); From 05bdc602309d1d9dbcde1129a37ae87a27bceb80 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Thu, 12 Oct 2023 16:06:20 -0500 Subject: [PATCH 7/8] Update CHANGELOG.next.toml Co-authored-by: John DiSanti --- CHANGELOG.next.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index f2c733adf8..9529f472d8 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -306,7 +306,7 @@ author = "rcoh" [[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). +[`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" } From 18e99d6cc9e860aa9a2cdf1957d5810e04cc2dae Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Thu, 12 Oct 2023 16:17:27 -0500 Subject: [PATCH 8/8] Take `Into` rather than `String` in a pub function This commit addresses https://github.com/awslabs/smithy-rs/pull/3032#discussion_r1357080897 --- rust-runtime/aws-smithy-http/src/endpoint/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust-runtime/aws-smithy-http/src/endpoint/error.rs b/rust-runtime/aws-smithy-http/src/endpoint/error.rs index 06581a933a..eeb703523c 100644 --- a/rust-runtime/aws-smithy-http/src/endpoint/error.rs +++ b/rust-runtime/aws-smithy-http/src/endpoint/error.rs @@ -79,12 +79,12 @@ impl InvalidEndpointError { /// Construct a build error for an invalid authority pub fn failed_to_construct_authority( - authority: String, + authority: impl Into, source: impl Into>, ) -> Self { Self { kind: InvalidEndpointErrorKind::FailedToConstructAuthority { - authority, + authority: authority.into(), source: source.into(), }, }