From cd36a8d8f7acd1d801ef097251b5f57571bdf16c Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Wed, 7 Dec 2022 16:18:56 -0800 Subject: [PATCH 1/5] Add RFC for improving client error ergonomics --- design/src/SUMMARY.md | 2 + design/src/rfcs/overview.md | 1 + ...rfc0026_improve_client_error_ergonomics.md | 121 ++++++++++++++++++ 3 files changed, 124 insertions(+) create mode 100644 design/src/rfcs/rfc0026_improve_client_error_ergonomics.md diff --git a/design/src/SUMMARY.md b/design/src/SUMMARY.md index fb4968938d..9ae7855a93 100644 --- a/design/src/SUMMARY.md +++ b/design/src/SUMMARY.md @@ -47,6 +47,8 @@ - [RFC-0022: Error Context and Compatibility](./rfcs/rfc0022_error_context_and_compatibility.md) - [RFC-0023: Evolving the new service builder API](./rfcs/rfc0023_refine_builder.md) - [RFC-0024: RequestID](./rfcs/rfc0024_request_id.md) + - [RFC-0025: Constraint Traits](./rfcs/rfc0025_constraint_traits.md) + - [RFC-0026: Improve client error ergonomics](./rfcs/rfc0026_improve_client_error_ergonomics.md) - [Contributing](./contributing/overview.md) - [Writing and debugging a low-level feature that relies on HTTP](./contributing/writing_and_debugging_a_low-level_feature_that_relies_on_HTTP.md) diff --git a/design/src/rfcs/overview.md b/design/src/rfcs/overview.md index 7cb76e7401..1c62e32fb3 100644 --- a/design/src/rfcs/overview.md +++ b/design/src/rfcs/overview.md @@ -35,3 +35,4 @@ - [RFC-0023: Evolving the new service builder API](./rfc0023_refine_builder.md) - [RFC-0024: RequestID](./rfc0024_request_id.md) - [RFC-0025: Constraint traits](./rfc0025_constraint_traits.md) +- [RFC-0026: Improve client error ergonomics](./rfc0026_improve_client_error_ergonomics.md) diff --git a/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md b/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md new file mode 100644 index 0000000000..f392b7bed7 --- /dev/null +++ b/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md @@ -0,0 +1,121 @@ +RFC: Improve Client Error Ergonomics +==================================== + +> Status: RFC +> +> Applies to: clients + +This RFC proposes some changes to code generated errors to make them easier to use for customers. +With the SDK and code generated clients, customers have two primary use-cases that should be made +easy without compromising the compatibility rules established in [RFC-0022]: + +1. Checking the error type +2. Retrieving information specific to that error type + +Case Study: Handling an error in S3 +----------------------------------- + +The following is an example of handling errors with S3 with the latest generated (and unreleased) +SDK as of 2022-12-07: + +```rust +let result = client + .get_object() + .bucket(BUCKET_NAME) + .key("some-key") + .send() + .await; +match result { + Ok(_output) => { /* Do something with the output */ } + Err(err) => match err.into_service_error() { + GetObjectError { + kind: GetObjectErrorKind::InvalidObjectState(value), + .. + } => { + println!("invalid object state: {:?}", value); + } + GetObjectError { + kind: GetObjectErrorKind::NoSuchKey(_), + .. + } => { + println!("object didn't exist"); + } + err @ GetObjectError { .. } if err.code() == Some("SomeUnmodeledError") => {} + err @ _ => return Err(err.into()), + }, +} +``` + +The refactor that implemented [RFC-0022] added the `into_service_error()` method on `SdkError` that +infallibly converts the `SdkError` into the generic error type for the `SdkError::ServiceError` variant. +This improvement let customers discard the transient failures to immediately handle modeled errors +returned by the service. + +Despite this, the code is still quite verbose. + +Proposal: Combine `Error` and `ErrorKind` +----------------------------------------- + +At time of writing, each operation has both an `Error` and `ErrorKind` type generated. +The `Error` type holds information that is common across all operation errors: message, +error code, "extra" key/value pairs, and the request ID. + +The `ErrorKind` is always nested inside the `Error`, which results in the verbose +nested matching shown in the case study above. + +To make error handling more ergonomic, the code generated `Error` and `ErrorKind` types +should be combined. Hypothetically, this would allow for the case study above to look as follows: + +```rust +let result = client + .get_object() + .bucket(BUCKET_NAME) + .key("some-key") + .send() + .await; +match result { + Ok(_output) => { /* Do something with the output */ } + Err(err) => match err.into_service_error() { + GetObjectError::InvalidObjectState(value) => { + println!("invalid object state: {:?}", value); + } + err if err.is_no_such_key() => { + println!("object didn't exist"); + } + err if err.code() == Some("SomeUnmodeledError") => {} + err @ _ => return Err(err.into()), + }, +} +``` + +If a customer only cares about checking one specific error type, they can also do: + +```rust +match result { + Ok(_output) => { /* Do something with the output */ } + Err(err) => { + let err = err.into_service_error(); + if err.is_no_such_key() { + println!("object didn't exist"); + } else { + return Err(err); + } + } +} +``` + +The downside of this is that combining the error types requires adding the general error +metadata to each generated error struct so that it's accessible by the enum error type. +However, this aligns with our tenet of making things easier for customers even if it +makes it harder for ourselves. + +Changes Checklist +----------------- + +- [ ] Merge the `${operation}Error`/`${operation}ErrorKind` code generators to only generate an `${operation}Error` enum: + - Preserve the `is_${variant}` methods + - Preserve error metadata by adding it to each individual variant's context struct +- [ ] Write upgrade guidance +- [ ] Fix examples + +[RFC-0022]: ./rfc0022_error_context_and_compatibility.md From 312b82f64acdb7b64c85e9d92f4de39e890966c6 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 8 Dec 2022 14:49:01 -0800 Subject: [PATCH 2/5] Apply suggestion Co-authored-by: Zelda Hessler --- design/src/rfcs/rfc0026_improve_client_error_ergonomics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md b/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md index f392b7bed7..31f6e1bc38 100644 --- a/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md +++ b/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md @@ -47,7 +47,7 @@ match result { ``` The refactor that implemented [RFC-0022] added the `into_service_error()` method on `SdkError` that -infallibly converts the `SdkError` into the generic error type for the `SdkError::ServiceError` variant. +infallibly converts the `SdkError` into the concrete error type held by the `SdkError::ServiceError` variant. This improvement let customers discard the transient failures to immediately handle modeled errors returned by the service. From 7931c96e3968626fa0ae0f632465bb9d2540a843 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 8 Dec 2022 14:49:30 -0800 Subject: [PATCH 3/5] Apply suggestion Co-authored-by: Zelda Hessler --- design/src/rfcs/rfc0026_improve_client_error_ergonomics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md b/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md index 31f6e1bc38..8a3144c084 100644 --- a/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md +++ b/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md @@ -48,7 +48,7 @@ match result { The refactor that implemented [RFC-0022] added the `into_service_error()` method on `SdkError` that infallibly converts the `SdkError` into the concrete error type held by the `SdkError::ServiceError` variant. -This improvement let customers discard the transient failures to immediately handle modeled errors +This improvement lets customers discard transient failures and immediately handle modeled errors returned by the service. Despite this, the code is still quite verbose. From 8073bb1f6922c95e5bf3cdc8a82f9f67f683d28d Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 8 Dec 2022 14:50:13 -0800 Subject: [PATCH 4/5] Apply suggestion Co-authored-by: Zelda Hessler --- ...rfc0026_improve_client_error_ergonomics.md | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md b/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md index 8a3144c084..9105682b55 100644 --- a/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md +++ b/design/src/rfcs/rfc0026_improve_client_error_ergonomics.md @@ -25,25 +25,17 @@ let result = client .key("some-key") .send() .await; -match result { - Ok(_output) => { /* Do something with the output */ } - Err(err) => match err.into_service_error() { - GetObjectError { - kind: GetObjectErrorKind::InvalidObjectState(value), - .. - } => { - println!("invalid object state: {:?}", value); - } - GetObjectError { - kind: GetObjectErrorKind::NoSuchKey(_), - .. - } => { - println!("object didn't exist"); - } - err @ GetObjectError { .. } if err.code() == Some("SomeUnmodeledError") => {} - err @ _ => return Err(err.into()), - }, -} + match result { + Ok(_output) => { /* Do something with the output */ } + Err(err) => match err.into_service_error() { + GetObjectError { kind, .. } => match kind { + GetObjectErrorKind::InvalidObjectState(value) => println!("invalid object state: {:?}", value), + GetObjectErrorKind::NoSuchKey(_) => println!("object didn't exist"), + } + err @ GetObjectError { .. } if err.code() == Some("SomeUnmodeledError") => {} + err @ _ => return Err(err.into()), + }, + } ``` The refactor that implemented [RFC-0022] added the `into_service_error()` method on `SdkError` that From 99e5235ab8bebca9e7d8f5af15cc0b6ff365a31c Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 5 Apr 2024 15:30:43 -0700 Subject: [PATCH 5/5] Fix book CI checks --- design/src/rfcs/rfc0041_improve_client_error_ergonomics.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/design/src/rfcs/rfc0041_improve_client_error_ergonomics.md b/design/src/rfcs/rfc0041_improve_client_error_ergonomics.md index a5a7a35336..94aefafd21 100644 --- a/design/src/rfcs/rfc0041_improve_client_error_ergonomics.md +++ b/design/src/rfcs/rfc0041_improve_client_error_ergonomics.md @@ -18,7 +18,7 @@ Case Study: Handling an error in S3 The following is an example of handling errors with S3 with the latest generated (and unreleased) SDK as of 2022-12-07: -```rust +```rust,ignore let result = client .get_object() .bucket(BUCKET_NAME) @@ -58,7 +58,7 @@ nested matching shown in the case study above. To make error handling more ergonomic, the code generated `Error` and `ErrorKind` types should be combined. Hypothetically, this would allow for the case study above to look as follows: -```rust +```rust,ignore let result = client .get_object() .bucket(BUCKET_NAME) @@ -82,7 +82,7 @@ match result { If a customer only cares about checking one specific error type, they can also do: -```rust +```rust,ignore match result { Ok(_output) => { /* Do something with the output */ } Err(err) => {