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

add TokenError #3

Merged
merged 1 commit into from
Jan 5, 2023
Merged

add TokenError #3

merged 1 commit into from
Jan 5, 2023

Conversation

lansi951
Copy link
Contributor

@lansi951 lansi951 commented Jan 4, 2023

If ErrTokenExpired occured, want to read previous token and extend expire at.

However, to get previous token in ErrorHandler, need to parse one more time in ErrorHandler.

Using ErrorHandlerWithToken, can get previous token directly without re-parsing.

@aldas
Copy link
Contributor

aldas commented Jan 4, 2023

Hi, maybe more eloquent way would be to introduce new error struct that will hold parsed error and token, has Unwrap(), Error() etc methods - like fs.PathError does it.

@aldas
Copy link
Contributor

aldas commented Jan 4, 2023

type TokenError struct {
	Token *jwt.Token
	Err   error
}

func (e *TokenError) Error() string { return e.Err.Error() }

func (e *TokenError) Unwrap() error { return e.Err }

and modify defaultParseTokenFunc and defaultKeyFunc to return it when error occurs

@lansi951
Copy link
Contributor Author

lansi951 commented Jan 4, 2023

@aldas Hi! Thanks for good review.
I modified commit.

@lansi951 lansi951 changed the title add ErrorHandlerWithToken add TokenError Jan 4, 2023
@aldas
Copy link
Contributor

aldas commented Jan 4, 2023

This looks OK but needs comments to pass linter. Fix these and you are good to go!

@lansi951
Copy link
Contributor Author

lansi951 commented Jan 5, 2023

I added comments, but I can't speak english well.
Are comments ok?

Copy link
Contributor

@aldas aldas left a comment

Choose a reason for hiding this comment

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

LGTM

@aldas aldas merged commit 1f68bd7 into labstack:main Jan 5, 2023
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