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

Implement errors.Unwrap for awserr.Error #2820

Closed
TennyZhuang opened this issue Sep 9, 2019 · 5 comments
Closed

Implement errors.Unwrap for awserr.Error #2820

TennyZhuang opened this issue Sep 9, 2019 · 5 comments
Labels
feature-request A feature should be added or improved.

Comments

@TennyZhuang
Copy link

TennyZhuang commented Sep 9, 2019

Feature description

awserr.Error implement a method called Orig(), It's exactly same as errors.Unwrap defined in errors from go1.13. It's better to keep consistent with standard library.

@TennyZhuang TennyZhuang added the feature-request A feature should be added or improved. label Sep 9, 2019
@paxan
Copy link
Member

paxan commented May 30, 2020

We utilize context.WithTimeout(...) combined with various AWS Go SDK ...WithContext methods.

Because errors in our code may be wrapped, we use errors.Is to detect that a context deadline was exceeded:

if errors.Is(err, context.DeadlineExceeded) { ... }

Unfortunately, errors produced by awserr do not play well with the standard errors.Unwrap. We've ran into subtle bugs in our "deadline exceeds" logic. I resorted to doing stuff like this as a workaround:

c := make(chan error)
ctx, cancel := context.WithTimeout(ctx, 5*time.Minute)
defer cancel()

// move the cancellable logic into a goro!
go func() {
	defer close(c)
	if err := DoAWSThing1(ctx, ...); err != nil {
		c <- err
		return
	}
	if err := DoAWSThing2(ctx, ...); err != nil {
		c <- err
		return
	}
	if err := DoAWSThing3(ctx, ...); err != nil {
		c <- err
		return
	}
}()

select {
case <-ctx.Done():
	return ctx.Err()
case err := <-c:
	// extra defence against awserr.Error-s that bury
	// context deadline error in OrigErr :(
	if ctxerr := ctx.Err(); ctxerr != nil {
		return ctxerr
	}
	return err
}

@migueleliasweb
Copy link

if ctxerr := ctx.Err(); ctxerr != nil {
		return ctxerr
	}
	return err

OMG I'm having this exact problem. The errors returned are impossible to be checked using errors.Is() which makes VERY HARD to react to specific errors in the code.

I will definitely try your workaround @paxan ! Thanks for the idea!

@skotambkar
Copy link
Contributor

The aws/aws-sdk-go-v2 uses errors.Unwrap. We do not have plans to support this in V1 SDK. Please take a look at the v2 sdk.

@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.

@nolotz
Copy link

nolotz commented Jan 2, 2023

The aws/aws-sdk-go-v2 uses errors.Unwrap. We do not have plans to support this in V1 SDK. Please take a look at the v2 sdk.

Hi @skotambkar

Any chance to get it supported with a merge request like #4010? We have issues updating to v2 left alone that there are things that aren't even supported like DAX.

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.
Projects
None yet
Development

No branches or pull requests

5 participants