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

Revamp errors for aws-sigv4 #1937

Merged
merged 2 commits into from
Nov 12, 2022
Merged

Revamp errors for aws-sigv4 #1937

merged 2 commits into from
Nov 12, 2022

Conversation

jdisanti
Copy link
Collaborator

@jdisanti jdisanti commented Nov 3, 2022

Motivation and Context

This PR revamps errors in aws-sigv4 to adhere to RFC-0022: Error Context and Compatibility.

This PR is part of a series that will fully implement the RFC, tracked in #1926.

Changelog entries added in #1951.

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
Copy link

github-actions bot commented Nov 3, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@jdisanti jdisanti force-pushed the jdisanti-errors-aws-sigv4 branch from 88580e1 to 5c0f606 Compare November 3, 2022 17:12
@github-actions
Copy link

github-actions bot commented Nov 3, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link
Contributor

@ysaito1001 ysaito1001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Comment on lines +22 to +38
impl fmt::Display for SigningError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.kind {
SigningErrorKind::FailedToCreateCanonicalRequest { .. } => {
write!(f, "failed to create canonical request")
}
}
}
}

impl Error for SigningError {
fn source(&self) -> Option<&(dyn Error + 'static)> {
match &self.kind {
SigningErrorKind::FailedToCreateCanonicalRequest { source } => Some(source),
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this follows #1345?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The source doesn't get written in the Display impl so that it's not duplicated for error reporters that follow the source chain.

use std::str;

/// Signing error type
pub type Error = Box<dyn StdError + Send + Sync + 'static>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@@ -121,8 +121,6 @@ impl fmt::Debug for SigV4Signer {
}
}

pub type SigningError = Box<dyn Error + Send + Sync>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@jdisanti jdisanti marked this pull request as ready for review November 11, 2022 23:26
@jdisanti jdisanti requested a review from a team as a code owner November 11, 2022 23:26
@jdisanti jdisanti enabled auto-merge (squash) November 11, 2022 23:27
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@jdisanti jdisanti merged commit 653e4cd into main Nov 12, 2022
@jdisanti jdisanti deleted the jdisanti-errors-aws-sigv4 branch November 12, 2022 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants