Skip to content

Commit 9421a1a

Browse files
committed
fix err handling
1 parent e153f9f commit 9421a1a

4 files changed

Lines changed: 38 additions & 8 deletions

File tree

modules/git/gitcmd/command.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,9 +426,9 @@ type RunStdError interface {
426426
}
427427

428428
type runStdError struct {
429-
err error
430-
stderr string
431-
errMsg string
429+
err error // usually the low-level error like exec.ExitError
430+
stderr string // git command's stderr output
431+
errMsg string // the cached error message for Error() method
432432
}
433433

434434
func (r *runStdError) Error() string {
@@ -448,6 +448,22 @@ func (r *runStdError) Stderr() string {
448448
return r.stderr
449449
}
450450

451+
func ErrorAsStderr(err error) (string, bool) {
452+
var runErr RunStdError
453+
if errors.As(err, &runErr) {
454+
return runErr.Stderr(), true
455+
}
456+
return "", false
457+
}
458+
459+
func StderrHasPrefix(err error, prefix string) bool {
460+
stderr, ok := ErrorAsStderr(err)
461+
if !ok {
462+
return false
463+
}
464+
return strings.HasPrefix(stderr, prefix)
465+
}
466+
451467
func IsErrorExitCode(err error, code int) bool {
452468
var exitError *exec.ExitError
453469
if errors.As(err, &exitError) {

modules/git/gitcmd/utils.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,16 @@
33

44
package gitcmd
55

6-
import "fmt"
6+
import (
7+
"errors"
8+
"fmt"
9+
)
710

8-
// ConcatenateError concatenats an error with stderr string
11+
// ConcatenateError concatenates an error with stderr string
12+
// FIXME: use RunStdError instead
913
func ConcatenateError(err error, stderr string) error {
1014
if len(stderr) == 0 {
1115
return err
1216
}
13-
return fmt.Errorf("%w - %s", err, stderr)
17+
return errors.Join(fmt.Errorf("%w - %s", err, stderr), &runStdError{err: err, stderr: stderr})
1418
}

services/repository/archiver/archiver.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,8 @@ func ServeRepoArchive(ctx *gitea_context.Base, archiveReq *ArchiveRequest) error
329329
// the header must be set before starting streaming even an error would occur,
330330
// because errors may happen in git command and such cases aren't in our control.
331331
httplib.ServeSetHeaders(ctx.Resp, &httplib.ServeHeaderOptions{Filename: downloadName})
332-
var gitErr gitcmd.RunStdError
333332
if err := archiveReq.Stream(ctx, ctx.Resp); err != nil && !ctx.Written() {
334-
if errors.As(err, &gitErr); strings.HasPrefix(gitErr.Stderr(), "fatal: pathspec") {
333+
if gitcmd.StderrHasPrefix(err, "fatal: pathspec") {
335334
return util.NewInvalidArgumentErrorf("path doesn't exist or is invalid")
336335
}
337336
return fmt.Errorf("archive repo %s: failed to stream: %w", archiveReq.Repo.FullName(), err)

services/repository/archiver/archiver_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ import (
88
"time"
99

1010
"code.gitea.io/gitea/models/unittest"
11+
"code.gitea.io/gitea/modules/util"
1112
"code.gitea.io/gitea/services/contexttest"
1213

1314
_ "code.gitea.io/gitea/models/actions"
1415

1516
"github.com/stretchr/testify/assert"
17+
"github.com/stretchr/testify/require"
1618
)
1719

1820
func TestMain(m *testing.M) {
@@ -124,4 +126,13 @@ func TestArchive_Basic(t *testing.T) {
124126
// Ideally, the extension would match what we originally requested.
125127
assert.NotEqual(t, zipReq.GetArchiveName(), tgzReq.GetArchiveName())
126128
assert.NotEqual(t, zipReq.GetArchiveName(), secondReq.GetArchiveName())
129+
130+
t.Run("BadPath", func(t *testing.T) {
131+
badRequest, err := NewRequest(ctx.Repo.Repository, ctx.Repo.GitRepo, firstCommit+".tar.gz", []string{"not-a-path"})
132+
require.NoError(t, err)
133+
err = ServeRepoArchive(ctx.Base, badRequest)
134+
require.Error(t, err)
135+
assert.ErrorIs(t, err, util.ErrInvalidArgument)
136+
assert.ErrorContains(t, err, "path doesn't exist or is invalid")
137+
})
127138
}

0 commit comments

Comments
 (0)