Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impl ProvideErrorMetadata for [service]::Error #780

Closed
2 tasks
imp opened this issue Apr 8, 2023 · 7 comments
Closed
2 tasks

impl ProvideErrorMetadata for [service]::Error #780

imp opened this issue Apr 8, 2023 · 7 comments
Labels
feature-request A feature should be added or improved. pending-release This issue will be fixed by an approved PR that hasn't been released yet.

Comments

@imp
Copy link
Contributor

imp commented Apr 8, 2023

Describe the feature

For each service (such as dynamodb, ec2, iam, etc) there is trait service::error::ProvideErrorMetadata which is implemented for all the variants of the service::Error. However the service::Error itself does not implement this trait. It may be useful to provide this out of the box. Automatically generating this impl block should not be a problem as is it should just call a respective .meta() method on the inner error variant.

Use Case

I am writing some error handling code and using service::Error is very convenient. Alas, after converting to this type I am loosing access to error details, and this makes general purpose diagnostic very hard.

Proposed Solution

Here is example implementation for aws_sdk_iam::Error (very similar to how RequestId is implemented):

impl ProvideErrorMetadata for Error {
    fn meta(&self) -> ErrorMetadata {
        match self {
            Self::ConcurrentModificationException(e) => e.meta(),
            Self::CredentialReportExpiredException(e) => e.meta(),
            Self::CredentialReportNotPresentException(e) => e.meta(),
            Self::CredentialReportNotReadyException(e) => e.meta(),
            Self::DeleteConflictException(e) => e.meta(),
            Self::DuplicateCertificateException(e) => e.meta(),
            Self::DuplicateSshPublicKeyException(e) => e.meta(),
            Self::EntityAlreadyExistsException(e) => e.meta(),
            Self::EntityTemporarilyUnmodifiableException(e) => e.meta(),
            Self::InvalidAuthenticationCodeException(e) => e.meta(),
            Self::InvalidCertificateException(e) => e.meta(),
            Self::InvalidInputException(e) => e.meta(),
            Self::InvalidPublicKeyException(e) => e.meta(),
            Self::InvalidUserTypeException(e) => e.meta(),
            Self::KeyPairMismatchException(e) => e.meta(),
            Self::LimitExceededException(e) => e.meta(),
            Self::MalformedCertificateException(e) => e.meta(),
            Self::MalformedPolicyDocumentException(e) => e.meta(),
            Self::NoSuchEntityException(e) => e.meta(),
            Self::PasswordPolicyViolationException(e) => e.meta(),
            Self::PolicyEvaluationException(e) => e.meta(),
            Self::PolicyNotAttachableException(e) => e.meta(),
            Self::ReportGenerationLimitExceededException(e) => e.meta(),
            Self::ServiceFailureException(e) => e.meta(),
            Self::ServiceNotSupportedException(e) => e.meta(),
            Self::UnmodifiableEntityException(e) => e.meta(),
            Self::UnrecognizedPublicKeyEncodingException(e) => e.meta(),
            Self::Unhandled(e) => e.meta(),
        }
    }
}

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

A note for the community

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue, please leave a comment
@imp imp added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Apr 8, 2023
@Velfi Velfi removed the needs-triage This issue or PR still needs to be triaged. label Apr 10, 2023
@Velfi
Copy link
Contributor

Velfi commented Apr 10, 2023

Hey @imp, thanks for submitting this issue. This would increase crate file sizes but it seems worth it. We'll add it to our backlog.

@imp
Copy link
Contributor Author

imp commented Apr 25, 2023

Perhaps fix for this could be implemented together with #784 (which was recently addressed by smithy-rs#2564).
It feels like both tickets belong to the same problem domain and could save a few cycles if addressed together.

@imp
Copy link
Contributor Author

imp commented May 26, 2023

Hello, is there anything that could be done to help progress with this issue? (Short of submitting actual PR - I am not familiar with Kotlin).

@jdisanti
Copy link
Contributor

Sorry for the delay on this. I've implemented the functionality in smithy-lang/smithy-rs#3189

github-merge-queue bot pushed a commit to smithy-lang/smithy-rs that referenced this issue Nov 14, 2023
This PR implements the `ProvideErrorMetadata` trait for service errors
as a prerequisite for implementing
[RFC-39](https://github.com/awslabs/smithy-rs/blob/main/design/src/rfcs/rfc0039_forward_compatible_errors.md).

Related SDK issue: awslabs/aws-sdk-rust#780

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
@jdisanti
Copy link
Contributor

This will go out in the next release.

@jdisanti jdisanti added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Nov 14, 2023
@jdisanti
Copy link
Contributor

This went out with the November 16th release.

Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. pending-release This issue will be fixed by an approved PR that hasn't been released yet.
Projects
None yet
Development

No branches or pull requests

3 participants