diff --git a/gitrpc/enum/ref.go b/gitrpc/enum/ref.go index 08f49f865..f97258d6b 100644 --- a/gitrpc/enum/ref.go +++ b/gitrpc/enum/ref.go @@ -54,3 +54,22 @@ func RefToRPC(t RefType) rpc.RefType { return rpc.RefType_Undefined } } + +func (t RefType) String() string { + switch t { + case RefTypeRaw: + return "raw" + case RefTypeBranch: + return "branch" + case RefTypeTag: + return "tag" + case RefTypePullReqHead: + return "head" + case RefTypePullReqMerge: + return "merge" + case RefTypeUndefined: + fallthrough + default: + return "" + } +} diff --git a/gitrpc/errors.go b/gitrpc/errors.go index 442b88108..c21a93431 100644 --- a/gitrpc/errors.go +++ b/gitrpc/errors.go @@ -6,11 +6,157 @@ package gitrpc import ( "errors" + "fmt" + + "github.com/harness/gitness/gitrpc/rpc" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) -var ErrNoParamsProvided = errors.New("no params provided") -var ErrAlreadyExists = errors.New("already exists") -var ErrInvalidArgument = errors.New("invalid argument") -var ErrNotFound = errors.New("not found") -var ErrPreconditionFailed = errors.New("precondition failed") -var ErrNotMergeable = errors.New("not mergeable") +var ( + ErrNoParamsProvided = ErrInvalidArgumentf("params not provided") +) + +type Status string + +const ( + StatusConflict Status = "conflict" + StatusInternal Status = "internal" + StatusInvalidArgument Status = "invalid" + StatusNotFound Status = "not_found" + StatusNotImplemented Status = "not_implemented" + StatusUnauthorized Status = "unauthorized" + StatusFailed Status = "failed" + StatusPreconditionFailed Status = "precondition_failed" + StatusNotMergeable Status = "not_mergeable" + StatusAborted Status = "aborted" +) + +type Error struct { + // Machine-readable status code. + Status Status + + // Human-readable error message. + Message string + + // Details + Details map[string]any +} + +// Error implements the error interface. +func (e *Error) Error() string { + return e.Message +} + +// ErrorStatus unwraps an gitrpc error and returns its code. +// Non-application errors always return StatusInternal. +func ErrorStatus(err error) Status { + var ( + e *Error + ) + if err == nil { + return "" + } + if errors.As(err, &e) { + return e.Status + } + return StatusInternal +} + +// ErrorMessage unwraps an gitrpc error and returns its message. +// Non-gitrpc errors always return "Internal error". +func ErrorMessage(err error) string { + var ( + e *Error + ) + if err == nil { + return "" + } + if errors.As(err, &e) { + return e.Message + } + return "Internal error." +} + +// ErrorDetails unwraps an gitrpc error and returns its details. +// Non-gitrpc errors always return nil. +func ErrorDetails(err error) map[string]any { + var ( + e *Error + ) + if err == nil { + return nil + } + if errors.As(err, &e) { + return e.Details + } + return nil +} + +// NewError is a factory function to return an Error with a given status and message. +func NewError(code Status, message string) *Error { + return &Error{ + Status: code, + Message: message, + } +} + +// NewError is a factory function to return an Error with a given status, message and details. +func NewErrorWithDetails(code Status, message string, details map[string]any) *Error { + err := NewError(code, message) + err.Details = details + return err +} + +// Errorf is a helper function to return an Error with a given status and formatted message. +func Errorf(code Status, format string, args ...interface{}) *Error { + return &Error{ + Status: code, + Message: fmt.Sprintf(format, args...), + } +} + +// ErrInvalidArgumentf is a helper function to return an invalid argument Error. +func ErrInvalidArgumentf(format string, args ...interface{}) *Error { + return Errorf(StatusInvalidArgument, format, args...) +} + +func processRPCErrorf(err error, format string, args ...interface{}) error { + // create fallback error returned if we can't map it + fallbackErr := fmt.Errorf(format, args...) + + // ensure it's an rpc error + st, ok := status.FromError(err) + if !ok { + return fallbackErr + } + + msg := st.Message() + + switch { + case st.Code() == codes.AlreadyExists: + return NewError(StatusConflict, msg) + case st.Code() == codes.NotFound: + return NewError(StatusNotFound, msg) + case st.Code() == codes.InvalidArgument: + return NewError(StatusInvalidArgument, msg) + case st.Code() == codes.FailedPrecondition: + code := StatusPreconditionFailed + details := make(map[string]any) + for _, detail := range st.Details() { + switch t := detail.(type) { + case *rpc.MergeConflictError: + details["conflict_files"] = t.ConflictingFiles + code = StatusNotMergeable + default: + } + } + if len(details) > 0 { + return NewErrorWithDetails(code, msg, details) + } + return NewError(code, msg) + default: + return fallbackErr + } +} diff --git a/gitrpc/internal/gitea/merge.go b/gitrpc/internal/gitea/merge.go index 97dc4d9f7..e8943ecd8 100644 --- a/gitrpc/internal/gitea/merge.go +++ b/gitrpc/internal/gitea/merge.go @@ -206,6 +206,9 @@ func (g Adapter) Merge( // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error + if err = conflictFiles(ctx, pr, env, tmpBasePath, &outbuf); err != nil { + return err + } return &types.MergeConflictsError{ Method: mergeMethod, StdOut: outbuf.String(), @@ -228,6 +231,29 @@ func (g Adapter) Merge( return nil } +func conflictFiles(ctx context.Context, + pr *types.PullRequest, + env []string, + repoPath string, + buf *strings.Builder, +) error { + stdout, stderr, cferr := git.NewCommand( + ctx, "diff", "--name-only", "--diff-filter=U", "--relative", + ).RunStdString(&git.RunOpts{ + Env: env, + Dir: repoPath, + }) + if cferr != nil { + return processGiteaErrorf(cferr, "failed to list conflict files [%s -> %s], stderr: %v, err: %v", + pr.HeadBranch, pr.BaseBranch, stderr, cferr) + } + if len(stdout) > 0 { + buf.Reset() + buf.WriteString(stdout) + } + return nil +} + func (g Adapter) GetDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string) (string, error) { getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { var outbuf, errbuf strings.Builder diff --git a/gitrpc/internal/middleware/error.go b/gitrpc/internal/middleware/error.go index 166e46b51..2868ff87d 100644 --- a/gitrpc/internal/middleware/error.go +++ b/gitrpc/internal/middleware/error.go @@ -9,8 +9,7 @@ import ( "errors" "reflect" - "github.com/harness/gitness/gitrpc/internal/types" - + "github.com/rs/zerolog/log" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -30,7 +29,7 @@ func (i ErrInterceptor) UnaryInterceptor() grpc.UnaryServerInterceptor { if (value == nil || reflect.ValueOf(value).IsNil()) && err == nil { return nil, status.Error(codes.Internal, "service returned no error and no object") } - err = processError(err) + err = processError(ctx, err) return value, err } } @@ -39,44 +38,56 @@ func (i ErrInterceptor) StreamInterceptor() grpc.StreamServerInterceptor { return func(srv interface{}, stream grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { err := handler(srv, stream) - err = processError(err) + err = processError(stream.Context(), err) return err } } -func processError(err error) error { +func processError(ctx context.Context, err error) (rerr error) { if err == nil { return nil } - // check if error already is grpc error - // TODO: this should be removed once all error handling has been refactored. + defer func() { + statusErr, ok := status.FromError(rerr) + if !ok { + return + } + //nolint: exhaustive // log only server side errors, no need to log user based errors + switch statusErr.Code() { + case codes.Unknown, + codes.DeadlineExceeded, + codes.ResourceExhausted, + codes.FailedPrecondition, + codes.Aborted, + codes.OutOfRange, + codes.Unimplemented, + codes.Internal, + codes.Unavailable, + codes.DataLoss: + { + logCtx := log.Ctx(ctx) + logCtx.Error().Msg(err.Error()) + } + } + }() + + // custom errors should implement StatusError + var statusError interface { + Status() (*status.Status, error) + } + + if errors.As(err, &statusError) { + st, sterr := statusError.Status() + if sterr != nil { + return sterr + } + return st.Err() + } + if status, ok := status.FromError(err); ok { return status.Err() } - message := err.Error() - - switch { - case errors.Is(err, context.DeadlineExceeded): - return status.Error(codes.DeadlineExceeded, message) - case errors.Is(err, types.ErrNotFound): - return status.Error(codes.NotFound, message) - case errors.Is(err, types.ErrAlreadyExists): - return status.Error(codes.AlreadyExists, message) - case errors.Is(err, types.ErrInvalidArgument): - return status.Error(codes.InvalidArgument, message) - case errors.Is(err, types.ErrInvalidPath): - return status.Error(codes.InvalidArgument, message) - case errors.Is(err, types.ErrUndefinedAction): - return status.Error(codes.InvalidArgument, message) - case errors.Is(err, types.ErrHeaderCannotBeEmpty): - return status.Error(codes.InvalidArgument, message) - case errors.Is(err, types.ErrActionListEmpty): - return status.Error(codes.FailedPrecondition, message) - case errors.Is(err, types.ErrContentSentBeforeAction): - return status.Error(codes.FailedPrecondition, message) - default: - return status.Errorf(codes.Unknown, message) - } + return status.Errorf(codes.Unknown, err.Error()) } diff --git a/gitrpc/internal/service/errors.go b/gitrpc/internal/service/errors.go new file mode 100644 index 000000000..9de02b112 --- /dev/null +++ b/gitrpc/internal/service/errors.go @@ -0,0 +1,215 @@ +// Copyright 2022 Harness Inc. All rights reserved. +// Use of this source code is governed by the Polyform Free Trial License +// that can be found in the LICENSE.md file for this repository. + +package service + +import ( + "errors" + "fmt" + "strings" + + "github.com/harness/gitness/gitrpc/internal/types" + "github.com/harness/gitness/gitrpc/rpc" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/anypb" +) + +type Error struct { + Code codes.Code + Message string + Err error + details []proto.Message +} + +func (e *Error) Error() string { + return fmt.Sprintf("%s, err: %v", e.Message, e.Err.Error()) +} + +func (e *Error) Status() (*status.Status, error) { + st := status.New(e.Code, e.Message) + if len(e.details) == 0 { + return st, nil + } + // add details + proto := st.Proto() + for _, detail := range e.details { + marshaled, err := anypb.New(detail) + if err != nil { + return nil, err + } + + proto.Details = append(proto.Details, marshaled) + } + return status.FromProto(proto), nil +} + +func (e *Error) Details() any { + return e.details +} + +func (e *Error) Unwrap() error { + return e.Err +} + +// Errorf generates new Error with status code and custom arguments. +// args can contain format args and additional arg like err which will be logged +// by middleware and details object type of map. Ordering of args element +// should first process format args and then error or detail. +func Errorf(code codes.Code, format string, args ...any) (err error) { + details := make([]proto.Message, 0, 8) + newargs := make([]any, 0, len(args)) + + for _, arg := range args { + switch t := arg.(type) { + case error: + err = t + case proto.Message: + details = append(details, t) + default: + newargs = append(newargs, arg) + } + } + + return &Error{ + Code: code, + Message: fmt.Sprintf(format, newargs...), + Err: err, + details: details, + } +} + +func wrapError(code codes.Code, err error) error { + var e *Error + if errors.As(err, &e) { + return err + } + return &Error{ + Code: code, + Message: err.Error(), + Err: err, + } +} + +// ErrCanceled wraps err with codes.Canceled, unless err is already a Error error. +func ErrCanceled(err error) error { return wrapError(codes.Canceled, err) } + +// ErrDeadlineExceeded wraps err with codes.DeadlineExceeded, unless err is already a Error error. +func ErrDeadlineExceeded(err error) error { return wrapError(codes.DeadlineExceeded, err) } + +// ErrInternal wraps err with codes.Internal, unless err is already a Error error. +func ErrInternal(err error) error { return wrapError(codes.Internal, err) } + +// ErrInvalidArgument wraps err with codes.InvalidArgument, unless err is already a Error error. +func ErrInvalidArgument(err error) error { return wrapError(codes.InvalidArgument, err) } + +// ErrNotFound wraps error with codes.NotFound, unless err is already a Error error. +func ErrNotFound(err error) error { return wrapError(codes.NotFound, err) } + +// ErrFailedPrecondition wraps err with codes.FailedPrecondition, unless err is already a Error +// error. +func ErrFailedPrecondition(err error) error { return wrapError(codes.FailedPrecondition, err) } + +// ErrUnavailable wraps err with codes.Unavailable, unless err is already a gRPC error. +func ErrUnavailable(err error) error { return wrapError(codes.Unavailable, err) } + +// ErrPermissionDenied wraps err with codes.PermissionDenied, unless err is already a Error error. +func ErrPermissionDenied(err error) error { return wrapError(codes.PermissionDenied, err) } + +// ErrAlreadyExists wraps err with codes.AlreadyExists, unless err is already a Error error. +func ErrAlreadyExists(err error) error { return wrapError(codes.AlreadyExists, err) } + +// ErrAborted wraps err with codes.Aborted, unless err is already a Error type. +func ErrAborted(err error) error { return wrapError(codes.Aborted, err) } + +// ErrCanceledf wraps a formatted error with codes.Canceled, unless the formatted error is a +// wrapped Error error. +func ErrCanceledf(format string, a ...interface{}) error { + return Errorf(codes.Canceled, format, a...) +} + +// ErrDeadlineExceededf wraps a formatted error with codes.DeadlineExceeded, unless the formatted +// error is a wrapped Error error. +func ErrDeadlineExceededf(format string, a ...interface{}) error { + return Errorf(codes.DeadlineExceeded, format, a...) +} + +// ErrInternalf wraps a formatted error with codes.Internal, unless the formatted error is a +// wrapped Error error. +func ErrInternalf(format string, a ...interface{}) error { + return Errorf(codes.Internal, format, a...) +} + +// ErrInvalidArgumentf wraps a formatted error with codes.InvalidArgument, unless the formatted +// error is a wrapped Error error. +func ErrInvalidArgumentf(format string, a ...interface{}) error { + return Errorf(codes.InvalidArgument, format, a...) +} + +// ErrNotFoundf wraps a formatted error with codes.NotFound, unless the +// formatted error is a wrapped Error error. +func ErrNotFoundf(format string, a ...interface{}) error { + return Errorf(codes.NotFound, format, a...) +} + +// ErrFailedPreconditionf wraps a formatted error with codes.FailedPrecondition, unless the +// formatted error is a wrapped Error error. +func ErrFailedPreconditionf(format string, a ...interface{}) error { + return Errorf(codes.FailedPrecondition, format, a...) +} + +// ErrUnavailablef wraps a formatted error with codes.Unavailable, unless the +// formatted error is a wrapped Error error. +func ErrUnavailablef(format string, a ...interface{}) error { + return Errorf(codes.Unavailable, format, a...) +} + +// ErrPermissionDeniedf wraps a formatted error with codes.PermissionDenied, unless the formatted +// error is a wrapped Error error. +func ErrPermissionDeniedf(format string, a ...interface{}) error { + return Errorf(codes.PermissionDenied, format, a...) +} + +// ErrAlreadyExistsf wraps a formatted error with codes.AlreadyExists, unless the formatted error is +// a wrapped Error error. +func ErrAlreadyExistsf(format string, a ...interface{}) error { + return Errorf(codes.AlreadyExists, format, a...) +} + +// ErrAbortedf wraps a formatted error with codes.Aborted, unless the formatted error is a wrapped +// Error error. +func ErrAbortedf(format string, a ...interface{}) error { + return Errorf(codes.Aborted, format, a...) +} + +// processGitErrorf translates error. +func processGitErrorf(err error, format string, args ...interface{}) error { + var ( + cferr *types.MergeConflictsError + ) + const nl = "\n" + // when we add err as argument it will be part of the new error + args = append(args, err) + switch { + case errors.Is(err, types.ErrNotFound): + return ErrNotFoundf(format, args...) + case errors.Is(err, types.ErrAlreadyExists): + return ErrAlreadyExistsf(format, args...) + case errors.Is(err, types.ErrInvalidArgument): + return ErrInvalidArgumentf(format, args...) + case errors.As(err, &cferr): + stdout := strings.Trim(cferr.StdOut, nl) + conflictingFiles := strings.Split(stdout, nl) + files := &rpc.MergeConflictError{ + ConflictingFiles: conflictingFiles, + } + return ErrFailedPreconditionf("merging failed! conflict files error", files, err) + case types.IsMergeUnrelatedHistoriesError(err): + return ErrFailedPreconditionf(format, args...) + default: + return Errorf(codes.Unknown, format, args...) + } +} diff --git a/gitrpc/internal/service/mapping.go b/gitrpc/internal/service/mapping.go index 8dad0b96e..5accfb0df 100644 --- a/gitrpc/internal/service/mapping.go +++ b/gitrpc/internal/service/mapping.go @@ -5,42 +5,13 @@ package service import ( - "errors" - "fmt" - "github.com/harness/gitness/gitrpc/internal/types" "github.com/harness/gitness/gitrpc/rpc" - "github.com/rs/zerolog/log" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) -// Logs the error and message, returns either the provided message or a rpc equivalent if possible. -// Always logs the full message with error as warning. -func processGitErrorf(err error, format string, args ...interface{}) error { - // create fallback error returned if we can't map it - message := fmt.Sprintf(format, args...) - - // always log internal error together with message. - log.Warn().Msgf("%s: [GIT] %v", message, err) - - switch { - case errors.Is(err, types.ErrNotFound): - return status.Error(codes.NotFound, message) - case errors.Is(err, types.ErrAlreadyExists): - return status.Errorf(codes.AlreadyExists, message) - case errors.Is(err, types.ErrInvalidArgument): - return status.Errorf(codes.InvalidArgument, message) - case types.IsMergeConflictsError(err): - return status.Errorf(codes.Aborted, message) - case types.IsMergeUnrelatedHistoriesError(err): - return status.Errorf(codes.Aborted, message) - default: - return status.Errorf(codes.Unknown, message) - } -} - func mapSortOrder(s rpc.SortOrder) types.SortOrder { switch s { case rpc.SortOrder_Asc: diff --git a/gitrpc/internal/service/tree.go b/gitrpc/internal/service/tree.go index 2fa1a9078..f085babf4 100644 --- a/gitrpc/internal/service/tree.go +++ b/gitrpc/internal/service/tree.go @@ -71,7 +71,7 @@ func (s RepositoryService) GetTreeNode(ctx context.Context, // TODO: do we need to validate request for nil? gitNode, err := s.adapter.GetTreeNode(ctx, repoPath, request.GetGitRef(), request.GetPath()) if err != nil { - return nil, processGitErrorf(err, "failed to get tree node") + return nil, processGitErrorf(err, "failed to get tree node %s", request.Path) } res := &rpc.GetTreeNodeResponse{ diff --git a/gitrpc/mapping.go b/gitrpc/mapping.go index 2b471deaf..ce8fa99c7 100644 --- a/gitrpc/mapping.go +++ b/gitrpc/mapping.go @@ -9,44 +9,8 @@ import ( "time" "github.com/harness/gitness/gitrpc/rpc" - - "github.com/rs/zerolog/log" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) -// Logs the error and message, returns either the provided message or a gitrpc equivalent if possible. -// Always logs the full message with error as warning. -func processRPCErrorf(err error, format string, args ...interface{}) error { - // create fallback error returned if we can't map it - fallbackErr := fmt.Errorf(format, args...) - - // always log internal error together with message. - log.Warn().Msgf("%v: [RPC] %v", fallbackErr, err) - - // ensure it's an rpc error - rpcErr, ok := status.FromError(err) - if !ok { - return fallbackErr - } - - switch { - case rpcErr.Code() == codes.AlreadyExists: - return ErrAlreadyExists - case rpcErr.Code() == codes.NotFound: - return ErrNotFound - case rpcErr.Code() == codes.InvalidArgument: - return ErrInvalidArgument - case rpcErr.Code() == codes.FailedPrecondition: - return ErrPreconditionFailed - case rpcErr.Code() == codes.Aborted: - // TODO: this should not be so generic ... - return ErrNotMergeable - default: - return fallbackErr - } -} - func mapToRPCSortOrder(o SortOrder) rpc.SortOrder { switch o { case SortOrderAsc: diff --git a/gitrpc/proto/merge.proto b/gitrpc/proto/merge.proto index 792198757..435c229d3 100644 --- a/gitrpc/proto/merge.proto +++ b/gitrpc/proto/merge.proto @@ -54,4 +54,11 @@ message MergeResponse { string merge_base_sha = 3; // merge_sha is the sha of the commit after merging head_sha with base_sha. string merge_sha = 4; +} + +// MergeConflictError is an error returned in the case when merging two commits +// fails due to a merge conflict. +message MergeConflictError { + // ConflictingFiles is the set of files which have been conflicting. + repeated string conflicting_files = 1; } \ No newline at end of file diff --git a/gitrpc/ref.go b/gitrpc/ref.go index b981e1a63..b9bbcb9df 100644 --- a/gitrpc/ref.go +++ b/gitrpc/ref.go @@ -9,9 +9,6 @@ import ( "github.com/harness/gitness/gitrpc/enum" "github.com/harness/gitness/gitrpc/rpc" - - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) type GetRefParams struct { @@ -27,7 +24,7 @@ type GetRefResponse struct { func (c *Client) GetRef(ctx context.Context, params GetRefParams) (GetRefResponse, error) { refType := enum.RefToRPC(params.Type) if refType == rpc.RefType_Undefined { - return GetRefResponse{}, ErrInvalidArgument + return GetRefResponse{}, ErrInvalidArgumentf("invalid argument: '%s'", refType) } result, err := c.refService.GetRef(ctx, &rpc.GetRefRequest{ @@ -35,8 +32,8 @@ func (c *Client) GetRef(ctx context.Context, params GetRefParams) (GetRefRespons RefName: params.Name, RefType: refType, }) - if s, ok := status.FromError(err); err != nil && ok && s.Code() == codes.NotFound { - return GetRefResponse{}, ErrNotFound + if err != nil { + return GetRefResponse{}, processRPCErrorf(err, "failed to get %s ref '%s'", params.Type.String(), params.Name) } return GetRefResponse{SHA: result.Sha}, nil @@ -57,7 +54,7 @@ type UpdateRefParams struct { func (c *Client) UpdateRef(ctx context.Context, params UpdateRefParams) error { refType := enum.RefToRPC(params.Type) if refType == rpc.RefType_Undefined { - return ErrInvalidArgument + return ErrInvalidArgumentf("invalid argument: '%s'", refType) } _, err := c.refService.UpdateRef(ctx, &rpc.UpdateRefRequest{ @@ -67,8 +64,8 @@ func (c *Client) UpdateRef(ctx context.Context, params UpdateRefParams) error { NewValue: params.NewValue, OldValue: params.OldValue, }) - if s, ok := status.FromError(err); err != nil && ok && s.Code() == codes.NotFound { - return ErrNotFound + if err != nil { + return processRPCErrorf(err, "failed to update %s ref '%s'", params.Type.String(), params.Name) } return err diff --git a/gitrpc/rpc/http.pb.go b/gitrpc/rpc/http.pb.go index 9afe821bf..7006fe359 100644 --- a/gitrpc/rpc/http.pb.go +++ b/gitrpc/rpc/http.pb.go @@ -151,6 +151,7 @@ type ServicePackRequest struct { // Depending on the service the matching base type has to be passed // // Types that are assignable to Base: + // // *ServicePackRequest_ReadBase // *ServicePackRequest_WriteBase Base isServicePackRequest_Base `protobuf_oneof:"base"` diff --git a/gitrpc/rpc/merge.pb.go b/gitrpc/rpc/merge.pb.go index cda62b44e..4ed5134e5 100644 --- a/gitrpc/rpc/merge.pb.go +++ b/gitrpc/rpc/merge.pb.go @@ -245,6 +245,56 @@ func (x *MergeResponse) GetMergeSha() string { return "" } +// MergeConflictError is an error returned in the case when merging two commits +// fails due to a merge conflict. +type MergeConflictError struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + // ConflictingFiles is the set of files which have been conflicting. + ConflictingFiles []string `protobuf:"bytes,1,rep,name=conflicting_files,json=conflictingFiles,proto3" json:"conflicting_files,omitempty"` +} + +func (x *MergeConflictError) Reset() { + *x = MergeConflictError{} + if protoimpl.UnsafeEnabled { + mi := &file_merge_proto_msgTypes[2] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *MergeConflictError) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MergeConflictError) ProtoMessage() {} + +func (x *MergeConflictError) ProtoReflect() protoreflect.Message { + mi := &file_merge_proto_msgTypes[2] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MergeConflictError.ProtoReflect.Descriptor instead. +func (*MergeConflictError) Descriptor() ([]byte, []int) { + return file_merge_proto_rawDescGZIP(), []int{2} +} + +func (x *MergeConflictError) GetConflictingFiles() []string { + if x != nil { + return x.ConflictingFiles + } + return nil +} + var File_merge_proto protoreflect.FileDescriptor var file_merge_proto_rawDesc = []byte{ @@ -285,14 +335,19 @@ var file_merge_proto_rawDesc = []byte{ 0x72, 0x67, 0x65, 0x5f, 0x62, 0x61, 0x73, 0x65, 0x5f, 0x73, 0x68, 0x61, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0c, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x61, 0x73, 0x65, 0x53, 0x68, 0x61, 0x12, 0x1b, 0x0a, 0x09, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x5f, 0x73, 0x68, 0x61, 0x18, 0x04, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x08, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x53, 0x68, 0x61, 0x32, 0x40, 0x0a, - 0x0c, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x30, 0x0a, - 0x05, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x12, 0x11, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72, - 0x67, 0x65, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x12, 0x2e, 0x72, 0x70, 0x63, 0x2e, - 0x4d, 0x65, 0x72, 0x67, 0x65, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x42, - 0x27, 0x5a, 0x25, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, - 0x72, 0x6e, 0x65, 0x73, 0x73, 0x2f, 0x67, 0x69, 0x74, 0x6e, 0x65, 0x73, 0x73, 0x2f, 0x67, 0x69, - 0x74, 0x72, 0x70, 0x63, 0x2f, 0x72, 0x70, 0x63, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x01, 0x28, 0x09, 0x52, 0x08, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x53, 0x68, 0x61, 0x22, 0x41, 0x0a, + 0x12, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x43, 0x6f, 0x6e, 0x66, 0x6c, 0x69, 0x63, 0x74, 0x45, 0x72, + 0x72, 0x6f, 0x72, 0x12, 0x2b, 0x0a, 0x11, 0x63, 0x6f, 0x6e, 0x66, 0x6c, 0x69, 0x63, 0x74, 0x69, + 0x6e, 0x67, 0x5f, 0x66, 0x69, 0x6c, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x09, 0x52, 0x10, + 0x63, 0x6f, 0x6e, 0x66, 0x6c, 0x69, 0x63, 0x74, 0x69, 0x6e, 0x67, 0x46, 0x69, 0x6c, 0x65, 0x73, + 0x32, 0x40, 0x0a, 0x0c, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, + 0x12, 0x30, 0x0a, 0x05, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x12, 0x11, 0x2e, 0x72, 0x70, 0x63, 0x2e, + 0x4d, 0x65, 0x72, 0x67, 0x65, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x12, 0x2e, 0x72, + 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, + 0x22, 0x00, 0x42, 0x27, 0x5a, 0x25, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, + 0x2f, 0x68, 0x61, 0x72, 0x6e, 0x65, 0x73, 0x73, 0x2f, 0x67, 0x69, 0x74, 0x6e, 0x65, 0x73, 0x73, + 0x2f, 0x67, 0x69, 0x74, 0x72, 0x70, 0x63, 0x2f, 0x72, 0x70, 0x63, 0x62, 0x06, 0x70, 0x72, 0x6f, + 0x74, 0x6f, 0x33, } var ( @@ -307,19 +362,20 @@ func file_merge_proto_rawDescGZIP() []byte { return file_merge_proto_rawDescData } -var file_merge_proto_msgTypes = make([]protoimpl.MessageInfo, 2) +var file_merge_proto_msgTypes = make([]protoimpl.MessageInfo, 3) var file_merge_proto_goTypes = []interface{}{ - (*MergeRequest)(nil), // 0: rpc.MergeRequest - (*MergeResponse)(nil), // 1: rpc.MergeResponse - (*WriteRequest)(nil), // 2: rpc.WriteRequest - (*Identity)(nil), // 3: rpc.Identity - (RefType)(0), // 4: rpc.RefType + (*MergeRequest)(nil), // 0: rpc.MergeRequest + (*MergeResponse)(nil), // 1: rpc.MergeResponse + (*MergeConflictError)(nil), // 2: rpc.MergeConflictError + (*WriteRequest)(nil), // 3: rpc.WriteRequest + (*Identity)(nil), // 4: rpc.Identity + (RefType)(0), // 5: rpc.RefType } var file_merge_proto_depIdxs = []int32{ - 2, // 0: rpc.MergeRequest.base:type_name -> rpc.WriteRequest - 3, // 1: rpc.MergeRequest.author:type_name -> rpc.Identity - 3, // 2: rpc.MergeRequest.committer:type_name -> rpc.Identity - 4, // 3: rpc.MergeRequest.ref_type:type_name -> rpc.RefType + 3, // 0: rpc.MergeRequest.base:type_name -> rpc.WriteRequest + 4, // 1: rpc.MergeRequest.author:type_name -> rpc.Identity + 4, // 2: rpc.MergeRequest.committer:type_name -> rpc.Identity + 5, // 3: rpc.MergeRequest.ref_type:type_name -> rpc.RefType 0, // 4: rpc.MergeService.Merge:input_type -> rpc.MergeRequest 1, // 5: rpc.MergeService.Merge:output_type -> rpc.MergeResponse 5, // [5:6] is the sub-list for method output_type @@ -360,6 +416,18 @@ func file_merge_proto_init() { return nil } } + file_merge_proto_msgTypes[2].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*MergeConflictError); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } } type x struct{} out := protoimpl.TypeBuilder{ @@ -367,7 +435,7 @@ func file_merge_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: file_merge_proto_rawDesc, NumEnums: 0, - NumMessages: 2, + NumMessages: 3, NumExtensions: 0, NumServices: 1, }, diff --git a/gitrpc/rpc/operations.pb.go b/gitrpc/rpc/operations.pb.go index 517aeef89..a06443219 100644 --- a/gitrpc/rpc/operations.pb.go +++ b/gitrpc/rpc/operations.pb.go @@ -247,6 +247,7 @@ type CommitFilesAction struct { unknownFields protoimpl.UnknownFields // Types that are assignable to Payload: + // // *CommitFilesAction_Header // *CommitFilesAction_Content Payload isCommitFilesAction_Payload `protobuf_oneof:"payload"` @@ -330,6 +331,7 @@ type CommitFilesRequest struct { unknownFields protoimpl.UnknownFields // Types that are assignable to Payload: + // // *CommitFilesRequest_Header // *CommitFilesRequest_Action Payload isCommitFilesRequest_Payload `protobuf_oneof:"payload"` diff --git a/gitrpc/rpc/repo.pb.go b/gitrpc/rpc/repo.pb.go index fb0ce39f2..8fae9e594 100644 --- a/gitrpc/rpc/repo.pb.go +++ b/gitrpc/rpc/repo.pb.go @@ -130,6 +130,7 @@ type CreateRepositoryRequest struct { unknownFields protoimpl.UnknownFields // Types that are assignable to Data: + // // *CreateRepositoryRequest_Header // *CreateRepositoryRequest_File Data isCreateRepositoryRequest_Data `protobuf_oneof:"data"` diff --git a/gitrpc/rpc/shared.pb.go b/gitrpc/rpc/shared.pb.go index 084d3dfb6..65817ec5b 100644 --- a/gitrpc/rpc/shared.pb.go +++ b/gitrpc/rpc/shared.pb.go @@ -298,6 +298,7 @@ type FileUpload struct { unknownFields protoimpl.UnknownFields // Types that are assignable to Data: + // // *FileUpload_Header // *FileUpload_Chunk Data isFileUpload_Data `protobuf_oneof:"data"` diff --git a/internal/api/controller/pullreq/controller.go b/internal/api/controller/pullreq/controller.go index f47213c8e..93beef690 100644 --- a/internal/api/controller/pullreq/controller.go +++ b/internal/api/controller/pullreq/controller.go @@ -6,7 +6,6 @@ package pullreq import ( "context" - "errors" "fmt" "github.com/harness/gitness/gitrpc" @@ -83,7 +82,7 @@ func (c *Controller) verifyBranchExistence(ctx context.Context, Name: branch, Type: gitrpcenum.RefTypeBranch, }) - if errors.Is(err, gitrpc.ErrNotFound) { + if gitrpc.ErrorStatus(err) == gitrpc.StatusNotFound { return "", usererror.BadRequest( fmt.Sprintf("branch %s does not exist in the repository %s", branch, repo.UID)) } diff --git a/internal/api/controller/repo/merge_check.go b/internal/api/controller/repo/merge_check.go index caebbcea7..0d7d264f7 100644 --- a/internal/api/controller/repo/merge_check.go +++ b/internal/api/controller/repo/merge_check.go @@ -6,7 +6,6 @@ package repo import ( "context" - "errors" "fmt" "github.com/harness/gitness/gitrpc" @@ -24,24 +23,24 @@ func (c *Controller) MergeCheck( session *auth.Session, repoRef string, diffPath string, -) (MergeCheck, error) { +) error { repo, err := c.repoStore.FindByRef(ctx, repoRef) if err != nil { - return MergeCheck{}, err + return err } if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, enum.PermissionRepoView, false); err != nil { - return MergeCheck{}, fmt.Errorf("access check failed: %w", err) + return fmt.Errorf("access check failed: %w", err) } info, err := parseDiffPath(diffPath) if err != nil { - return MergeCheck{}, err + return err } writeParams, err := CreateRPCWriteParams(ctx, c.urlProvider, session, repo) if err != nil { - return MergeCheck{}, fmt.Errorf("failed to create rpc write params: %w", err) + return fmt.Errorf("failed to create rpc write params: %w", err) } _, err = c.gitRPCClient.Merge(ctx, &gitrpc.MergeParams{ @@ -50,16 +49,9 @@ func (c *Controller) MergeCheck( HeadRepoUID: writeParams.RepoUID, // forks are not supported for now HeadBranch: info.HeadRef, }) - if errors.Is(err, gitrpc.ErrNotMergeable) { - return MergeCheck{ - Mergeable: false, - }, nil - } if err != nil { - return MergeCheck{}, fmt.Errorf("merge execution failed: %w", err) + return fmt.Errorf("merge execution failed: %w", err) } - return MergeCheck{ - Mergeable: true, - }, nil + return nil } diff --git a/internal/api/handler/repo/merge_check.go b/internal/api/handler/repo/merge_check.go index eb8112917..1e2e9b5ee 100644 --- a/internal/api/handler/repo/merge_check.go +++ b/internal/api/handler/repo/merge_check.go @@ -25,12 +25,12 @@ func HandleMergeCheck(repoCtrl *repo.Controller) http.HandlerFunc { path := request.GetOptionalRemainderFromPath(r) - output, err := repoCtrl.MergeCheck(ctx, session, repoRef, path) + err = repoCtrl.MergeCheck(ctx, session, repoRef, path) if err != nil { render.TranslatedUserError(w, err) return } - render.JSON(w, http.StatusOK, output) + w.WriteHeader(http.StatusNoContent) } } diff --git a/internal/api/openapi/repo.go b/internal/api/openapi/repo.go index e96575b2e..ff6e23c77 100644 --- a/internal/api/openapi/repo.go +++ b/internal/api/openapi/repo.go @@ -550,7 +550,8 @@ func repoOperations(reflector *openapi3.Reflector) { opMergeCheck.WithTags("repository") opMergeCheck.WithMapOfAnything(map[string]interface{}{"operationId": "mergeCheck"}) _ = reflector.SetRequest(&opMergeCheck, new(getRawDiffRequest), http.MethodPost) - _ = reflector.SetJSONResponse(&opMergeCheck, new(repo.MergeCheck), http.StatusOK) + _ = reflector.SetJSONResponse(&opMergeCheck, nil, http.StatusNoContent) + _ = reflector.SetJSONResponse(&opMergeCheck, new(usererror.Error), http.StatusPreconditionFailed) _ = reflector.SetJSONResponse(&opMergeCheck, new(usererror.Error), http.StatusInternalServerError) _ = reflector.SetJSONResponse(&opMergeCheck, new(usererror.Error), http.StatusUnauthorized) _ = reflector.SetJSONResponse(&opMergeCheck, new(usererror.Error), http.StatusForbidden) diff --git a/internal/api/usererror/translate.go b/internal/api/usererror/translate.go index d99df99cb..d0e228d50 100644 --- a/internal/api/usererror/translate.go +++ b/internal/api/usererror/translate.go @@ -18,8 +18,12 @@ import ( ) func Translate(err error) *Error { - var rError *Error - var checkError *check.ValidationError + var ( + rError *Error + checkError *check.ValidationError + gitrpcError *gitrpc.Error + ) + switch { // api errors case errors.As(err, &rError): @@ -52,16 +56,12 @@ func Translate(err error) *Error { return ErrSpaceWithChildsCantBeDeleted // gitrpc errors - case errors.Is(err, gitrpc.ErrAlreadyExists): - return ErrDuplicate - case errors.Is(err, gitrpc.ErrInvalidArgument): - return ErrBadRequest - case errors.Is(err, gitrpc.ErrNotFound): - return ErrNotFound - case errors.Is(err, gitrpc.ErrPreconditionFailed): - return ErrPreconditionFailed - case errors.Is(err, gitrpc.ErrNotMergeable): - return ErrNotMergeable + case errors.As(err, &gitrpcError): + return NewWithPayload(httpStatusCode( + gitrpcError.Status), + gitrpcError.Message, + gitrpcError.Details, + ) // webhook errors case errors.Is(err, webhook.ErrWebhookNotRetriggerable): @@ -73,3 +73,23 @@ func Translate(err error) *Error { return ErrInternal } } + +// lookup of gitrpc error codes to HTTP status codes. +var codes = map[gitrpc.Status]int{ + gitrpc.StatusConflict: http.StatusConflict, + gitrpc.StatusInvalidArgument: http.StatusBadRequest, + gitrpc.StatusNotFound: http.StatusNotFound, + gitrpc.StatusNotImplemented: http.StatusNotImplemented, + gitrpc.StatusPreconditionFailed: http.StatusPreconditionFailed, + gitrpc.StatusUnauthorized: http.StatusUnauthorized, + gitrpc.StatusInternal: http.StatusInternalServerError, + gitrpc.StatusNotMergeable: http.StatusPreconditionFailed, +} + +// httpStatusCode returns the associated HTTP status code for a gitrpc error code. +func httpStatusCode(code gitrpc.Status) int { + if v, ok := codes[code]; ok { + return v + } + return http.StatusInternalServerError +} diff --git a/internal/services/pullreq/handlers_mergeable.go b/internal/services/pullreq/handlers_mergeable.go index 581ea8b50..10e1d9976 100644 --- a/internal/services/pullreq/handlers_mergeable.go +++ b/internal/services/pullreq/handlers_mergeable.go @@ -6,7 +6,6 @@ package pullreq import ( "context" - "errors" "fmt" "strconv" @@ -206,12 +205,12 @@ func (s *Service) updateMergeData( HeadExpectedSHA: newSHA, Force: true, }) - if errors.Is(err, gitrpc.ErrPreconditionFailed) { + if gitrpc.ErrorStatus(err) == gitrpc.StatusPreconditionFailed { return events.NewDiscardEventErrorf("Source branch '%s' is not on SHA '%s' anymore.", pr.SourceBranch, newSHA) } - isNotMergeableError := errors.Is(err, gitrpc.ErrNotMergeable) + isNotMergeableError := gitrpc.ErrorStatus(err) == gitrpc.StatusNotMergeable if err != nil && !isNotMergeableError { return fmt.Errorf("merge check failed for %s and %s with err: %w", targetRepo.UID+":"+pr.TargetBranch, sourceRepo.UID+":"+pr.SourceBranch, err) diff --git a/internal/services/webhook/branch.go b/internal/services/webhook/branch.go index f73035280..4431b4bd0 100644 --- a/internal/services/webhook/branch.go +++ b/internal/services/webhook/branch.go @@ -6,7 +6,6 @@ package webhook import ( "context" - "errors" "fmt" "github.com/harness/gitness/events" @@ -140,7 +139,7 @@ func (s *Service) fetchCommitInfoForEvent(ctx context.Context, repoUID string, s SHA: sha, }) - if errors.Is(err, gitrpc.ErrNotFound) { + if gitrpc.ErrorStatus(err) == gitrpc.StatusNotFound { // this could happen if the commit has been deleted and garbage collected by now // or if the sha doesn't point to an event - either way discard the event. return CommitInfo{}, events.NewDiscardEventErrorf("commit with sha '%s' doesn't exist", sha)