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

Wrap errors with %w instead of %v #68

Merged
merged 1 commit into from
Aug 10, 2020

Conversation

d10i
Copy link
Contributor

@d10i d10i commented Aug 10, 2020

Description

Wrap errors using %w instead of %v. This allows handling errors based on the internal errors with errors.Is and errors.As, as explained here. This feature has been introduced in Go 1.13 and is back-compatible and as this repo already uses Go 1.14 it should not introduce breaking changes.

This allows returning different error types in the ValidationKeyGetter function and then use them in the error returned by CheckJWT. For example:

type MyCustomError struct {
	error       string
	serverError bool
}

// Implement error interface
func (e MyCustomError) Error() string {
	return e.error
}

func main() {
	mw := jwtmiddleware.New(jwtmiddleware.Options{
		ErrorHandler: func(w http.ResponseWriter, r *http.Request, err string) {
			// ...
		},
		ValidationKeyGetter: func(token *jwt.Token) (interface{}, error) {
			if somethingUnexpectedHappened() {
				return nil, MyCustomError{error: "Server error", serverError: true}
			}

			if badRequest() {
				return nil, MyCustomError{error: "Bad request", serverError: false}
			}

			// ...

			return publicKey, nil
		},
		SigningMethod: jwt.SigningMethodRS256,
	})

	err := mw.CheckJWT(...)
	if err != nil {
		var myCustomError MyCustomError
		
		if errors.As(err, &myCustomError) {
			if myCustomError.serverError {
				// Return HTTP 500 Internal Server Error
			}

			// Return HTTP 401 Unauthorized
		}

		// Default: Return HTTP 500 Internal Server Error
	}
}

References

Testing

No tests have been added because:

  • There doesn't currently seem to be tests testing this low-level type of thing at the moment.

  • It's non-breaking change that doesn't change the behaviour, unless errors.Is or errors.As are used.

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not master

@d10i d10i mentioned this pull request Aug 10, 2020
4 tasks
@grounded042
Copy link
Contributor

Great change. Thanks @d10i!

@grounded042 grounded042 merged commit a32d7af into auth0:master Aug 10, 2020
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