diff options
| author | Sami Hiltunen <shiltunen@gitlab.com> | 2021-02-02 14:47:37 +0300 |
|---|---|---|
| committer | Sami Hiltunen <shiltunen@gitlab.com> | 2021-02-02 14:47:37 +0300 |
| commit | 626e9745fe18b571ad1f59c5a4f86e54df7f116e (patch) | |
| tree | 36c5c087ecf1a7434121957cf187832c9c8fe856 | |
| parent | f747aef48c410f5177e9e3c3446132870dc74cd7 (diff) | |
log errors in User{Create,Update,Delete}Branch RPCssmh-fix-branch-rpc-error-handling
UserCreateBranch, UserUpdateBranch and UserDeleteBranch do not log
errors they are dropping in favor of alternative error messages.
Some errors are also packed in to RPC responses which. This causes
the errors to look like successful responses which do not get logged
by Gitaly's gRPC logging interceptor. These behaviors can make
debugging difficult as it's not clear what was the original cause of
the error. This commit addresses the situation by logging the swallowed
errors.
| -rw-r--r-- | internal/gitaly/service/operations/branches.go | 5 |
1 files changed, 5 insertions, 0 deletions
diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go index 2bee1d805..9b3a19dc4 100644 --- a/internal/gitaly/service/operations/branches.go +++ b/internal/gitaly/service/operations/branches.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" + "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" @@ -49,6 +50,7 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB referenceName := fmt.Sprintf("refs/heads/%s", req.BranchName) _, err = localrepo.New(req.Repository, s.cfg).GetReference(ctx, git.ReferenceName(referenceName)) if err == nil { + ctxlogrus.Extract(ctx).Error("branch already exists") return nil, status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", req.BranchName) } else if !errors.Is(err, git.ErrReferenceNotFound) { return nil, status.Error(codes.Internal, err.Error()) @@ -57,6 +59,7 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, startPointCommit.Id, git.ZeroOID.String()); err != nil { var preReceiveError preReceiveError if errors.As(err, &preReceiveError) { + ctxlogrus.Extract(ctx).WithError(err).Error("failed to create branch") return &gitalypb.UserCreateBranchResponse{ PreReceiveError: preReceiveError.message, }, nil @@ -113,6 +116,7 @@ func (s *Server) userUpdateBranchGo(ctx context.Context, req *gitalypb.UserUpdat referenceName := fmt.Sprintf("refs/heads/%s", req.BranchName) if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, string(req.Newrev), string(req.Oldrev)); err != nil { + ctxlogrus.Extract(ctx).WithError(err).Error("failed to update branch") var preReceiveError preReceiveError if errors.As(err, &preReceiveError) { return &gitalypb.UserUpdateBranchResponse{ @@ -178,6 +182,7 @@ func (s *Server) UserDeleteBranch(ctx context.Context, req *gitalypb.UserDeleteB if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, referenceName, git.ZeroOID.String(), referenceValue.Target); err != nil { var preReceiveError preReceiveError if errors.As(err, &preReceiveError) { + ctxlogrus.Extract(ctx).WithError(err).Error("failed to delete branch") return &gitalypb.UserDeleteBranchResponse{ PreReceiveError: preReceiveError.message, }, nil |