From 129b709887b4528ced42c8d74f4c2609800a8942 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Sat, 7 Oct 2023 00:02:01 +0200 Subject: plumbing: transport/common, Improve handling of remote errors Instead of simply returning the first line that the remote returned, go-git now actively searches all of stderr for lines that may contain a more actionable error message and returns that. In addition, this change adds a case to map the GitLab-specific error message to an ErrRepositoryNotFound error. Signed-off-by: Max Jonas Werner --- plumbing/transport/internal/common/common.go | 25 ++++++++-- plumbing/transport/internal/common/common_test.go | 57 +++++++++++++++++++++++ plumbing/transport/internal/common/mocks.go | 46 ++++++++++++++++++ 3 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 plumbing/transport/internal/common/mocks.go diff --git a/plumbing/transport/internal/common/common.go b/plumbing/transport/internal/common/common.go index 5fdf425..6574116 100644 --- a/plumbing/transport/internal/common/common.go +++ b/plumbing/transport/internal/common/common.go @@ -11,6 +11,7 @@ import ( "errors" "fmt" "io" + "regexp" "strings" "time" @@ -28,6 +29,10 @@ const ( var ( ErrTimeoutExceeded = errors.New("timeout exceeded") + // stdErrSkipPattern is used for skipping lines from a command's stderr output. + // Any line matching this pattern will be skipped from further + // processing and not be returned to calling code. + stdErrSkipPattern = regexp.MustCompile("^remote:( =*){0,1}$") ) // Commander creates Command instances. This is the main entry point for @@ -149,10 +154,17 @@ func (c *client) listenFirstError(r io.Reader) chan string { errLine := make(chan string, 1) go func() { s := bufio.NewScanner(r) - if s.Scan() { - errLine <- s.Text() - } else { - close(errLine) + for { + if s.Scan() { + line := s.Text() + if !stdErrSkipPattern.MatchString(line) { + errLine <- line + break + } + } else { + close(errLine) + break + } } _, _ = io.Copy(io.Discard, r) @@ -393,6 +405,7 @@ var ( gitProtocolNoSuchErr = "ERR no such repository" gitProtocolAccessDeniedErr = "ERR access denied" gogsAccessDeniedErr = "Gogs: Repository does not exist or you do not have access" + gitlabRepoNotFoundErr = "remote: ERROR: The project you were looking for could not be found" ) func isRepoNotFoundError(s string) bool { @@ -424,6 +437,10 @@ func isRepoNotFoundError(s string) bool { return true } + if strings.HasPrefix(s, gitlabRepoNotFoundErr) { + return true + } + return false } diff --git a/plumbing/transport/internal/common/common_test.go b/plumbing/transport/internal/common/common_test.go index affa787..f6f2f67 100644 --- a/plumbing/transport/internal/common/common_test.go +++ b/plumbing/transport/internal/common/common_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + "github.com/go-git/go-git/v5/plumbing/transport" . "gopkg.in/check.v1" ) @@ -77,6 +78,14 @@ func (s *CommonSuite) TestIsRepoNotFoundErrorForGogsAccessDenied(c *C) { c.Assert(isRepoNotFound, Equals, true) } +func (s *CommonSuite) TestIsRepoNotFoundErrorForGitlab(c *C) { + msg := fmt.Sprintf("%s : some error stuf", gitlabRepoNotFoundErr) + + isRepoNotFound := isRepoNotFoundError(msg) + + c.Assert(isRepoNotFound, Equals, true) +} + func (s *CommonSuite) TestCheckNotFoundError(c *C) { firstErrLine := make(chan string, 1) @@ -90,3 +99,51 @@ func (s *CommonSuite) TestCheckNotFoundError(c *C) { c.Assert(err, IsNil) } + +func TestAdvertisedReferencesWithRemoteError(t *testing.T) { + tests := []struct { + name string + stderr string + wantErr error + }{ + { + name: "unknown error", + stderr: "something", + wantErr: fmt.Errorf("unknown error: something"), + }, + { + name: "GitLab: repository not found", + stderr: `remote: +remote: ======================================================================== +remote: +remote: ERROR: The project you were looking for could not be found or you don't have permission to view it. + +remote: +remote: ======================================================================== +remote:`, + wantErr: transport.ErrRepositoryNotFound, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := NewClient(MockCommander{stderr: tt.stderr}) + sess, err := client.NewUploadPackSession(nil, nil) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + _, err = sess.AdvertisedReferences() + + if tt.wantErr != nil { + if tt.wantErr != err { + if tt.wantErr.Error() != err.Error() { + t.Fatalf("expected a different error: got '%s', expected '%s'", err, tt.wantErr) + } + } + } else if err != nil { + t.Fatalf("unexpected error: %s", err) + } + }) + } +} diff --git a/plumbing/transport/internal/common/mocks.go b/plumbing/transport/internal/common/mocks.go new file mode 100644 index 0000000..bc18b27 --- /dev/null +++ b/plumbing/transport/internal/common/mocks.go @@ -0,0 +1,46 @@ +package common + +import ( + "bytes" + "io" + + gogitioutil "github.com/go-git/go-git/v5/utils/ioutil" + + "github.com/go-git/go-git/v5/plumbing/transport" +) + +type MockCommand struct { + stdin bytes.Buffer + stdout bytes.Buffer + stderr bytes.Buffer +} + +func (c MockCommand) StderrPipe() (io.Reader, error) { + return &c.stderr, nil +} + +func (c MockCommand) StdinPipe() (io.WriteCloser, error) { + return gogitioutil.WriteNopCloser(&c.stdin), nil +} + +func (c MockCommand) StdoutPipe() (io.Reader, error) { + return &c.stdout, nil +} + +func (c MockCommand) Start() error { + return nil +} + +func (c MockCommand) Close() error { + panic("not implemented") +} + +type MockCommander struct { + stderr string +} + +func (c MockCommander) Command(cmd string, ep *transport.Endpoint, auth transport.AuthMethod) (Command, error) { + return &MockCommand{ + stderr: *bytes.NewBufferString(c.stderr), + }, nil +} -- cgit