Revise unhandled error variant according to RFC-39 #3191

jdisanti · 2023-11-14T01:31:27Z

This PR implements RFC-39 with a couple slight deviations:

No introspect method is added since Error already implements ProvideErrorMetadata.
The same opaqueness and deprecation pointer is applied to the enum unknown variant for consistency.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2023-11-14T01:50:48Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-11-14T19:17:48Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

aws/sdk/integration-tests/s3/tests/request_id.rs

...n/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt

...oftware/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt

.../software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt

github-actions · 2023-11-14T22:06:22Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

…ror-refactor

github-actions · 2023-11-14T22:37:17Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

rcoh

approved with a few comments—I think we should also workshop that deprecation message but we can do that later

rcoh · 2023-11-14T22:28:13Z

aws/rust-runtime/aws-inlineable/src/s3_request_id.rs

@@ -36,6 +37,7 @@ impl RequestIdExt for ErrorMetadata {
    }
 }

+#[allow(deprecated)]


I guess this is where I'd consider using Introspect but this is fine, I don't think it's a huge deal if we do it this way.

rcoh · 2023-11-15T01:03:49Z

examples/pokemon-service-client-usage/examples/handling-errors.rs

-                // An unexpected error occurred (e.g., invalid JSON returned by the service or an unknown error code).
-                GetStorageError::Unhandled(uh) => {
-                    tracing::error!(error = %uh, "An unhandled error has occurred.")
-                }


if this is an example it should probably use DisplayErrorContext

Good call. Fixed.

rcoh · 2023-11-15T01:04:24Z

rust-runtime/aws-smithy-types/src/error/unhandled.rs

@@ -58,6 +61,7 @@ impl Builder {
 /// [`DisplayErrorContext`](crate::error::display::DisplayErrorContext), use another
 /// error reporter library that visits the error's cause/source chain, or call
 /// [`Error::source`](std::error::Error::source) for more details about the underlying cause.
+#[deprecated(note = "This type is no longer used by errors.")]


so is this type still used at all?

It's no longer used. I left it in to be removed in the next release in case anyone was referencing it.

rcoh · 2023-11-15T01:05:15Z

CHANGELOG.next.toml

+```rust
+match service_error.err() {
+    GetStorageError::StorageAccessNotAuthorized(_) => { /* ... */ }
+    GetStorageError::Unhandled(unhandled) if unhandled.code() == "SomeUnmodeledErrorCode" {


code is optional; == Some(...), same for below

Good catch!

jdisanti · 2023-11-15T01:56:30Z

While fixing examples for this, I realized we need a way to parse an enum while disallowing unknown variants. For example, this is what examples are currently doing:

    // parse resource type from user input
    let parsed = ResourceType::from(resource_type.as_str());
    if matches!(parsed, ResourceType::Unknown(_)) {
        panic!(
            "unknown resource type: `{}`. Valid resource types: {:#?}",
            &resource_type,
            ResourceType::values()
        )
    }

I'm going to add a try_parse method to the enums for this use-case that will error if the variant doesn't exist, which will make that look as follows:

    // parse resource type from user input
    let parsed = match ResourceType::try_parse(resource_type.as_str()) {
        Ok(parsed) => parsed,
        Err(_) => panic!(
            "unknown resource type: `{}`. Valid resource types: {:#?}",
            &resource_type,
            ResourceType::values()
        ),
    };

…ror-refactor

jdisanti · 2023-11-15T02:20:00Z

Also found that StatusCode was inaccessible while fixing examples. Added a fix here in 34b3dea.

github-actions · 2023-11-15T02:38:00Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-11-15T03:28:44Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

rcoh · 2023-11-15T15:47:39Z

LGTM. Suggested deprecation message:

#[deprecated(
    note = "Matching `Unhandled` directly is not forwards compatible. Instead, match using a 
    variable wildcard pattern and check `.code()`: 
    &nbsp;&nbsp;&nbsp;`err if err.code() == Some(\"SpecificExceptionCode\") => { /* handle the error */ }`.
    See [`ProvideErrorMetadata`](#impl-Unhandled-for-<NameOfStruct>) for more information."
)]

It renders like this (and the link works!)

…ror-refactor

jdisanti · 2023-11-15T17:34:33Z

Made a slight tweak to your suggested message:

See [`ProvideErrorMetadata`]($link) for what information is available for the error.

Instead of "for more information" since the trait doesn't give more information about this deprecation, so that could have been confusing.

github-actions · 2023-11-15T17:52:31Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

jdisanti added breaking-change This will require a breaking change needs-sdk-review labels Nov 14, 2023

Base automatically changed from jdisanti-error-meta-service-error to main November 14, 2023 19:40

rcoh reviewed Nov 14, 2023

View reviewed changes

jdisanti added 5 commits November 14, 2023 13:47

Revise unhandled error variant according to RFC-39

1eeb930

Add the deprecation pointer to the enum unknown variant

2d276b0

Fix several CI failures

7c40699

Remove is_unhandled()

5b1952d

Incorporate feedback

b308867

jdisanti force-pushed the jdisanti-unhandled-error-refactor branch from a71467e to b308867 Compare November 14, 2023 21:47

jdisanti marked this pull request as ready for review November 14, 2023 22:10

jdisanti requested review from a team as code owners November 14, 2023 22:10

jdisanti added 3 commits November 14, 2023 14:16

Update changelog

fcc8133

Fix name collision

3391a9b

Merge remote-tracking branch 'origin/main' into jdisanti-unhandled-er…

25905f6

…ror-refactor

ysaito1001 approved these changes Nov 15, 2023

View reviewed changes

rcoh approved these changes Nov 15, 2023

View reviewed changes

jdisanti added 5 commits November 14, 2023 18:04

Add try_parse method to client enums

3fdb09b

Merge remote-tracking branch 'origin/main' into jdisanti-unhandled-er…

98fe102

…ror-refactor

Use DisplayErrorContext in example

bf24f21

Fix changelog code

abbb142

Fix visibility of StatusCode

34b3dea

Fix more CI issues

759c58d

jdisanti mentioned this pull request Nov 15, 2023

Rust: Fix examples after Rust SDK breaking changes awsdocs/aws-doc-sdk-examples#5662

Merged

rcoh approved these changes Nov 15, 2023

View reviewed changes

jdisanti added 2 commits November 15, 2023 09:33

Tweak deprecation message

a2da78e

Merge remote-tracking branch 'origin/main' into jdisanti-unhandled-er…

e5fef35

…ror-refactor

jdisanti removed the needs-sdk-review label Nov 15, 2023

jdisanti enabled auto-merge November 15, 2023 17:50

jdisanti disabled auto-merge November 15, 2023 17:51

jdisanti merged commit c830caa into main Nov 15, 2023
40 of 41 checks passed

jdisanti deleted the jdisanti-unhandled-error-refactor branch November 15, 2023 18:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revise unhandled error variant according to RFC-39 #3191

Revise unhandled error variant according to RFC-39 #3191

jdisanti commented Nov 14, 2023 •

edited

Loading

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

rcoh left a comment

rcoh Nov 14, 2023

rcoh Nov 15, 2023

jdisanti Nov 15, 2023

rcoh Nov 15, 2023

jdisanti Nov 15, 2023

rcoh Nov 15, 2023

jdisanti Nov 15, 2023

jdisanti commented Nov 15, 2023 •

edited

Loading

jdisanti commented Nov 15, 2023

github-actions bot commented Nov 15, 2023

github-actions bot commented Nov 15, 2023

rcoh commented Nov 15, 2023

jdisanti commented Nov 15, 2023

github-actions bot commented Nov 15, 2023

Revise unhandled error variant according to RFC-39 #3191

Revise unhandled error variant according to RFC-39 #3191

Conversation

jdisanti commented Nov 14, 2023 • edited Loading

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

rcoh left a comment

Choose a reason for hiding this comment

rcoh Nov 14, 2023

Choose a reason for hiding this comment

rcoh Nov 15, 2023

Choose a reason for hiding this comment

jdisanti Nov 15, 2023

Choose a reason for hiding this comment

rcoh Nov 15, 2023

Choose a reason for hiding this comment

jdisanti Nov 15, 2023

Choose a reason for hiding this comment

rcoh Nov 15, 2023

Choose a reason for hiding this comment

jdisanti Nov 15, 2023

Choose a reason for hiding this comment

jdisanti commented Nov 15, 2023 • edited Loading

jdisanti commented Nov 15, 2023

github-actions bot commented Nov 15, 2023

github-actions bot commented Nov 15, 2023

rcoh commented Nov 15, 2023

jdisanti commented Nov 15, 2023

github-actions bot commented Nov 15, 2023

jdisanti commented Nov 14, 2023 •

edited

Loading

jdisanti commented Nov 15, 2023 •

edited

Loading