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

Better propagation of underlying errors #458

Closed
mitar opened this issue Aug 12, 2020 · 3 comments · Fixed by #479
Closed

Better propagation of underlying errors #458

mitar opened this issue Aug 12, 2020 · 3 comments · Fixed by #479
Labels
feat New feature or request. good first issue A good issue to tackle when being a novice to the project. help wanted We are looking for help on this one.
Milestone

Comments

@mitar
Copy link
Contributor

mitar commented Aug 12, 2020

In client_authentication.go there is code like:

			client, err = f.Store.GetClient(ctx, clientID)
			if err != nil {
				return nil, errors.WithStack(ErrInvalidClient.WithDebug(err.Error()))
			}

This means that all the information about the err (stack trace, other wrapped errors) is lost at that point. Could RFC6749Error implement WithError and Unwrap to allow chaining errors? So then the above would be:

return nil, errors.WithStack(ErrInvalidClient.WithError(err).WithDebug(err.Error()))

External facing nothing would change, but if you inspect returned error, you would have access to the underlying wrapped error using (now) standard error wrapping.

@aeneasr
Copy link
Member

aeneasr commented Aug 21, 2020

100% on this one!

@mitar
Copy link
Contributor Author

mitar commented Aug 21, 2020

Awesome.

@aeneasr aeneasr added feat New feature or request. good first issue A good issue to tackle when being a novice to the project. help wanted We are looking for help on this one. labels Aug 21, 2020
@aeneasr aeneasr added this to the unplanned milestone Aug 21, 2020
mitar added a commit to mitar/fosite that referenced this issue Sep 16, 2020
@mitar
Copy link
Contributor Author

mitar commented Sep 16, 2020

I made: #479

aeneasr pushed a commit that referenced this issue Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request. good first issue A good issue to tackle when being a novice to the project. help wanted We are looking for help on this one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants