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

refactor structured errors/logs in pkg (#2323)

Browse Source 
## Description

Refactors errors and logs to support structured
data throughout the smaller packages within
pkg/...  Larger packages will come later.

## Does this PR need a docs update or release note?

- [x] ⛔ No 

## Type of change

- [x] 🧹 Tech Debt/Cleanup

## Issue(s)

* #1970

## Test Plan

- [x] ⚡ Unit test
This commit is contained in:
Keepers 2023-02-06 13:45:09 -07:00 committed by GitHub
parent c4cc3b1a23
commit faad5d35a4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 69 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/pkg/account/m365.go
View File

@ -1,6 +1,7 @@
package account
import (
"github.com/alcionai/clues"
"github.com/pkg/errors"
"github.com/alcionai/corso/src/pkg/credentials"
@ -66,7 +67,7 @@ func (c M365Config) validate() error {
for k, v := range check {
if len(v) == 0 {
return errors.Wrap(errMissingRequired, k)
return clues.Stack(errMissingRequired, errors.New(k))
}
}

3
src/pkg/credentials/aws.go
View File

@ -3,6 +3,7 @@ package credentials
import (
"os"
"github.com/alcionai/clues"
"github.com/pkg/errors"
)
@ -48,7 +49,7 @@ func (c AWS) Validate() error {
for k, v := range check {
if len(v) == 0 {
return errors.Wrap(errMissingRequired, k)
return clues.Stack(errMissingRequired, errors.New(k))
}
}

3
src/pkg/credentials/corso.go
View File

@ -3,6 +3,7 @@ package credentials
import (
"os"
"github.com/alcionai/clues"
"github.com/pkg/errors"
)
@ -34,7 +35,7 @@ func (c Corso) Validate() error {
for k, v := range check {
if len(v) == 0 {
return errors.Wrap(errMissingRequired, k)
return clues.Stack(errMissingRequired, errors.New(k))
}
}

3
src/pkg/credentials/m365.go
View File

@ -3,6 +3,7 @@ package credentials
import (
"os"
"github.com/alcionai/clues"
"github.com/pkg/errors"
)
@ -36,7 +37,7 @@ func (c M365) Validate() error {
for k, v := range check {
if len(v) == 0 {
return errors.Wrap(errMissingRequired, k)
return clues.Stack(errMissingRequired, errors.New(k))
}
}

11
src/pkg/path/onedrive.go
View File

@ -1,8 +1,6 @@
package path
import (
"github.com/pkg/errors"
)
import "github.com/alcionai/clues"
// drivePath is used to represent path components
// of an item within the drive i.e.
@ -20,10 +18,9 @@ func ToOneDrivePath(p Path) (*DrivePath, error) {
// Must be at least `drives/<driveID>/root:`
if len(folders) < 3 {
return nil, errors.Errorf(
"folder path doesn't match expected format for OneDrive items: %s",
p.Folder(),
)
return nil, clues.
New("folder path doesn't match expected format for OneDrive items").
With("folders", p.Folder())
}
return &DrivePath{DriveID: folders[1], Folders: folders[3:]}, nil

26
src/pkg/path/path.go
View File

@ -56,11 +56,10 @@ import (
"fmt"
"strings"
"github.com/alcionai/clues"
"github.com/pkg/errors"
)
const templateErrPathParsing = "parsing resource path from %s"
const (
escapeCharacter = '\\'
PathSeparator = '/'
@ -73,7 +72,10 @@ var charactersToEscape = map[rune]struct{}{
escapeCharacter: {},
}
var errMissingSegment = errors.New("missing required path element")
var (
errMissingSegment = errors.New("missing required path element")
errParsingPath = errors.New("parsing resource path")
)
// For now, adding generic functions to pull information from segments.
// Resources that don't have the requested information should return an empty
@ -252,11 +254,11 @@ func (pb Builder) join(start, end int) string {
func verifyInputValues(tenant, resourceOwner string) error {
if len(tenant) == 0 {
return errors.Wrap(errMissingSegment, "tenant")
return clues.Stack(errMissingSegment, errors.New("tenant"))
}
if len(resourceOwner) == 0 {
return errors.Wrap(errMissingSegment, "resourceOwner")
return clues.Stack(errMissingSegment, errors.New("resourceOwner"))
}
return nil
@ -418,17 +420,17 @@ func FromDataLayerPath(p string, isItem bool) (Path, error) {
p = TrimTrailingSlash(p)
// If p was just the path separator then it will be empty now.
if len(p) == 0 {
return nil, errors.Errorf("logically empty path given: %s", p)
return nil, clues.New("logically empty path given").With("path_string", p)
}
// Turn into a Builder to reuse code that ignores empty elements.
pb, err := Builder{}.UnescapeAndAppend(Split(p)...)
if err != nil {
return nil, errors.Wrapf(err, templateErrPathParsing, p)
return nil, clues.Stack(errParsingPath, err).With("path_string", p)
}
if len(pb.elements) < 5 {
return nil, errors.Errorf("path has too few segments: %s", p)
return nil, clues.New("path has too few segments").With("path_string", p)
}
service, category, err := validateServiceAndCategoryStrings(
@ -436,7 +438,7 @@ func FromDataLayerPath(p string, isItem bool) (Path, error) {
pb.elements[3],
)
if err != nil {
return nil, errors.Wrapf(err, templateErrPathParsing, p)
return nil, clues.Stack(errParsingPath, err).With("path_string", p)
}
return &dataLayerResourcePath{
@ -519,8 +521,8 @@ func validateEscapedElement(element string) error {
prevWasEscape = false
if _, ok := charactersToEscape[c]; !ok {
return errors.Errorf(
"bad escape sequence in path: '%c%c'", escapeCharacter, c)
return clues.New("bad escape sequence in path").
With("escape_sequence", fmt.Sprintf("'%c%c'", escapeCharacter, c))
}
case false:
@ -530,7 +532,7 @@ func validateEscapedElement(element string) error {
}
if _, ok := charactersToEscape[c]; ok {
return errors.Errorf("unescaped '%c' in path", c)
return clues.New("unescaped character in path").With("character", c)
}
}
}

17
src/pkg/path/resource_path.go
View File

@ -1,8 +1,10 @@
package path
import (
"fmt"
"strings"
"github.com/alcionai/clues"
"github.com/pkg/errors"
)
@ -119,12 +121,12 @@ var serviceCategories = map[ServiceType]map[CategoryType]struct{}{
func validateServiceAndCategoryStrings(s, c string) (ServiceType, CategoryType, error) {
service := toServiceType(s)
if service == UnknownService {
return UnknownService, UnknownCategory, errors.Wrapf(ErrorUnknownService, "%q", s)
return UnknownService, UnknownCategory, clues.Stack(ErrorUnknownService).With("service", fmt.Sprintf("%q", s))
}
category := ToCategoryType(c)
if category == UnknownCategory {
return UnknownService, UnknownCategory, errors.Wrapf(ErrorUnknownCategory, "%q", c)
return UnknownService, UnknownCategory, clues.Stack(ErrorUnknownService).With("category", fmt.Sprintf("%q", c))
}
if err := validateServiceAndCategory(service, category); err != nil {
@ -137,15 +139,12 @@ func validateServiceAndCategoryStrings(s, c string) (ServiceType, CategoryType,
func validateServiceAndCategory(service ServiceType, category CategoryType) error {
cats, ok := serviceCategories[service]
if !ok {
return errors.New("unsupported service")
return clues.New("unsupported service").With("service", fmt.Sprintf("%q", service))
}
if _, ok := cats[category]; !ok {
return errors.Errorf(
"unknown service/category combination %q/%q",
service,
category,
)
return clues.New("unknown service/category combination").
WithAll("service", fmt.Sprintf("%q", service), "category", fmt.Sprintf("%q", category))
}
return nil
@ -234,7 +233,7 @@ func (rp dataLayerResourcePath) Item() string {
func (rp dataLayerResourcePath) Dir() (Path, error) {
if len(rp.elements) <= 4 {
return nil, errors.Errorf("unable to shorten path %q", rp)
return nil, clues.New("unable to shorten path").With("path", fmt.Sprintf("%q", rp))
}
return &dataLayerResourcePath{

20
src/pkg/repository/repository.go
View File

@ -89,13 +89,17 @@ func Initialize(
s storage.Storage,
opts control.Options,
) (Repository, error) {
ctx = clues.AddAll(ctx, "acct_provider", acct.Provider, "storage_provider", s.Provider)
ctx = clues.AddAll(
ctx,
"acct_provider", acct.Provider.String(),
"acct_id", acct.ID(), // TODO: pii
"storage_provider", s.Provider.String())
kopiaRef := kopia.NewConn(s)
if err := kopiaRef.Initialize(ctx); err != nil {
// replace common internal errors so that sdk users can check results with errors.Is()
if kopia.IsRepoAlreadyExistsError(err) {
return nil, ErrorRepoAlreadyExists
return nil, clues.Stack(ErrorRepoAlreadyExists).WithClues(ctx)
}
return nil, errors.Wrap(err, "initializing kopia")
@ -153,7 +157,11 @@ func Connect(
s storage.Storage,
opts control.Options,
) (Repository, error) {
ctx = clues.AddAll(ctx, "acct_provider", acct.Provider, "storage_provider", s.Provider)
ctx = clues.AddAll(
ctx,
"acct_provider", acct.Provider.String(),
"acct_id", acct.ID(), // TODO: pii
"storage_provider", s.Provider.String())
// Close/Reset the progress bar. This ensures callers don't have to worry about
// their output getting clobbered (#1720)
@ -210,12 +218,12 @@ func Connect(
func (r *repository) Close(ctx context.Context) error {
if err := r.Bus.Close(); err != nil {
logger.Ctx(ctx).Debugw("closing the event bus", "err", err)
logger.Ctx(ctx).With("err", err).Debugw("closing the event bus", clues.In(ctx).Slice()...)
}
if r.dataLayer != nil {
if err := r.dataLayer.Close(ctx); err != nil {
logger.Ctx(ctx).Debugw("closing Datalayer", "err", err)
logger.Ctx(ctx).With("err", err).Debugw("closing Datalayer", clues.In(ctx).Slice()...)
}
r.dataLayer = nil
@ -223,7 +231,7 @@ func (r *repository) Close(ctx context.Context) error {
if r.modelStore != nil {
if err := r.modelStore.Close(ctx); err != nil {
logger.Ctx(ctx).Debugw("closing modelStore", "err", err)
logger.Ctx(ctx).With("err", err).Debugw("closing modelStore", clues.In(ctx).Slice()...)
}
r.modelStore = nil

32
src/pkg/services/m365/m365.go
View File

@ -3,6 +3,7 @@ package m365
import (
"context"
"github.com/alcionai/clues"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/pkg/errors"
@ -20,10 +21,10 @@ type User struct {
// Users returns a list of users in the specified M365 tenant
// TODO: Implement paging support
func Users(ctx context.Context, m365Account account.Account) ([]*User, error) {
gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), m365Account, connector.Users)
func Users(ctx context.Context, acct account.Account) ([]*User, error) {
gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), acct, connector.Users)
if err != nil {
return nil, errors.Wrap(err, "could not initialize M365 graph connection")
return nil, errors.Wrap(err, "initializing M365 graph connection")
}
users, err := discovery.Users(ctx, gc.Owners.Users())
@ -36,7 +37,7 @@ func Users(ctx context.Context, m365Account account.Account) ([]*User, error) {
for _, u := range users {
pu, err := parseUser(u)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "parsing userable")
}
ret = append(ret, pu)
@ -45,8 +46,8 @@ func Users(ctx context.Context, m365Account account.Account) ([]*User, error) {
return ret, nil
}
func UserIDs(ctx context.Context, m365Account account.Account) ([]string, error) {
users, err := Users(ctx, m365Account)
func UserIDs(ctx context.Context, acct account.Account) ([]string, error) {
users, err := Users(ctx, acct)
if err != nil {
return nil, err
}
@ -61,8 +62,8 @@ func UserIDs(ctx context.Context, m365Account account.Account) ([]string, error)
// UserPNs retrieves all user principleNames in the tenant. Principle Names
// can be used analogous userIDs in graph API queries.
func UserPNs(ctx context.Context, m365Account account.Account) ([]string, error) {
users, err := Users(ctx, m365Account)
func UserPNs(ctx context.Context, acct account.Account) ([]string, error) {
users, err := Users(ctx, acct)
if err != nil {
return nil, err
}
@ -76,20 +77,20 @@ func UserPNs(ctx context.Context, m365Account account.Account) ([]string, error)
}
// SiteURLs returns a list of SharePoint site WebURLs in the specified M365 tenant
func SiteURLs(ctx context.Context, m365Account account.Account) ([]string, error) {
gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), m365Account, connector.Sites)
func SiteURLs(ctx context.Context, acct account.Account) ([]string, error) {
gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), acct, connector.Sites)
if err != nil {
return nil, errors.Wrap(err, "could not initialize M365 graph connection")
return nil, errors.Wrap(err, "initializing M365 graph connection")
}
return gc.GetSiteWebURLs(), nil
}
// SiteURLs returns a list of SharePoint sites IDs in the specified M365 tenant
func SiteIDs(ctx context.Context, m365Account account.Account) ([]string, error) {
gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), m365Account, connector.Sites)
func SiteIDs(ctx context.Context, acct account.Account) ([]string, error) {
gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), acct, connector.Sites)
if err != nil {
return nil, errors.Wrap(err, "could not initialize M365 graph connection")
return nil, errors.Wrap(err, "initializing graph connection")
}
return gc.GetSiteIDs(), nil
@ -98,7 +99,8 @@ func SiteIDs(ctx context.Context, m365Account account.Account) ([]string, error)
// parseUser extracts information from `models.Userable` we care about
func parseUser(item models.Userable) (*User, error) {
if item.GetUserPrincipalName() == nil {
return nil, errors.Errorf("no principal name for User: %s", *item.GetId())
return nil, clues.New("user missing principal name").
With("user_id", *item.GetId()) // TODO: pii
}
u := &User{PrincipalName: *item.GetUserPrincipalName(), ID: *item.GetId()}

3
src/pkg/storage/common.go
View File

@ -1,6 +1,7 @@
package storage
import (
"github.com/alcionai/clues"
"github.com/pkg/errors"
"github.com/alcionai/corso/src/pkg/credentials"
@ -45,7 +46,7 @@ func (s Storage) CommonConfig() (CommonConfig, error) {
// ensures all required properties are present
func (c CommonConfig) validate() error {
if len(c.CorsoPassphrase) == 0 {
return errors.Wrap(errMissingRequired, credentials.CorsoPassphrase)
return clues.Stack(errMissingRequired, errors.New(credentials.CorsoPassphrase))
}
// kopiaCfgFilePath is not required

3
src/pkg/storage/s3.go
View File

@ -3,6 +3,7 @@ package storage
import (
"strconv"
"github.com/alcionai/clues"
"github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/common"
@ -81,7 +82,7 @@ func (c S3Config) validate() error {
}
for k, v := range check {
if len(v) == 0 {
return errors.Wrap(errMissingRequired, k)
return clues.Stack(errMissingRequired, errors.New(k))
}
}