larryh/corso
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

remove common errors

Browse Source 
Now that clues is in place, we don't need the
errors.As handling provided by common errors.
Current errors.As can be replaced with either
errors.Is checks, or HasLabel checks on the
status code.
This commit is contained in:
ryanfkeepers 2023-02-21 10:42:41 -07:00
parent 5cf020d5c8
commit 294ee7173e
7 changed files with 40 additions and 371 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
src/internal/common/errors.go
View File

@ -1,62 +0,0 @@
package common
import (
"fmt"
"io"
)
// TODO: Remove in favor of clues.Stack()
// Err provides boiler-plate functions that other types of errors can use
// if they wish to be compared with `errors.As()`. This struct ensures that
// stack traces are printed when requested (if present) and that Err
// chains `errors.As()`, `errors.Is()`, and `errors.Cause()` calls properly.
//
// When using errors.As, note that the variable that is passed as the second
// parameter must be a pointer to a type that exactly matches the returned type of the error previously. For
// example, if a struct was returned, the second parameter should be a pointer
// to said struct. If a pointer to a struct was returned, then a pointer to a
// pointer of the struct should be passed.
type Err struct {
Err error
}
func EncapsulateError(e error) *Err {
return &Err{Err: e}
}
func (e Err) Error() string {
return e.Err.Error()
}
func (e Err) Cause() error {
return e.Err
}
func (e Err) Unwrap() error {
return e.Err
}
// Format complies with the Formatter interface and gives pretty printing when
// functions like `fmt.Printf("%+v")` are called. Implementing this allows Err
// to print stack traces from the encapsulated error.
func (e Err) Format(s fmt.State, verb rune) {
if f, ok := e.Err.(fmt.Formatter); ok {
f.Format(s, verb)
return
}
// Formatting magic courtesy of github.com/pkg/errors.
switch verb {
case 'v':
if s.Flag('+') {
fmt.Fprintf(s, "%+v\n", e.Cause())
return
}
fallthrough
case 's', 'q':
// nolint:errcheck
_, _ = io.WriteString(s, e.Error())
}
}

84
src/internal/common/errors_test.go
View File

@ -1,84 +0,0 @@
package common_test
import (
"fmt"
"testing"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
"github.com/alcionai/corso/src/internal/common"
"github.com/alcionai/corso/src/internal/tester"
)
type testErr struct {
common.Err
}
type testErr2 struct {
common.Err
}
type ErrorsUnitSuite struct {
tester.Suite
}
func TestErrorsUnitSuite(t *testing.T) {
s := &ErrorsUnitSuite{Suite: tester.NewUnitSuite(t)}
suite.Run(t, s)
}
func (suite *ErrorsUnitSuite) TestPropagatesCause() {
err := assert.AnError
te := testErr{*common.EncapsulateError(err)}
te2 := testErr2{*common.EncapsulateError(te)}
assert.Equal(suite.T(), assert.AnError, errors.Cause(te2))
}
func (suite *ErrorsUnitSuite) TestPropagatesIs() {
err := assert.AnError
te := testErr{*common.EncapsulateError(err)}
te2 := testErr2{*common.EncapsulateError(te)}
assert.True(suite.T(), errors.Is(te2, err))
}
func (suite *ErrorsUnitSuite) TestPropagatesAs() {
err := assert.AnError
te := testErr{*common.EncapsulateError(err)}
te2 := testErr2{*common.EncapsulateError(te)}
tmp := testErr{}
assert.True(suite.T(), errors.As(te2, &tmp))
}
func (suite *ErrorsUnitSuite) TestAs() {
err := assert.AnError
te := testErr{*common.EncapsulateError(err)}
te2 := testErr2{*common.EncapsulateError(te)}
tmp := testErr2{}
assert.True(suite.T(), errors.As(te2, &tmp))
}
func (suite *ErrorsUnitSuite) TestAsIsUnique() {
err := assert.AnError
te := testErr{*common.EncapsulateError(err)}
tmp := testErr2{}
assert.False(suite.T(), errors.As(te, &tmp))
}
func (suite *ErrorsUnitSuite) TestPrintsStack() {
err := assert.AnError
err = errors.Wrap(err, "wrapped error")
te := testErr{*common.EncapsulateError(err)}
te2 := testErr2{*common.EncapsulateError(te)}
out := fmt.Sprintf("%+v", te2)
// Stack trace should include a line noting that we're running testify.
assert.Contains(suite.T(), out, "testify")
}

5
src/internal/connector/exchange/exchange_data_collection_test.go
View File

@ -10,7 +10,6 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/alcionai/corso/src/internal/common"
"github.com/alcionai/corso/src/internal/connector/graph"
"github.com/alcionai/corso/src/internal/data"
"github.com/alcionai/corso/src/internal/tester"
@ -214,9 +213,7 @@ func (suite *ExchangeDataCollectionSuite) TestGetItemWithRetries() {
{
name: "deleted in flight",
items: &mockItemer{
getErr: graph.ErrDeletedInFlight{
Err: *common.EncapsulateError(assert.AnError),
},
getErr: graph.ErrDeletedInFlight,
},
expectErr: func(t *testing.T, err error) {
assert.True(t, graph.IsErrDeletedInFlight(err), "is ErrDeletedInFlight")

3
src/internal/connector/exchange/service_iterators_test.go
View File

@ -9,7 +9,6 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/alcionai/corso/src/internal/common"
"github.com/alcionai/corso/src/internal/connector/exchange/api"
"github.com/alcionai/corso/src/internal/connector/graph"
"github.com/alcionai/corso/src/internal/connector/support"
@ -140,7 +139,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections() {
added: []string{"a1", "a2", "a3"},
removed: []string{"r1", "r2", "r3"},
newDelta: api.DeltaUpdate{URL: "delta_url"},
err: graph.ErrDeletedInFlight{Err: *common.EncapsulateError(assert.AnError)},
err: graph.ErrDeletedInFlight,
}
container1 = mockContainer{
id: strPtr("1"),

126
src/internal/connector/graph/errors.go
View File

@ -13,7 +13,6 @@ import (
"golang.org/x/exp/slices"
"github.com/alcionai/clues"
"github.com/alcionai/corso/src/internal/common"
"github.com/alcionai/corso/src/internal/common/ptr"
)
@ -32,18 +31,7 @@ const (
errCodeMailboxNotEnabledForRESTAPI = "MailboxNotEnabledForRESTAPI"
)
var (
Err401Unauthorized = errors.New("401 unauthorized")
// normally the graph client will catch this for us, but in case we
// run our own client Do(), we need to translate it to a timeout type
// failure locally.
Err429TooManyRequests = errors.New("429 too many requests")
Err503ServiceUnavailable = errors.New("503 Service Unavailable")
Err504GatewayTimeout = errors.New("504 Gateway Timeout")
Err500InternalServerError = errors.New("500 Internal Server Error")
)
var (
const (
mysiteURLNotFound = "unable to retrieve user's mysite URL"
mysiteNotFound = "user's mysite not found"
)
@ -54,15 +42,25 @@ var Labels = struct {
MysiteNotFound: "mysite_not_found",
}
// The folder or item was deleted between the time we identified
// it and when we tried to fetch data for it.
type ErrDeletedInFlight struct {
common.Err
}
var (
// The folder or item was deleted between the time we identified
// it and when we tried to fetch data for it.
ErrDeletedInFlight = clues.New("deleted in flight")
// Delta tokens can be desycned or expired. In either case, the token
// becomes invalid, and cannot be used again.
// https://learn.microsoft.com/en-us/graph/errors#code-property
ErrInvalidDelta = clues.New("inalid delta token")
// Timeout errors are identified for tracking the need to retry calls.
// Other delay errors, like throttling, are already handled by the
// graph client's built-in retries.
// https://github.com/microsoftgraph/msgraph-sdk-go/issues/302
ErrTimeout = clues.New("communication timeout")
)
func IsErrDeletedInFlight(err error) bool {
e := ErrDeletedInFlight{}
if errors.As(err, &e) {
if errors.Is(err, ErrDeletedInFlight) {
return true
}
@ -73,103 +71,39 @@ func IsErrDeletedInFlight(err error) bool {
return false
}
// Delta tokens can be desycned or expired. In either case, the token
// becomes invalid, and cannot be used again.
// https://learn.microsoft.com/en-us/graph/errors#code-property
type ErrInvalidDelta struct {
common.Err
}
func IsErrInvalidDelta(err error) bool {
e := ErrInvalidDelta{}
if errors.As(err, &e) {
return true
}
if hasErrorCode(err, errCodeSyncStateNotFound, errCodeResyncRequired) {
return true
}
return false
return hasErrorCode(err, errCodeSyncStateNotFound, errCodeResyncRequired) ||
errors.Is(err, ErrInvalidDelta)
}
func IsErrExchangeMailFolderNotFound(err error) bool {
return hasErrorCode(err, errCodeResourceNotFound, errCodeMailboxNotEnabledForRESTAPI)
}
// Timeout errors are identified for tracking the need to retry calls.
// Other delay errors, like throttling, are already handled by the
// graph client's built-in retries.
// https://github.com/microsoftgraph/msgraph-sdk-go/issues/302
type ErrTimeout struct {
common.Err
}
func IsErrTimeout(err error) bool {
e := ErrTimeout{}
if errors.As(err, &e) {
return true
}
if errors.Is(err, context.DeadlineExceeded) || os.IsTimeout(err) || errors.Is(err, http.ErrHandlerTimeout) {
return true
}
switch err := err.(type) {
case *url.Error:
return err.Timeout()
default:
return false
}
}
type ErrThrottled struct {
common.Err
}
func IsErrThrottled(err error) bool {
if errors.Is(err, Err429TooManyRequests) {
return true
}
if hasErrorCode(err, errCodeActivityLimitReached) {
return true
}
e := ErrThrottled{}
return errors.As(err, &e)
}
type ErrUnauthorized struct {
common.Err
return errors.Is(err, ErrTimeout) ||
errors.Is(err, context.DeadlineExceeded) ||
errors.Is(err, http.ErrHandlerTimeout) ||
os.IsTimeout(err)
}
func IsErrUnauthorized(err error) bool {
// TODO: refine this investigation. We don't currently know if
// a specific item download url expired, or if the full connection
// auth expired.
if errors.Is(err, Err401Unauthorized) {
return true
}
e := ErrUnauthorized{}
return errors.As(err, &e)
return clues.HasLabel(err, LabelStatus(http.StatusUnauthorized))
}
type ErrInternalServerError struct {
common.Err
}
func IsInternalServerError(err error) bool {
if errors.Is(err, Err500InternalServerError) {
return true
}
e := ErrInternalServerError{}
return errors.As(err, &e)
// LabelStatus transforms the provided statusCode into
// a standard label that can be attached to a clues error
// and later reviewed when checking error statuses.
func LabelStatus(statusCode int) string {
return fmt.Sprintf("status_code_%d", statusCode)
}
// ---------------------------------------------------------------------------

110
src/internal/connector/graph/errors_test.go
View File

@ -7,8 +7,6 @@ import (
"github.com/microsoftgraph/msgraph-sdk-go/models/odataerrors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
"github.com/alcionai/corso/src/internal/common"
)
type GraphErrorsUnitSuite struct {
@ -46,7 +44,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrDeletedInFlight() {
},
{
name: "as",
err: ErrDeletedInFlight{Err: *common.EncapsulateError(assert.AnError)},
err: ErrDeletedInFlight,
expect: assert.True,
},
{
@ -90,7 +88,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrInvalidDelta() {
},
{
name: "as",
err: ErrInvalidDelta{Err: *common.EncapsulateError(assert.AnError)},
err: ErrInvalidDelta,
expect: assert.True,
},
{
@ -129,7 +127,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrTimeout() {
},
{
name: "as",
err: ErrTimeout{Err: *common.EncapsulateError(assert.AnError)},
err: ErrTimeout,
expect: assert.True,
},
{
@ -144,105 +142,3 @@ func (suite *GraphErrorsUnitSuite) TestIsErrTimeout() {
})
}
}
func (suite *GraphErrorsUnitSuite) TestIsErrThrottled() {
table := []struct {
name string
err error
expect assert.BoolAssertionFunc
}{
{
name: "nil",
err: nil,
expect: assert.False,
},
{
name: "non-matching",
err: assert.AnError,
expect: assert.False,
},
{
name: "as",
err: ErrThrottled{Err: *common.EncapsulateError(assert.AnError)},
expect: assert.True,
},
{
name: "is429",
err: Err429TooManyRequests,
expect: assert.True,
},
}
for _, test := range table {
suite.T().Run(test.name, func(t *testing.T) {
test.expect(t, IsErrThrottled(test.err))
})
}
}
func (suite *GraphErrorsUnitSuite) TestIsErrUnauthorized() {
table := []struct {
name string
err error
expect assert.BoolAssertionFunc
}{
{
name: "nil",
err: nil,
expect: assert.False,
},
{
name: "non-matching",
err: assert.AnError,
expect: assert.False,
},
{
name: "as",
err: ErrUnauthorized{Err: *common.EncapsulateError(assert.AnError)},
expect: assert.True,
},
{
name: "is429",
err: Err401Unauthorized,
expect: assert.True,
},
}
for _, test := range table {
suite.T().Run(test.name, func(t *testing.T) {
test.expect(t, IsErrUnauthorized(test.err))
})
}
}
func (suite *GraphErrorsUnitSuite) TestIsInternalServerError() {
table := []struct {
name string
err error
expect assert.BoolAssertionFunc
}{
{
name: "nil",
err: nil,
expect: assert.False,
},
{
name: "non-matching",
err: assert.AnError,
expect: assert.False,
},
{
name: "as",
err: ErrInternalServerError{Err: *common.EncapsulateError(assert.AnError)},
expect: assert.True,
},
{
name: "is429",
err: Err500InternalServerError,
expect: assert.True,
},
}
for _, test := range table {
suite.T().Run(test.name, func(t *testing.T) {
test.expect(t, IsInternalServerError(test.err))
})
}
}

21
src/internal/connector/onedrive/item.go
View File

@ -135,23 +135,12 @@ func downloadItem(ctx context.Context, hc *http.Client, item models.DriveItemabl
return resp, nil
}
if resp.StatusCode == http.StatusTooManyRequests {
return resp, graph.Err429TooManyRequests
}
// upstream error checks can compare the status with
// clues.HasLabel(err, graph.LabelStatus(http.KnownStatusCode))
cerr := clues.Wrap(clues.New(resp.Status), "non-2xx http response").
Label(graph.LabelStatus(resp.StatusCode))
if resp.StatusCode == http.StatusUnauthorized {
return resp, graph.Err401Unauthorized
}
if resp.StatusCode == http.StatusInternalServerError {
return resp, graph.Err500InternalServerError
}
if resp.StatusCode == http.StatusServiceUnavailable {
return resp, graph.Err503ServiceUnavailable
}
return resp, clues.Wrap(clues.New(resp.Status), "non-2xx http response")
return resp, cerr
}
// oneDriveItemInfo will populate a details.OneDriveInfo struct