Skip to content

Commit

Permalink
fix: improve error stack wrapping
Browse files Browse the repository at this point in the history
BREAKING CHANGES: As part of this change, the error interface and its fields have changed:

- `RFC6749Error.Name` was renamed to `RFC6749Error.ErrorField`.
- `RFC6749Error.Description` was renamed to `RFC6749Error.DescriptionField`.
- `RFC6749Error.Hint` was renamed to `RFC6749Error.HintField`.
- `RFC6749Error.Code` was renamed to `RFC6749Error.CodeField`.
- `RFC6749Error.Hint` was renamed to `RFC6749Error.HintField`.
- `RFC6749Error.WithCause()` was renamed to `RFC6749Error.WithWrap() *RFC6749Error` and alternatively to `RFC6749Error.Wrap()` (without return value) to standardize naming conventions around the new Go 1.14+ error interfaces.
  • Loading branch information
aeneasr committed Nov 16, 2020
1 parent e84fc17 commit 620d4c1
Show file tree
Hide file tree
Showing 48 changed files with 1,410 additions and 513 deletions.
9 changes: 3 additions & 6 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,11 @@ jobs:
steps:
- checkout
- setup_remote_docker
-
restore_cache:
- restore_cache:
keys:
- go-github-ory-fosite-v1-{{ checksum "go.sum" }}
-
run: go mod download
-
save_cache:
- run: go mod download
- save_cache:
key: go-github-ory-fosite-v1-{{ checksum "go.sum" }}
paths:
- "/go/pkg/mod"
Expand Down
2 changes: 1 addition & 1 deletion access_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (f *Fosite) writeJsonError(rw http.ResponseWriter, err error) {
return
}

rw.WriteHeader(rfcerr.Code)
rw.WriteHeader(rfcerr.CodeField)
// ignoring the error because the connection is broken when it happens
_, _ = rw.Write(js)
}
8 changes: 4 additions & 4 deletions access_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ func TestWriteAccessError_RFC6749(t *testing.T) {
require.NoError(t, err)

assert.Equal(t, c.code, params.Error)
assert.Equal(t, c.err.Hint, params.Hint)
assert.Equal(t, c.err.HintField, params.Hint)

expectDescription := c.err.Description
if c.err.Hint != "" {
expectDescription += " " + c.err.Hint
expectDescription := c.err.DescriptionField
if c.err.HintField != "" {
expectDescription += " " + c.err.HintField
}

if !c.debug {
Expand Down
12 changes: 7 additions & 5 deletions access_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"net/http"
"strings"

"github.com/ory/x/errorsx"

"github.com/pkg/errors"
)

Expand Down Expand Up @@ -59,11 +61,11 @@ func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
accessRequest := NewAccessRequest(session)

if r.Method != "POST" {
return accessRequest, errors.WithStack(ErrInvalidRequest.WithHintf("HTTP method is '%s', expected 'POST'.", r.Method))
return accessRequest, errorsx.WithStack(ErrInvalidRequest.WithHintf("HTTP method is '%s', expected 'POST'.", r.Method))
} else if err := r.ParseMultipartForm(1 << 20); err != nil && err != http.ErrNotMultipart {
return accessRequest, errors.WithStack(ErrInvalidRequest.WithHint("Unable to parse HTTP body, make sure to send a properly formatted form request body.").WithCause(err).WithDebug(err.Error()))
return accessRequest, errorsx.WithStack(ErrInvalidRequest.WithHint("Unable to parse HTTP body, make sure to send a properly formatted form request body.").WithWrap(err).WithDebug(err.Error()))
} else if len(r.PostForm) == 0 {
return accessRequest, errors.WithStack(ErrInvalidRequest.WithHint("The POST body can not be empty."))
return accessRequest, errorsx.WithStack(ErrInvalidRequest.WithHint("The POST body can not be empty."))
}

accessRequest.Form = r.PostForm
Expand All @@ -75,7 +77,7 @@ func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
accessRequest.SetRequestedAudience(GetAudiences(r.PostForm))
accessRequest.GrantTypes = RemoveEmpty(strings.Split(r.PostForm.Get("grant_type"), " "))
if len(accessRequest.GrantTypes) < 1 {
return accessRequest, errors.WithStack(ErrInvalidRequest.WithHint("Request parameter 'grant_type' is missing"))
return accessRequest, errorsx.WithStack(ErrInvalidRequest.WithHint("Request parameter 'grant_type' is missing"))
}

client, err := f.AuthenticateClient(ctx, r, r.PostForm)
Expand All @@ -96,7 +98,7 @@ func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
}

if !found {
return nil, errors.WithStack(ErrInvalidRequest)
return nil, errorsx.WithStack(ErrInvalidRequest)
}
return accessRequest, nil
}
4 changes: 3 additions & 1 deletion access_response_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ package fosite
import (
"context"

"github.com/ory/x/errorsx"

"github.com/pkg/errors"
)

Expand All @@ -43,7 +45,7 @@ func (f *Fosite) NewAccessResponse(ctx context.Context, requester AccessRequeste
}

if response.GetAccessToken() == "" || response.GetTokenType() == "" {
return nil, errors.WithStack(ErrServerError.WithHint("An internal server occurred while trying to complete the request.").WithDebug("Access token or token type not set by TokenEndpointHandlers."))
return nil, errorsx.WithStack(ErrServerError.WithHint("An internal server occurred while trying to complete the request.").WithDebug("Access token or token type not set by TokenEndpointHandlers."))
}

return response, nil
Expand Down
10 changes: 5 additions & 5 deletions audience_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"net/url"
"strings"

"github.com/pkg/errors"
"github.com/ory/x/errorsx"
)

type AudienceMatchingStrategy func(haystack []string, needle []string) error
Expand All @@ -18,14 +18,14 @@ func DefaultAudienceMatchingStrategy(haystack []string, needle []string) error {
for _, n := range needle {
nu, err := url.Parse(n)
if err != nil {
return errors.WithStack(ErrInvalidRequest.WithHintf("Unable to parse requested audience '%s'.", n).WithCause(err).WithDebug(err.Error()))
return errorsx.WithStack(ErrInvalidRequest.WithHintf("Unable to parse requested audience '%s'.", n).WithWrap(err).WithDebug(err.Error()))
}

var found bool
for _, h := range haystack {
hu, err := url.Parse(h)
if err != nil {
return errors.WithStack(ErrInvalidRequest.WithHintf("Unable to parse whitelisted audience '%s'.", h).WithCause(err).WithDebug(err.Error()))
return errorsx.WithStack(ErrInvalidRequest.WithHintf("Unable to parse whitelisted audience '%s'.", h).WithWrap(err).WithDebug(err.Error()))
}

allowedPath := strings.TrimRight(hu.Path, "/")
Expand All @@ -39,7 +39,7 @@ func DefaultAudienceMatchingStrategy(haystack []string, needle []string) error {
}

if !found {
return errors.WithStack(ErrInvalidRequest.WithHintf("Requested audience '%s' has not been whitelisted by the OAuth 2.0 Client.", n))
return errorsx.WithStack(ErrInvalidRequest.WithHintf("Requested audience '%s' has not been whitelisted by the OAuth 2.0 Client.", n))
}
}

Expand All @@ -64,7 +64,7 @@ func ExactAudienceMatchingStrategy(haystack []string, needle []string) error {
}

if !found {
return errors.WithStack(ErrInvalidRequest.WithHintf(`Requested audience "%s" has not been whitelisted by the OAuth 2.0 Client.`, n))
return errorsx.WithStack(ErrInvalidRequest.WithHintf(`Requested audience "%s" has not been whitelisted by the OAuth 2.0 Client.`, n))
}
}

Expand Down
2 changes: 1 addition & 1 deletion authorize_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (f *Fosite) WriteAuthorizeError(rw http.ResponseWriter, ar AuthorizeRequest
return
}

rw.WriteHeader(rfcerr.Code)
rw.WriteHeader(rfcerr.CodeField)
_, _ = rw.Write(js)
return
}
Expand Down
5 changes: 3 additions & 2 deletions authorize_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ import (
"regexp"
"strings"

"github.com/ory/x/errorsx"

"github.com/asaskevich/govalidator"
"github.com/pkg/errors"
)

var FormPostDefaultTemplate = template.Must(template.New("form_post").Parse(`<html>
Expand Down Expand Up @@ -93,7 +94,7 @@ func MatchRedirectURIWithClientRedirectURIs(rawurl string, client Client) (*url.
}
}

return nil, errors.WithStack(ErrInvalidRequest.WithHint("The 'redirect_uri' parameter does not match any of the OAuth 2.0 Client's pre-registered redirect urls."))
return nil, errorsx.WithStack(ErrInvalidRequest.WithHint("The 'redirect_uri' parameter does not match any of the OAuth 2.0 Client's pre-registered redirect urls."))
}

// Match a requested redirect URI against a pool of registered client URIs
Expand Down
Loading

0 comments on commit 620d4c1

Please sign in to comment.