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

aws-sdk-s3 - re-export aws_smithy_http::byte_stream::Error #600

Closed
1 of 2 tasks
jeremychone opened this issue Aug 6, 2022 · 5 comments
Closed
1 of 2 tasks

aws-sdk-s3 - re-export aws_smithy_http::byte_stream::Error #600

jeremychone opened this issue Aug 6, 2022 · 5 comments
Labels
feature-request A feature should be added or improved. good first issue Good for newcomers p2 This is a standard priority issue

Comments

@jeremychone
Copy link

Describe the feature

In the s3/src/lib.rs, it would be great to add aws_smithy_http::byte_stream::Error to the list of re-exported types.

This is probably a low priority and more convenient as a workable solution exists.

Use Case

When using AWS S3 SDK and wanting to write error convertors to S3-related Errors, we can do most of them, except the aws_smithy_http::byte_stream::Error since it is not re-exported from the S3 SDK.

The simple workaround is to add aws_smithy_http to the Cargo.toml, which might create lib out-of-sync issues at compile time.

Having one place to import all of those relevant to AWS S3 SDK errors will remove this little friction.

Proposed Solution

In s3/src/lib.rs add to the types submodule:

pub mod types {
    pub use aws_smithy_http::byte_stream::Error; // <<-- new line
    // ...
}

Other Information

I just hit an error today when I upgraded all my dependencies, and when I upgraded aws-smithy-http to 0.47.0 it caused some compiler issues.

Now, there might be a good reason why this should not be done, and if it is the case, feel free to disregard this feature request.

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
@jeremychone jeremychone added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 6, 2022
@aws-sdk-rust-ci aws-sdk-rust-ci added good first issue Good for newcomers and removed needs-triage This issue or PR still needs to be triaged. labels Aug 8, 2022
@rcoh
Copy link
Contributor

rcoh commented Aug 8, 2022

We probably need to either:

  • Switch to re-exporting the bytestream module
  • re-export it with a rename eg. ByteStreamError

It's probably a little misleading for types::Error to actually be a ByteStream error. Definitely agree that this would be nice to fix though

@awslabs awslabs deleted a comment from aws-sdk-rust-ci Aug 8, 2022
@mattklein123
Copy link

Funny enough I just came here to open a similar issue. Would it be possible to just rexport all of aws-smithy-http? I'm not sure the implications of this, but I have test code that looks something like:

fn make_http_response(status: http::StatusCode) -> SdkError<GetObjectError> {
  SdkError::ServiceError {
    err: GetObjectError::unhandled(error::Error::AwsSdk("fake".to_string())),
    raw: aws_smithy_http::operation::Response::new(
      http::Response::builder()
        .status(status)
        .body(aws_smithy_http::body::SdkBody::empty())
        .unwrap(),
    ),
  }
}

And I keep getting errors when aws_smithy_http gets bumped without the SDK getting bumped. Is there a better way to do what I'm trying to do?

@jdisanti
Copy link
Contributor

@mattklein123 - Thank you for the code snippet. That's very informative for what we should be re-exporting.

Tracking implementation of this in smithy-rs#1759.

jesuscovam added a commit to jesuscovam/aws-sdk-rust that referenced this issue Oct 13, 2022
@jmklix jmklix added the p2 This is a standard priority issue label Nov 14, 2022
@ysaito1001
Copy link
Collaborator

@github-actions
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. good first issue Good for newcomers p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

7 participants