Skip to content

Commit

Permalink
Revamp errors in aws-smithy-json
Browse files Browse the repository at this point in the history
  • Loading branch information
jdisanti committed Oct 21, 2022
1 parent 7a043e5 commit 0194b67
Show file tree
Hide file tree
Showing 12 changed files with 160 additions and 129 deletions.
4 changes: 2 additions & 2 deletions aws/rust-runtime/aws-config/src/json_credentials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ impl From<EscapeError> for InvalidJsonCredentials {
}
}

impl From<aws_smithy_json::deserialize::Error> for InvalidJsonCredentials {
fn from(err: aws_smithy_json::deserialize::Error) -> Self {
impl From<aws_smithy_json::deserialize::error::DeserializeError> for InvalidJsonCredentials {
fn from(err: aws_smithy_json::deserialize::error::DeserializeError) -> Self {
InvalidJsonCredentials::JsonError(err.into())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ open class AwsJson(
"Bytes" to RuntimeType.Bytes,
"Error" to RuntimeType.GenericError(runtimeConfig),
"HeaderMap" to RuntimeType.http.member("HeaderMap"),
"JsonError" to CargoDependency.smithyJson(runtimeConfig).asType().member("deserialize::Error"),
"JsonError" to CargoDependency.smithyJson(runtimeConfig).asType().member("deserialize::error::DeserializeError"),
"Response" to RuntimeType.http.member("Response"),
"json_errors" to RuntimeType.jsonErrors(runtimeConfig),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ open class RestJson(val codegenContext: CodegenContext) : Protocol {
"Bytes" to RuntimeType.Bytes,
"Error" to RuntimeType.GenericError(runtimeConfig),
"HeaderMap" to RuntimeType.http.member("HeaderMap"),
"JsonError" to CargoDependency.smithyJson(runtimeConfig).asType().member("deserialize::Error"),
"JsonError" to CargoDependency.smithyJson(runtimeConfig).asType().member("deserialize::error::DeserializeError"),
"Response" to RuntimeType.http.member("Response"),
"json_errors" to RuntimeType.jsonErrors(runtimeConfig),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import software.amazon.smithy.model.traits.TimestampFormatTrait
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
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.RustType
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.asType
Expand Down Expand Up @@ -68,8 +69,7 @@ class JsonParserGenerator(
private val jsonDeserModule = RustModule.private("json_deser")
private val typeConversionGenerator = TypeConversionGenerator(model, symbolProvider, runtimeConfig)
private val codegenScope = arrayOf(
"Error" to smithyJson.member("deserialize::Error"),
"ErrorReason" to smithyJson.member("deserialize::ErrorReason"),
"Error" to smithyJson.member("deserialize::error::DeserializeError"),
"expect_blob_or_null" to smithyJson.member("deserialize::token::expect_blob_or_null"),
"expect_bool_or_null" to smithyJson.member("deserialize::token::expect_bool_or_null"),
"expect_document" to smithyJson.member("deserialize::token::expect_document"),
Expand Down Expand Up @@ -370,7 +370,7 @@ class JsonParserGenerator(
*codegenScope,
) {
startObjectOrNull {
rust("let mut map = #T::new();", software.amazon.smithy.rust.codegen.core.rustlang.RustType.HashMap.RuntimeType)
rust("let mut map = #T::new();", RustType.HashMap.RuntimeType)
objectKeyLoop(hasMembers = true) {
withBlock("let key =", "?;") {
deserializeStringInner(keyTarget, "key")
Expand Down Expand Up @@ -415,9 +415,7 @@ class JsonParserGenerator(
withBlock("Ok(Some(builder.build()", "))") {
if (StructureGenerator.hasFallibleBuilder(shape, symbolProvider)) {
rustTemplate(
""".map_err(|err| #{Error}::new(
#{ErrorReason}::Custom(format!("{}", err).into()), None)
)?""",
".map_err(|err| #{Error}::custom(format!(\"{err}\")))?",
*codegenScope,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1213,12 +1213,12 @@ private class ServerHttpBoundProtocolTraitImplGenerator(
if (model.expectShape(binding.member.target) is StringShape) {
return ServerRuntimeType.RequestRejection(runtimeConfig)
}
when (codegenContext.protocol) {
return when (codegenContext.protocol) {
RestJson1Trait.ID, AwsJson1_0Trait.ID, AwsJson1_1Trait.ID -> {
return CargoDependency.smithyJson(runtimeConfig).asType().member("deserialize").member("Error")
CargoDependency.smithyJson(runtimeConfig).asType().member("deserialize::error::DeserializeError")
}
RestXmlTrait.ID -> {
return CargoDependency.smithyXml(runtimeConfig).asType().member("decode").member("XmlError")
CargoDependency.smithyXml(runtimeConfig).asType().member("decode").member("XmlError")
}
else -> {
TODO("Protocol ${codegenContext.protocol} not supported yet")
Expand Down
2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-http-server/src/rejection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ impl From<MissingContentTypeReason> for RequestRejection {
// type. Generated functions that use [crate::rejection::RequestRejection] can thus use `?` to
// bubble up instead of having to sprinkle things like [`Result::map_err`] everywhere.

convert_to_request_rejection!(aws_smithy_json::deserialize::Error, JsonDeserialize);
convert_to_request_rejection!(aws_smithy_json::deserialize::error::DeserializeError, JsonDeserialize);
convert_to_request_rejection!(aws_smithy_xml::decode::XmlError, XmlDeserialize);
convert_to_request_rejection!(aws_smithy_http::operation::error::BuildError, Build);
convert_to_request_rejection!(aws_smithy_http::header::ParseError, HeaderParse);
Expand Down
28 changes: 14 additions & 14 deletions rust-runtime/aws-smithy-json/src/deserialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@
* SPDX-License-Identifier: Apache-2.0
*/

use crate::deserialize::error::{DeserializeError as Error, DeserializeErrorKind as ErrorKind};
use aws_smithy_types::Number;
use ErrorKind::*;

mod error;
pub mod error;
pub mod token;

pub use error::{Error, ErrorReason};
pub use token::{EscapeError, EscapedStr, Offset, Token};

use ErrorReason::*;

/// JSON token parser as a Rust iterator
///
/// This parser will parse and yield exactly one [`Token`] per iterator `next()` call.
Expand Down Expand Up @@ -96,13 +95,13 @@ impl<'a> JsonTokenIterator<'a> {
}

/// Creates an error at the given `offset` in the stream.
fn error_at(&self, offset: usize, reason: ErrorReason) -> Error {
Error::new(reason, Some(offset))
fn error_at(&self, offset: usize, kind: ErrorKind) -> Error {
Error::new(kind, Some(offset))
}

/// Creates an error at the current offset in the stream.
fn error(&self, reason: ErrorReason) -> Error {
self.error_at(self.index, reason)
fn error(&self, kind: ErrorKind) -> Error {
self.error_at(self.index, kind)
}

/// Advances until it hits a non-whitespace character or the end of the slice.
Expand Down Expand Up @@ -504,11 +503,12 @@ fn must_not_be_finite(f: f64) -> Result<f64, ()> {

#[cfg(test)]
mod tests {
use crate::deserialize::error::{DeserializeError as Error, DeserializeErrorKind as ErrorKind};
use crate::deserialize::token::test::{
end_array, end_object, object_key, start_array, start_object, value_bool, value_null,
value_number, value_string,
};
use crate::deserialize::{json_token_iter, Error, ErrorReason, EscapedStr, Token};
use crate::deserialize::{json_token_iter, EscapedStr, Token};
use aws_smithy_types::Number;
use proptest::prelude::*;

Expand Down Expand Up @@ -655,7 +655,7 @@ mod tests {
let tokens: Vec<Result<Token, Error>> = json_token_iter(input).collect();
assert_eq!(
vec![Err(Error::new(
ErrorReason::UnexpectedToken(token, msg),
ErrorKind::UnexpectedToken(token, msg),
Some(offset)
))],
tokens,
Expand All @@ -667,7 +667,7 @@ mod tests {
let invalid_number = |input, offset| {
let tokens: Vec<Result<Token, Error>> = json_token_iter(input).collect();
assert_eq!(
vec![Err(Error::new(ErrorReason::InvalidNumber, Some(offset)))],
vec![Err(Error::new(ErrorKind::InvalidNumber, Some(offset)))],
tokens,
"input: \"{}\"",
std::str::from_utf8(input).unwrap(),
Expand Down Expand Up @@ -699,7 +699,7 @@ mod tests {
assert_eq!(start_array(1), iter.next());
assert_eq!(value_null(2), iter.next());
assert_eq!(
Some(Err(Error::new(ErrorReason::UnexpectedEos, Some(7)))),
Some(Err(Error::new(ErrorKind::UnexpectedEos, Some(7)))),
iter.next()
);
}
Expand Down Expand Up @@ -761,7 +761,7 @@ mod tests {
assert_eq!(value_string(11, "trailing"), iter.next());
assert_eq!(
Some(Err(Error::new(
ErrorReason::UnexpectedToken('}', "'\"'"),
ErrorKind::UnexpectedToken('}', "'\"'"),
Some(23),
))),
iter.next()
Expand All @@ -775,7 +775,7 @@ mod tests {
assert_eq!(start_object(1), iter.next());
assert_eq!(object_key(2, "test"), iter.next());
assert_eq!(
Some(Err(Error::new(ErrorReason::UnexpectedEos, Some(9),))),
Some(Err(Error::new(ErrorKind::UnexpectedEos, Some(9),))),
iter.next()
);
assert_eq!(None, iter.next());
Expand Down
80 changes: 47 additions & 33 deletions rust-runtime/aws-smithy-json/src/deserialize/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ use std::borrow::Cow;
use std::fmt;
use std::str::Utf8Error;

#[derive(Debug, PartialEq, Eq)]
pub enum ErrorReason {
#[derive(Debug)]
#[cfg_attr(test, derive(PartialEq, Eq))]
pub(in crate::deserialize) enum DeserializeErrorKind {
Custom(Cow<'static, str>),
ExpectedLiteral(String),
InvalidEscape(char),
Expand All @@ -20,73 +21,86 @@ pub enum ErrorReason {
UnexpectedEos,
UnexpectedToken(char, &'static str),
}
use ErrorReason::*;

#[derive(Debug, PartialEq, Eq)]
pub struct Error {
reason: ErrorReason,
#[derive(Debug)]
#[cfg_attr(test, derive(PartialEq, Eq))]
pub struct DeserializeError {
kind: DeserializeErrorKind,
offset: Option<usize>,
}

impl Error {
pub fn new(reason: ErrorReason, offset: Option<usize>) -> Self {
Error { reason, offset }
impl DeserializeError {
pub(in crate::deserialize) fn new(kind: DeserializeErrorKind, offset: Option<usize>) -> Self {
Self { kind, offset }
}

/// Returns a custom error without an offset.
pub fn custom(message: impl Into<Cow<'static, str>>) -> Error {
Error::new(ErrorReason::Custom(message.into()), None)
pub fn custom(message: impl Into<Cow<'static, str>>) -> Self {
Self::new(DeserializeErrorKind::Custom(message.into()), None)
}
}

impl std::error::Error for Error {}
impl std::error::Error for DeserializeError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use DeserializeErrorKind::*;
match &self.kind {
UnescapeFailed(source) => Some(source),
Custom(_)
| ExpectedLiteral(_)
| InvalidEscape(_)
| InvalidNumber
| InvalidUtf8
| UnexpectedControlCharacter(_)
| UnexpectedToken(..)
| UnexpectedEos => None,
}
}
}

impl fmt::Display for Error {
impl fmt::Display for DeserializeError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use DeserializeErrorKind::*;
if let Some(offset) = self.offset {
write!(f, "Error at offset {}: ", offset)?;
}
match &self.reason {
Custom(msg) => write!(f, "failed to parse JSON: {}", msg),
ExpectedLiteral(literal) => write!(f, "expected literal: {}", literal),
InvalidEscape(escape) => write!(f, "invalid JSON escape: \\{}", escape),
match &self.kind {
Custom(msg) => write!(f, "failed to parse JSON: {msg}"),
ExpectedLiteral(literal) => write!(f, "expected literal: {literal}"),
InvalidEscape(escape) => write!(f, "invalid JSON escape: \\{escape}"),
InvalidNumber => write!(f, "invalid number"),
InvalidUtf8 => write!(f, "invalid UTF-8 codepoint in JSON stream"),
UnescapeFailed(err) => write!(f, "failed to unescape JSON string: {}", err),
UnescapeFailed(_) => write!(f, "failed to unescape JSON string"),
UnexpectedControlCharacter(value) => write!(
f,
"encountered unescaped control character in string: 0x{:X}",
value
),
UnexpectedToken(token, expected) => write!(
f,
"unexpected token '{}'. Expected one of {}",
token, expected
"encountered unescaped control character in string: 0x{value:X}"
),
UnexpectedToken(token, expected) => {
write!(f, "unexpected token '{token}'. Expected one of {expected}",)
}
UnexpectedEos => write!(f, "unexpected end of stream"),
}
}
}

impl From<Utf8Error> for ErrorReason {
impl From<Utf8Error> for DeserializeErrorKind {
fn from(_: Utf8Error) -> Self {
InvalidUtf8
DeserializeErrorKind::InvalidUtf8
}
}

impl From<EscapeError> for Error {
impl From<EscapeError> for DeserializeError {
fn from(err: EscapeError) -> Self {
Error {
reason: ErrorReason::UnescapeFailed(err),
Self {
kind: DeserializeErrorKind::UnescapeFailed(err),
offset: None,
}
}
}

impl From<aws_smithy_types::TryFromNumberError> for Error {
impl From<aws_smithy_types::TryFromNumberError> for DeserializeError {
fn from(_: aws_smithy_types::TryFromNumberError) -> Self {
Error {
reason: ErrorReason::InvalidNumber,
Self {
kind: DeserializeErrorKind::InvalidNumber,
offset: None,
}
}
Expand Down
Loading

0 comments on commit 0194b67

Please sign in to comment.