Skip to content

Commit

Permalink
Fix errors.Error use throughout the stack by removing pointer assignment
Browse files Browse the repository at this point in the history 
This patch probably needs a second pass, but this works for now. In
particular, the errors.Error assertions are unwieldy -- the API will
need to change.

This change removes the pointer designation from *errors.Error to be
errors.Error, relying on the interface value of `error` to be nil for
our purposes.

This is much more idiomatic within the go ecosystem and things play
nicer as a result.

Signed-off-by: Erik Hollensbe <github@hollensbe.org>
  • Loading branch information
Erik Hollensbe committed Nov 2, 2019
1 parent 297ac99 commit 7edc5ca
Show file tree
Hide file tree
Showing 231 changed files with 999 additions and 1,062 deletions.
4 changes: 2 additions & 2 deletions api/assetsvc/log.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (as *AssetServer) GetLog(id *types.IntID, ag asset.Asset_GetLogServer) erro
func (as *AssetServer) submit(ap asset.Asset_PutLogServer, p string) (retErr error) {
defer func() {
if retErr != nil {
retErr = errors.New(retErr).ToGRPC(codes.FailedPrecondition)
retErr = errors.New(retErr).(errors.Error).ToGRPC(codes.FailedPrecondition)
md := metadata.New(nil)
md.Append("errors", retErr.Error())
ap.SetTrailer(md)
Expand Down Expand Up @@ -106,7 +106,7 @@ func write(ag asset.Asset_GetLogServer, buf []byte) error {
func (as *AssetServer) attach(id int64, ag asset.Asset_GetLogServer, p string) (retErr error) {
defer func() {
if retErr != nil {
retErr = errors.New(retErr).ToGRPC(codes.FailedPrecondition)
retErr = errors.New(retErr).(errors.Error).ToGRPC(codes.FailedPrecondition)
} else {
retErr = write(ag, []byte(color.New(color.FgGreen).Sprintln("---- LOG COMPLETE ----")))
}
Expand Down
2 changes: 1 addition & 1 deletion api/assetsvc/testserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

// MakeAssetServer makes an instance of the assetsvc on port 6000. It returns a
// chan which can be closed to terminate it, and any boot-time errors.
func MakeAssetServer() (*handler.H, chan struct{}, *errors.Error) {
func MakeAssetServer() (*handler.H, chan struct{}, error) {
t, err := transport.Listen(nil, "tcp", config.DefaultServices.Asset.String())
if err != nil {
return nil, nil, errors.New(err)
Expand Down
18 changes: 9 additions & 9 deletions api/auth/github/github.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (as *AuthServer) Capabilities(ctx context.Context, e *empty.Empty) (*auth.S
func (as *AuthServer) OAuthChallenge(ctx context.Context, ocr *auth.OAuthChallengeRequest) (*auth.OAuthInfo, error) {
scopes, eErr := as.H.Clients.Data.OAuthValidateState(ctx, ocr.State)
if eErr != nil {
return nil, eErr.Wrap("Locating state").ToGRPC(codes.FailedPrecondition)
return nil, eErr.(errors.Error).Wrap("Locating state").ToGRPC(codes.FailedPrecondition)
}

conf := as.H.OAuth.Config(scopes)
Expand All @@ -44,12 +44,12 @@ func (as *AuthServer) OAuthChallenge(ctx context.Context, ocr *auth.OAuthChallen
if err != nil {
switch err.(type) {
case *oauth2.RetrieveError:
return nil, errors.New(err).Wrap("exchanging code for a token").ToGRPC(codes.FailedPrecondition)
return nil, errors.New(err).(errors.Error).Wrap("exchanging code for a token").ToGRPC(codes.FailedPrecondition)
default:
as.H.Clients.Log.Error(ctx, err)
url, err := as.makeOAuthURL(ctx, scopes)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &auth.OAuthInfo{Url: url, Redirect: true}, nil
Expand All @@ -60,7 +60,7 @@ func (as *AuthServer) OAuthChallenge(ctx context.Context, ocr *auth.OAuthChallen
c := github.NewClient(client)
u, _, err := c.Users.Get(ctx, "")
if err != nil {
return nil, errors.New(err).Wrap("Looking up token user").ToGRPC(codes.FailedPrecondition)
return nil, errors.New(err).(errors.Error).Wrap("Looking up token user").ToGRPC(codes.FailedPrecondition)
}

user, eErr := as.H.Clients.Data.GetUser(ctx, u.GetLogin())
Expand All @@ -74,23 +74,23 @@ func (as *AuthServer) OAuthChallenge(ctx context.Context, ocr *auth.OAuthChallen
if eErr != nil { // same check as above; to determine whether to add or patch
user, eErr = as.H.Clients.Data.PutUser(ctx, user)
if eErr != nil {
return nil, eErr.Wrapf("Could not create user %v", u.GetLogin()).ToGRPC(codes.FailedPrecondition)
return nil, eErr.(errors.Error).Wrapf("Could not create user %v", u.GetLogin()).ToGRPC(codes.FailedPrecondition)
}
} else {
if err := as.H.Clients.Data.PatchUser(ctx, user); err != nil {
return nil, eErr.Wrapf("Could not patch user %v", u.GetLogin()).ToGRPC(codes.FailedPrecondition)
return nil, eErr.(errors.Error).Wrapf("Could not patch user %v", u.GetLogin()).ToGRPC(codes.FailedPrecondition)
}
}

return &auth.OAuthInfo{Username: user.Username}, nil
}

func (as *AuthServer) makeOAuthURL(ctx context.Context, scopes []string) (string, *errors.Error) {
func (as *AuthServer) makeOAuthURL(ctx context.Context, scopes []string) (string, error) {
conf := as.H.OAuth.Config(scopes)
state := strings.TrimRight(base32.StdEncoding.EncodeToString(securecookie.GenerateRandomKey(64)), "=")

if err := as.H.Clients.Data.OAuthRegisterState(ctx, state, scopes); err != nil {
return "", err.Wrap("registering state")
return "", err.(errors.Error).Wrap("registering state")
}

return conf.AuthCodeURL(
Expand All @@ -103,7 +103,7 @@ func (as *AuthServer) makeOAuthURL(ctx context.Context, scopes []string) (string
func (as *AuthServer) GetOAuthURL(ctx context.Context, scopes *auth.Scopes) (*auth.String, error) {
url, err := as.makeOAuthURL(ctx, scopes.List)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &auth.String{Str: url}, nil
Expand Down
10 changes: 5 additions & 5 deletions api/datasvc/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
func (ds *DataServer) GetErrors(ctx context.Context, name *data.Name) (*types.UserErrors, error) {
u, err := ds.H.Model.FindUserByName(name.Name)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

errors := &types.UserErrors{}
Expand All @@ -29,13 +29,13 @@ func (ds *DataServer) GetErrors(ctx context.Context, name *data.Name) (*types.Us
func (ds *DataServer) AddError(ctx context.Context, ue *types.UserError) (*empty.Empty, error) {
u, err := ds.H.Model.FindUserByID(ue.UserID)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

u.AddError(errors.New(ue.Error))

if err := ds.H.Model.Save(u).Error; err != nil {
return nil, errors.New(err).ToGRPC(codes.FailedPrecondition)
return nil, errors.New(err).(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &empty.Empty{}, nil
Expand All @@ -45,11 +45,11 @@ func (ds *DataServer) AddError(ctx context.Context, ue *types.UserError) (*empty
func (ds *DataServer) DeleteError(ctx context.Context, ue *types.UserError) (*empty.Empty, error) {
u, err := ds.H.Model.FindUserByID(ue.UserID)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

if err := ds.H.Model.DeleteError(u, ue.Id); err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &empty.Empty{}, nil
Expand Down
5 changes: 3 additions & 2 deletions api/datasvc/oauth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/golang/protobuf/ptypes/empty"
"github.com/tinyci/ci-agents/ci-gen/grpc/services/data"
"github.com/tinyci/ci-agents/errors"
"google.golang.org/grpc/codes"
)

Expand All @@ -13,7 +14,7 @@ import (
// to us.
func (ds *DataServer) OAuthRegisterState(ctx context.Context, oas *data.OAuthState) (*empty.Empty, error) {
if err := ds.H.Model.OAuthRegisterState(oas.State, oas.Scopes); err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &empty.Empty{}, nil
Expand All @@ -25,7 +26,7 @@ func (ds *DataServer) OAuthRegisterState(ctx context.Context, oas *data.OAuthSta
func (ds *DataServer) OAuthValidateState(ctx context.Context, oas *data.OAuthState) (*data.OAuthState, error) {
o, err := ds.H.Model.OAuthValidateState(oas.State)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return o.ToProto(), nil
Expand Down
35 changes: 21 additions & 14 deletions api/datasvc/queue.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
func (ds *DataServer) QueueCount(ctx context.Context, empty *empty.Empty) (*data.Count, error) {
res, err := ds.H.Model.QueueTotalCount()
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &data.Count{Count: res}, nil
Expand All @@ -27,12 +27,12 @@ func (ds *DataServer) QueueCount(ctx context.Context, empty *empty.Empty) (*data
func (ds *DataServer) QueueCountForRepository(ctx context.Context, repo *data.Name) (*data.Count, error) {
r, err := ds.H.Model.GetRepositoryByName(repo.Name)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

res, err := ds.H.Model.QueueTotalCountForRepository(r)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &data.Count{Count: res}, nil
Expand All @@ -42,17 +42,17 @@ func (ds *DataServer) QueueCountForRepository(ctx context.Context, repo *data.Na
func (ds *DataServer) QueueListForRepository(ctx context.Context, qlr *data.QueueListRequest) (*data.QueueList, error) {
r, err := ds.H.Model.GetRepositoryByName(qlr.Name)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

page, perPage, err := utils.ScopePaginationInt(qlr.Page, qlr.PerPage)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

list, err := ds.H.Model.QueueListForRepository(r, page, perPage)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

retList := &data.QueueList{}
Expand All @@ -71,15 +71,15 @@ func (ds *DataServer) QueueAdd(ctx context.Context, list *data.QueueList) (*data
for _, item := range list.Items {
it, err := model.NewQueueItemFromProto(item)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

modelItems = append(modelItems, it)
}

var err *errors.Error
var err error
if modelItems, err = ds.H.Model.QueuePipelineAdd(modelItems); err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

retList := &data.QueueList{}
Expand All @@ -95,8 +95,15 @@ func (ds *DataServer) QueueAdd(ctx context.Context, list *data.QueueList) (*data
func (ds *DataServer) QueueNext(ctx context.Context, r *types.QueueRequest) (*types.QueueItem, error) {
qi, err := ds.H.Model.NextQueueItem(r.RunningOn, r.QueueName)
if err != nil {
if err.Contains(errors.ErrNotFound) {
err.SetLog(false)
switch err.(type) {
case errors.Error, *errors.Error:
var e2 *errors.Error
if e, ok := err.(errors.Error); ok {
e2 = &e
}
if e2.Contains(errors.ErrNotFound) {
e2.SetLog(false)
}
}
return nil, err
}
Expand All @@ -107,7 +114,7 @@ func (ds *DataServer) QueueNext(ctx context.Context, r *types.QueueRequest) (*ty
// PutStatus sets the status for the given run_id
func (ds *DataServer) PutStatus(ctx context.Context, s *types.Status) (*empty.Empty, error) {
if err := ds.H.Model.SetRunStatus(s.Id, config.DefaultGithubClient(), s.Status, false, ds.H.URL, s.AdditionalMessage); err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &empty.Empty{}, nil
Expand All @@ -117,7 +124,7 @@ func (ds *DataServer) PutStatus(ctx context.Context, s *types.Status) (*empty.Em
// canceled. Will fail on finished tasks.
func (ds *DataServer) SetCancel(ctx context.Context, id *types.IntID) (*empty.Empty, error) {
if err := ds.H.Model.CancelRun(id.ID, ds.H.URL, config.DefaultGithubClient()); err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &empty.Empty{}, nil
Expand All @@ -128,7 +135,7 @@ func (ds *DataServer) GetCancel(ctx context.Context, id *types.IntID) (*types.St
s := &types.Status{Id: id.ID}
res, err := ds.H.Model.GetCancelForRun(id.ID)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

s.Status = res
Expand Down
9 changes: 5 additions & 4 deletions api/datasvc/ref.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/tinyci/ci-agents/ci-gen/grpc/services/data"
"github.com/tinyci/ci-agents/ci-gen/grpc/types"
"github.com/tinyci/ci-agents/config"
"github.com/tinyci/ci-agents/errors"
"github.com/tinyci/ci-agents/model"
"google.golang.org/grpc/codes"
)
Expand All @@ -15,7 +16,7 @@ import (
func (ds *DataServer) GetRefByNameAndSHA(ctx context.Context, rp *data.RefPair) (*types.Ref, error) {
ref, err := ds.H.Model.GetRefByNameAndSHA(rp.RepoName, rp.Sha)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}
return ref.ToProto(), nil
}
Expand All @@ -24,11 +25,11 @@ func (ds *DataServer) GetRefByNameAndSHA(ctx context.Context, rp *data.RefPair)
func (ds *DataServer) PutRef(ctx context.Context, ref *types.Ref) (*types.Ref, error) {
ret, err := model.NewRefFromProto(ref)
if err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

if err := ds.H.Model.PutRef(ret); err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return ret.ToProto(), nil
Expand All @@ -39,7 +40,7 @@ func (ds *DataServer) PutRef(ctx context.Context, ref *types.Ref) (*types.Ref, e
// cancel runs as new ones are being submitted.
func (ds *DataServer) CancelRefByName(ctx context.Context, rr *data.RepoRef) (*empty.Empty, error) {
if err := ds.H.Model.CancelRefByName(rr.Repository, rr.RefName, ds.H.URL, config.DefaultGithubClient()); err != nil {
return nil, err.ToGRPC(codes.FailedPrecondition)
return nil, err.(errors.Error).ToGRPC(codes.FailedPrecondition)
}

return &empty.Empty{}, nil
Expand Down
Loading

0 comments on commit 7edc5ca

Please sign in to comment.