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

Refactor SDK's error types #74

Closed
jasdel opened this issue Dec 22, 2017 · 5 comments
Closed

Refactor SDK's error types #74

jasdel opened this issue Dec 22, 2017 · 5 comments
Labels
breaking-change Issue requires a breaking change to remediate. refactor

Comments

@jasdel
Copy link
Contributor

jasdel commented Dec 22, 2017

The v1 SDK exposes a single interface that most errors in the SDK try to conform to. awserr.Error. This error doesn't make sense for a lot of the errors the SDK core and utilities expose. Specifically the code and message values only make sense for API response errors.

The v2 SDK should refactor awserr.Error into an APIError interface type that basically is the awserr.RequestFailure type. I think the generic awserr.Error can be replaced in favor with errors that support the github.com/pkg/errors package's Wrap and Cause pattern.

Refactoring these error types should also satisfy a goal of error checking in user applications should be based more on type assertions instead of string comparison.

@jasdel jasdel modified the milestone: AWS SDK for Go v2 GA Mar 5, 2019
@rajender
Copy link

rajender commented May 6, 2019

Looks like Go1.13 may have more error inspection capabilities. We should consider it part of this.

golang/go#29934

https://godoc.org/golang.org/x/xerrors

https://tip.golang.org/pkg/errors/
https://tip.golang.org/pkg/fmt/#Errorf

@jasdel
Copy link
Contributor Author

jasdel commented Jun 3, 2019

Thanks for the suggestion @rajender. I agree taking advantage of xerrors package with 1.13 would help to standardize the SDK's error chaining. Right now this chaining is an odd mix of hand rolled chain, and github.com/pkg/errors.

This would mean the SDK's minimum supported version would raise go Go 1.13. Since the SDK is still in developer preview, this is may not be a deal breaker, but 1.13 most likely won't release until around August. The SDK may need to consider back filling support for xerrors for older version of Go if 1.13 ends up being too recent of a change when the SDK does GA release.

@jasdel jasdel added the breaking-change Issue requires a breaking change to remediate. label Jul 11, 2019
@jasdel
Copy link
Contributor Author

jasdel commented Aug 6, 2019

APIErrors returned from an API operation call should probably include the metadata for the operation request/response that failed. This context would help will logging, and identifying context of a failed operation call.

Does it make sense to expose this behavior to all SDK errors that are returned from an operation call? One way this could happen is the operation's Send potentially could wrap the underlying error to ensure context is maintained.

@jasdel
Copy link
Contributor Author

jasdel commented Mar 10, 2020

The SDK's error handling refactor should be built with Unwrap in mind so that layering of errors can easily be peeled apart.

@jasdel
Copy link
Contributor Author

jasdel commented Mar 18, 2020

#487 made significant changes to the SDK's awserr.Error and internal error wrapping. The SDK now supports Unwrap in numerous places in the SDK.

There are still several instances of SDK logic errors that are using the awserr.Error Code wrapper. Ideally the SDK doesn't use awserr.Error for any non-API error response. All SDK logic errors are typed, or generic errors. The outstanding SDK logic awserr.Error usage are a good target for further burn down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Issue requires a breaking change to remediate. refactor
Projects
None yet
Development

No branches or pull requests

3 participants