aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormatej.risek <matej.risek@hashicorp.com>2023-07-12 14:40:09 +0200
committermatej.risek <matej.risek@hashicorp.com>2023-09-05 17:09:59 +0200
commit5ad72db3bd60351da9ab42727952654e3ad5bcb2 (patch)
tree35179f19c30d4cc31c8c9a0d5957e66c11e98dd5
parentcd3a21c619126288123c32f4a714181f9e1a68f8 (diff)
downloadgo-git-5ad72db3bd60351da9ab42727952654e3ad5bcb2.tar.gz
plumbing: Do not swallow http message coming from VCS providers.
For diagnostics reasons we want to surface error messages coming from VCS providers. That's why we introduce the reason field to Err struct in http package. This field can be used by an end user of the library in order to better understand failures.
-rw-r--r--plumbing/transport/http/common.go18
-rw-r--r--plumbing/transport/http/common_test.go19
-rw-r--r--plumbing/transport/http/receive_pack.go1
-rw-r--r--plumbing/transport/http/upload_pack.go1
4 files changed, 35 insertions, 4 deletions
diff --git a/plumbing/transport/http/common.go b/plumbing/transport/http/common.go
index a7cdc1e..54126fe 100644
--- a/plumbing/transport/http/common.go
+++ b/plumbing/transport/http/common.go
@@ -406,14 +406,28 @@ func (a *TokenAuth) String() string {
// Err is a dedicated error to return errors based on status code
type Err struct {
Response *http.Response
+ Reason string
}
-// NewErr returns a new Err based on a http response
+// NewErr returns a new Err based on a http response and closes response body
+// if needed
func NewErr(r *http.Response) error {
if r.StatusCode >= http.StatusOK && r.StatusCode < http.StatusMultipleChoices {
return nil
}
+ var reason string
+
+ // If a response message is present, add it to error
+ var messageBuffer bytes.Buffer
+ if r.Body != nil {
+ messageLength, _ := messageBuffer.ReadFrom(r.Body)
+ if messageLength > 0 {
+ reason = messageBuffer.String()
+ }
+ _ = r.Body.Close()
+ }
+
switch r.StatusCode {
case http.StatusUnauthorized:
return transport.ErrAuthenticationRequired
@@ -423,7 +437,7 @@ func NewErr(r *http.Response) error {
return transport.ErrRepositoryNotFound
}
- return plumbing.NewUnexpectedError(&Err{r})
+ return plumbing.NewUnexpectedError(&Err{r, reason})
}
// StatusCode returns the status code of the response
diff --git a/plumbing/transport/http/common_test.go b/plumbing/transport/http/common_test.go
index 1517228..6bd018b 100644
--- a/plumbing/transport/http/common_test.go
+++ b/plumbing/transport/http/common_test.go
@@ -3,6 +3,7 @@ package http
import (
"crypto/tls"
"fmt"
+ "io"
"log"
"net"
"net/http"
@@ -14,6 +15,7 @@ import (
"strings"
"testing"
+ "github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/transport"
fixtures "github.com/go-git/go-git-fixtures/v4"
@@ -90,6 +92,23 @@ func (s *ClientSuite) TestNewHTTPError40x(c *C) {
"unexpected client error.*")
}
+func (s *ClientSuite) TestNewUnexpectedError(c *C) {
+ res := &http.Response{
+ StatusCode: 500,
+ Body: io.NopCloser(strings.NewReader("Unexpected error")),
+ }
+
+ err := NewErr(res)
+ c.Assert(err, NotNil)
+ c.Assert(err, FitsTypeOf, &plumbing.UnexpectedError{})
+
+ unexpectedError, _ := err.(*plumbing.UnexpectedError)
+ c.Assert(unexpectedError.Err, FitsTypeOf, &Err{})
+
+ httpError, _ := unexpectedError.Err.(*Err)
+ c.Assert(httpError.Reason, Equals, "Unexpected error")
+}
+
func (s *ClientSuite) Test_newSession(c *C) {
cl := NewClientWithOptions(nil, &ClientOptions{
CacheMaxEntries: 2,
diff --git a/plumbing/transport/http/receive_pack.go b/plumbing/transport/http/receive_pack.go
index 4387ecf..3e736cd 100644
--- a/plumbing/transport/http/receive_pack.go
+++ b/plumbing/transport/http/receive_pack.go
@@ -102,7 +102,6 @@ func (s *rpSession) doRequest(
}
if err := NewErr(res); err != nil {
- _ = res.Body.Close()
return nil, err
}
diff --git a/plumbing/transport/http/upload_pack.go b/plumbing/transport/http/upload_pack.go
index 4f85145..3432618 100644
--- a/plumbing/transport/http/upload_pack.go
+++ b/plumbing/transport/http/upload_pack.go
@@ -100,7 +100,6 @@ func (s *upSession) doRequest(
}
if err := NewErr(res); err != nil {
- _ = res.Body.Close()
return nil, err
}