Skip to content

Commit

Permalink
Move aws_smithy_http::operation::error::BuildError into `aws-smithy…
Browse files Browse the repository at this point in the history
…-types` (#3032)

## Motivation and Context
Takes care of the first part of
#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 <[email protected]>
  • Loading branch information
ysaito1001 and jdisanti authored Oct 12, 2023
1 parent 98dadc0 commit 8439f2a
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 59 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
Expand Down
21 changes: 10 additions & 11 deletions rust-runtime/aws-smithy-http/src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -104,15 +103,13 @@ impl<T> ResolveEndpoint<T> for Endpoint {
pub struct EndpointPrefix(String);
impl EndpointPrefix {
/// Create a new endpoint prefix from an `impl Into<String>`. If the prefix argument is invalid,
/// a [`BuildError`] will be returned.
pub fn new(prefix: impl Into<String>) -> StdResult<Self, BuildError> {
/// a [`InvalidEndpointError`] will be returned.
pub fn new(prefix: impl Into<String>) -> StdResult<Self, InvalidEndpointError> {
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,
)),
}
}
Expand Down Expand Up @@ -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()
Expand Down
25 changes: 17 additions & 8 deletions rust-runtime/aws-smithy-http/src/endpoint/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ impl Error for ResolveEndpointError {
pub(super) enum InvalidEndpointErrorKind {
EndpointMustHaveScheme,
FailedToConstructAuthority {
authority: String,
source: Box<dyn Error + Send + Sync + 'static>,
},
FailedToConstructUri {
Expand All @@ -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<String>,
source: impl Into<Box<dyn Error + Send + Sync + 'static>>,
) -> 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<Box<dyn Error + Send + Sync + 'static>>,
) -> Self {
Self {
Expand All @@ -105,11 +111,11 @@ impl From<InvalidEndpointErrorKind> 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"),
}
Expand All @@ -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,
}
}
Expand Down
7 changes: 6 additions & 1 deletion rust-runtime/aws-smithy-http/src/operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
1 change: 1 addition & 0 deletions rust-runtime/aws-smithy-types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::fmt;

pub mod display;
pub mod metadata;
pub mod operation;
mod unhandled;

pub use metadata::ErrorMetadata;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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<dyn Error + Send + Sync + 'static>),
}
Expand All @@ -104,24 +93,14 @@ 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 {
kind: BuildErrorKind::MissingField { field, details },
}
}

/// 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<String>) -> Self {
Self {
kind: BuildErrorKind::InvalidField {
Expand Down Expand Up @@ -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")
}
Expand All @@ -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,
}
}
Expand Down

0 comments on commit 8439f2a

Please sign in to comment.