USEFULL
/
drone
mirror of https://github.com/harness/drone.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[fix] internal error should be always logged in error level (#982)

eb/code-1016-2
Enver Bisevac 2024-01-24 12:28:01 +00:00 committed by Harness
parent 6938ed79e8
commit f39852dc62
15 changed files with 57 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/api/controller/repo/commit.go
View File

@ -117,7 +117,7 @@ func (c *Controller) CommitFiles(ctx context.Context,
case enum.ContentEncodingTypeBase64:
rawPayload, err = base64.StdEncoding.DecodeString(action.Payload)
if err != nil {
return types.CommitFilesResponse{}, nil, errors.InvalidArgument("failed to decode base64 payload", err)
return types.CommitFilesResponse{}, nil, errors.Internal(err, "failed to decode base64 payload")
}
case enum.ContentEncodingTypeUTF8:
fallthrough

2
app/api/controller/repo/content_get.go
View File

@ -303,6 +303,6 @@ func mapNodeModeToContentType(m git.TreeNodeMode) (ContentType, error) {
case git.TreeNodeModeTree:
return ContentTypeDir, nil
default:
return ContentTypeFile, errors.Internal("unsupported tree node mode '%s'", m)
return ContentTypeFile, errors.Internal(nil, "unsupported tree node mode '%s'", m)
}
}

10
app/api/render/render.go
View File

@ -17,7 +17,6 @@ package render
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@ -25,6 +24,7 @@ import (
"strconv"
"github.com/harness/gitness/app/api/usererror"
"github.com/harness/gitness/errors"
"github.com/harness/gitness/types"
"github.com/rs/zerolog/log"
@ -41,7 +41,13 @@ func init() {
// TranslatedUserError writes the translated user error of the provided error.
func TranslatedUserError(w http.ResponseWriter, err error) {
log.Warn().Msgf("operation resulted in user facing error. Internal details: %s", err)
statusError := errors.AsError(err)
format := "operation resulted in user facing error. Internal details: %s"
if statusError != nil && statusError.Status == errors.StatusInternal {
log.Error().Msgf(format, statusError.Err)
} else {
log.Warn().Msgf(format, err)
}
UserError(w, usererror.Translate(err))
}

62
errors/status.go
View File

@ -34,19 +34,29 @@ const (
)
type Error struct {
// Source error
Err error
// Machine-readable status code.
Status Status
// Human-readable error message.
Message string
// Source error
Err error
// Details
Details map[string]any
}
func (e *Error) SetErr(err error) *Error {
e.Err = err
return e
}
func (e *Error) SetDetails(details map[string]any) *Error {
e.Details = details
return e
}
func (e *Error) Unwrap() error {
return e.Err
}
@ -104,48 +114,13 @@ func AsError(err error) (e *Error) {
return
}
type Arg struct {
Key string
Value any
}
// Format is a helper function to return an Error with a given status and formatted message.
func Format(code Status, format string, args ...interface{}) *Error {
var (
err error
details []Arg
newArgs []any
)
for _, arg := range args {
if arg == nil {
continue
}
switch obj := arg.(type) {
case error:
err = obj
case Arg:
details = append(details, obj)
case []Arg:
details = append(details, obj...)
default:
newArgs = append(newArgs, obj)
}
}
msg := fmt.Sprintf(format, newArgs...)
newErr := &Error{
msg := fmt.Sprintf(format, args...)
return &Error{
Status: code,
Message: msg,
Err: err,
}
if len(details) > 0 {
newErr.Details = make(map[string]any)
for _, arg := range details {
newErr.Details[arg.Key] = arg.Value
}
}
return newErr
}
// NotFound is a helper function to return an not found Error.
@ -159,8 +134,11 @@ func InvalidArgument(format string, args ...interface{}) *Error {
}
// Internal is a helper function to return an internal Error.
func Internal(format string, args ...interface{}) *Error {
return Format(StatusInternal, format, args...)
func Internal(err error, format string, args ...interface{}) *Error {
msg := fmt.Sprintf(format, args...)
return Format(StatusInternal, msg).SetErr(
fmt.Errorf("%s: %w", msg, err),
)
}
// Conflict is a helper function to return an conflict Error.

4
git/adapter/blame.go
View File

@ -198,13 +198,13 @@ func (r *BlameReader) NextPart() (*types.BlamePart, error) {
case blamePorcelainOutOfRangeErrorRE.MatchString(line):
return nil, errors.InvalidArgument(line)
default:
return nil, errors.Internal(line)
return nil, errors.Internal(nil, "failed to get next part: %s", line)
}
}
// This error can happen if the command git failed to start. Triggered by pipe writer's CloseWithError call.
if !errors.Is(err, io.EOF) {
return nil, errors.Internal(err.Error())
return nil, errors.Internal(err, "failed to start git blame command")
}
var part *types.BlamePart

4
git/adapter/blame_test.go
View File

@ -163,7 +163,7 @@ func TestBlameReader_NextPart_CmdError(t *testing.T) {
}
_, err := reader.NextPart()
if s := errors.AsError(err); s.Status != errors.StatusInternal || s.Message != "dummy error" {
t.Errorf("expected Internal error but got: %v", err)
if s := errors.AsError(err); s.Status != errors.StatusInternal || s.Message != "failed to start git blame command" {
t.Errorf("expected %v, but got: %v", s.Message, err)
}
}

6
git/adapter/errors.go
View File

@ -27,7 +27,7 @@ import (
var (
ErrRepositoryPathEmpty = errors.InvalidArgument("repository path cannot be empty")
ErrBranchNameEmpty = errors.InvalidArgument("branch name cannot be empty")
ErrParseDiffHunkHeader = errors.Internal("failed to parse diff hunk header")
ErrParseDiffHunkHeader = errors.Internal(nil, "failed to parse diff hunk header")
)
type runStdError struct {
@ -64,7 +64,7 @@ func (r *runStdError) IsExitCode(code int) bool {
// Always logs the full message with error as warning.
func processGiteaErrorf(err error, format string, args ...interface{}) error {
// create fallback error returned if we can't map it
fallbackErr := errors.Internal(format, args...)
fallbackErr := errors.Internal(err, format, args...)
// always log internal error together with message.
log.Warn().Msgf("%v: [GITEA] %v", fallbackErr, err)
@ -125,7 +125,7 @@ func mapGiteaRunStdError(err gitea.RunStdError, fallback error) error {
// exit status 128 - fatal: unable to access 'http://127.0.0.1:4101/hvfl1xj5fojwlrw77xjflw80uxjous254jrr967rvj/':
// Failed to connect to 127.0.0.1 port 4101 after 4 ms: Connection refused
case err.IsExitCode(128) && strings.Contains(err.Stderr(), "Failed to connect"):
return errors.Internal("failed to connect")
return errors.Internal(err, "failed to connect")
default:
return fallback

8
git/adapter/http.go
View File

@ -42,18 +42,18 @@ func (a Adapter) InfoRefs(
Dir: repoPath,
Stdout: cmd,
}); err != nil {
return errors.Internal("InfoRefsUploadPack: cmd: %v", err)
return errors.Internal(err, "InfoRefs service %s failed", service)
}
if _, err := w.Write(packetWrite("# service=git-" + service + "\n")); err != nil {
return errors.Internal("InfoRefsUploadPack: pktLine: %v", err)
return errors.Internal(err, "failed to write pktLine in InfoRefs %s service", service)
}
if _, err := w.Write([]byte("0000")); err != nil {
return errors.Internal("InfoRefsUploadPack: flush: %v", err)
return errors.Internal(err, "failed to flush data in InfoRefs %s service", service)
}
if _, err := io.Copy(w, cmd); err != nil {
return errors.Internal("InfoRefsUploadPack: %v", err)
return errors.Internal(err, "streaming InfoRefs %s service failed", service)
}
return nil
}

2
git/adapter/shared_repo.go
View File

@ -91,7 +91,7 @@ func (r *SharedRepo) Clone(ctx context.Context, branchName string) error {
} else if matched, _ = regexp.MatchString(".* repository .* does not exist.*", stderr); matched {
return errors.NotFound("repository '%s' does not exist", r.repoUID)
}
return errors.Internal("error while cloning repository: %s", stderr)
return errors.Internal(nil, "error while cloning repository: %s", stderr)
}
gitRepo, err := gitea.OpenRepository(ctx, r.tmpPath)
if err != nil {

4
git/branch.go
View File

@ -111,7 +111,7 @@ func (s *Service) CreateBranch(ctx context.Context, params *CreateBranchParams)
targetCommit.SHA,
)
if errors.IsConflict(err) {
return nil, errors.Conflict("branch %q already exists", params.BranchName, err)
return nil, errors.Conflict("branch %q already exists", params.BranchName)
}
if err != nil {
return nil, fmt.Errorf("failed to update branch reference: %w", err)
@ -171,7 +171,7 @@ func (s *Service) DeleteBranch(ctx context.Context, params *DeleteBranchParams)
types.NilSHA,
)
if types.IsNotFoundError(err) {
return errors.NotFound("branch %q does not exist", params.BranchName, err)
return errors.NotFound("branch %q does not exist", params.BranchName)
}
if err != nil {
return fmt.Errorf("failed to delete branch reference: %w", err)

4
git/operations.go
View File

@ -271,7 +271,7 @@ func (s *Service) prepareTreeEmptyRepo(ctx context.Context, shared *adapter.Shar
reader := bytes.NewReader(action.Payload)
if err = createFile(ctx, shared, nil, filePath, defaultFilePermission, reader); err != nil {
return errors.Internal("failed to create file '%s': %w", action.Path, err)
return errors.Internal(err, "failed to create file '%s'", action.Path)
}
}
@ -343,7 +343,7 @@ func (s *Service) processAction(
) (err error) {
filePath := files.CleanUploadFileName(action.Path)
if filePath == "" {
return errors.InvalidArgument("invalid path: %w", err)
return errors.InvalidArgument("path cannot be empty")
}
reader := bytes.NewReader(action.Payload)

5
git/push_remote.go
View File

@ -53,7 +53,10 @@ func (s *Service) PushRemote(ctx context.Context, params *PushRemoteParams) erro
return fmt.Errorf("PushRemote: failed to open repo: %w", err)
}
if ok, err := repo.IsEmpty(); ok {
return errors.InvalidArgument("cannot push empty repo", err)
if err != nil {
return errors.Internal(err, "push to repo failed")
}
return errors.InvalidArgument("cannot push empty repo")
}
err = s.adapter.Push(ctx, repoPath, types.PushOptions{

10
git/repo.go
View File

@ -255,12 +255,12 @@ func (s *Service) SyncRepository(
// create repo if requested
_, err := os.Stat(repoPath)
if err != nil && !os.IsNotExist(err) {
return nil, errors.Internal("failed to create repo: %w", err)
return nil, errors.Internal(err, "failed to create repository")
}
if os.IsNotExist(err) {
if !params.CreateIfNotExists {
return nil, errors.NotFound("repo not found")
return nil, errors.NotFound("repository not found")
}
// the default branch doesn't matter for a sync,
@ -459,7 +459,7 @@ func (s *Service) createRepositoryInternal(
break
}
if err != nil {
return errors.Internal("failed to receive file: %v", err)
return errors.Internal(err, "failed to receive file %s", file)
}
filePaths = append(filePaths, filePath)
@ -495,8 +495,8 @@ func (s *Service) createRepositoryInternal(
hookPath := path.Join(repoPath, gitHooksDir, hook)
err = os.Symlink(s.gitHookPath, hookPath)
if err != nil {
return errors.Internal("failed to setup symlink for hook '%s' ('%s' -> '%s'): %s",
hook, hookPath, s.gitHookPath, err)
return errors.Internal(err, "failed to setup symlink for hook '%s' ('%s' -> '%s')",
hook, hookPath, s.gitHookPath)
}
}

2
git/tag.go
View File

@ -347,7 +347,7 @@ func (s *Service) DeleteTag(ctx context.Context, params *DeleteTagParams) error
types.NilSHA,
)
if types.IsNotFoundError(err) {
return errors.NotFound("tag %q does not exist", params.Name, err)
return errors.NotFound("tag %q does not exist", params.Name)
}
if err != nil {
return fmt.Errorf("failed to delete tag reference: %w", err)

2
git/upload.go
View File

@ -106,7 +106,7 @@ func (s *Service) handleFileUploadIfAvailable(
log.Info().Msgf("saving file at path %s", fullPath)
_, err := s.store.Save(fullPath, bytes.NewReader(file.Content))
if err != nil {
return "", errors.Internal("cannot save file to the store: %v", err)
return "", errors.Internal(err, "cannot save file to the store: %s", fullPath)
}
return fullPath, nil