append response body to error if it exists

swift.go

@@ -9,6 +9,7 @@ import (
 	"crypto/sha1"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"hash"
 	"io"
@@ -24,14 +25,15 @@ import (
 )
 
 const (
-	DefaultUserAgent    = "goswift/1.0"         // Default user agent
-	DefaultRetries      = 3                     // Default number of retries on token expiry
-	TimeFormat          = "2006-01-02T15:04:05" // Python date format for json replies parsed as UTC
-	UploadTar           = "tar"                 // Data format specifier for Connection.BulkUpload().
-	UploadTarGzip       = "tar.gz"              // Data format specifier for Connection.BulkUpload().
-	UploadTarBzip2      = "tar.bz2"             // Data format specifier for Connection.BulkUpload().
-	allContainersLimit  = 10000                 // Number of containers to fetch at once
-	allObjectsChanLimit = 1000                  // Number objects to fetch when fetching to a channel
+	DefaultUserAgent     = "goswift/1.0"         // Default user agent
+	DefaultRetries       = 3                     // Default number of retries on token expiry
+	TimeFormat           = "2006-01-02T15:04:05" // Python date format for json replies parsed as UTC
+	UploadTar            = "tar"                 // Data format specifier for Connection.BulkUpload().
+	UploadTarGzip        = "tar.gz"              // Data format specifier for Connection.BulkUpload().
+	UploadTarBzip2       = "tar.bz2"             // Data format specifier for Connection.BulkUpload().
+	allContainersLimit   = 10000                 // Number of containers to fetch at once
+	allObjectsChanLimit  = 1000                  // Number objects to fetch when fetching to a channel
+	respBodyErrSizeLimit = 1024                  // Maximum size of response body to read when appending to error messages
 )
 
 // ObjectType is the type of the swift object, regular, static large,
@@ -372,22 +374,65 @@ func drainAndClose(rd io.ReadCloser, err *error) {
 }
 
 // parseHeaders checks a response for errors and translates into
-// standard errors if necessary. If an error is returned, resp.Body
+// standard errors if necessary. If an error message is present in the response body,
+// it will be included in the error. If an error is returned, resp.Body
 // has been drained and closed.
 func (c *Connection) parseHeaders(resp *http.Response, errorMap errorMap) error {
 	if errorMap != nil {
 		if err, ok := errorMap[resp.StatusCode]; ok {
+			err = appendResponseBodyToError(resp, err)
 			drainAndClose(resp.Body, nil)
 			return err
 		}
 	}
 	if resp.StatusCode < 200 || resp.StatusCode > 299 {
+		var err error = newErrorf(resp.StatusCode, "HTTP Error: %d: %s", resp.StatusCode, resp.Status)
+		err = appendResponseBodyToError(resp, err)
 		drainAndClose(resp.Body, nil)
-		return newErrorf(resp.StatusCode, "HTTP Error: %d: %s", resp.StatusCode, resp.Status)
+		return err
 	}
 	return nil
 }
 
+// appendResponseBodyToError tries to append the response body to the error message.
+func appendResponseBodyToError(resp *http.Response, err error) error {
+	if resp == nil || resp.Body == nil || err == nil {
+		return err
+	}
+
+	if resp.Header.Get("Content-Length") == "0" {
+		return err
+	}
+
+	ct := resp.Header.Get("Content-Type")
+	if ct == "" {
+		return err
+	}
+
+	lowerCT := strings.ToLower(ct)
+	if !(strings.Contains(lowerCT, "text") ||
+		strings.Contains(lowerCT, "json") ||
+		strings.Contains(lowerCT, "xml") ||
+		strings.Contains(lowerCT, "html") ||
+		strings.Contains(lowerCT, "plain")) {
+		return err
+	}
+
+	buf := make([]byte, respBodyErrSizeLimit)
+	limitedReader := io.LimitReader(resp.Body, respBodyErrSizeLimit)
+	n, readErr := limitedReader.Read(buf)
+	if readErr != nil || n == 0 {
+		return err
+	}
+
+	trimmed := strings.TrimSpace(string(buf[:n]))
+	if trimmed == "" {
+		return err
+	}
+
+	return fmt.Errorf("%w: %s", err, trimmed)
+}
+
 // readHeaders returns a Headers object from the http.Response.
 //
 // If it receives multiple values for a key (which should never
@@ -519,7 +564,7 @@ again:
 			// Try again for a limited number of times on
 			// AuthorizationFailed or BadRequest. This allows us
 			// to try some alternate forms of the request
-			if (err == AuthorizationFailed || err == BadRequest) && retries > 0 {
+			if (errors.Is(err, AuthorizationFailed) || errors.Is(err, BadRequest)) && retries > 0 {
 				retries--
 				goto again
 			}

swift_internal_test.go

@@ -7,12 +7,14 @@ package swift
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net"
 	"net/http"
 	"os"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 )
@@ -284,6 +286,84 @@ func TestInternalParseHeaders(t *testing.T) {
 	}
 }
 
+func TestInternalParseHeadersSomething(t *testing.T) {
+	testCases := []struct {
+		name     string
+		resp     *http.Response
+		errMap   map[int]error
+		expected string
+	}{
+		{
+			name: "Container error with body",
+			resp: &http.Response{
+				StatusCode: 400,
+				Header: http.Header{
+					"Content-Type": []string{"text/plain"},
+				},
+				ContentLength: 15,
+				Body:          io.NopCloser(strings.NewReader("Body message")),
+				Status:        "Status message",
+			},
+			errMap:   ContainerErrorMap,
+			expected: "Bad Request: Body message",
+		},
+		{
+			name: "Error with body",
+			resp: &http.Response{
+				StatusCode: 400,
+				Header: http.Header{
+					"Content-Type": []string{"text/plain"},
+				},
+				ContentLength: 15,
+				Body:          io.NopCloser(strings.NewReader("Body message")),
+				Status:        "Status message",
+			},
+			errMap:   nil,
+			expected: "HTTP Error: 400: Status message: Body message",
+		},
+		{
+			name: "Object error with body",
+			resp: &http.Response{
+				StatusCode: 400,
+				Header: http.Header{
+					"Content-Type": []string{"text/plain"},
+				},
+				ContentLength: 15,
+				Body:          io.NopCloser(strings.NewReader("Body message")),
+				Status:        "Status message",
+			},
+			errMap:   objectErrorMap,
+			expected: "Bad Request: Body message",
+		},
+		{
+			name: "Object error with body over 1024 bytes",
+			resp: &http.Response{
+				StatusCode: 400,
+				Header: http.Header{
+					"Content-Type": []string{"text/plain"},
+				},
+				ContentLength: 1025,
+				Body:          io.NopCloser(strings.NewReader(strings.Repeat("a", 1025))),
+				Status:        "Status message",
+			},
+			errMap:   objectErrorMap,
+			expected: fmt.Sprintf("%s: %s", "Bad Request", strings.Repeat("a", 1024)),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := c.parseHeaders(tc.resp, tc.errMap)
+			if err == nil {
+				t.Fatalf("Expected error, got nil")
+			}
+			if err.Error() != tc.expected {
+				t.Errorf("Expected error %q, got %q", tc.expected, err.Error())
+			}
+		})
+	}
+}
+
 func TestInternalReadHeaders(t *testing.T) {
 	resp := &http.Response{Header: http.Header{}}
 	compareMaps(t, readHeaders(resp), Headers{})
@@ -341,7 +421,7 @@ func TestInternalAuthenticateDenied(t *testing.T) {
 	defer server.Finished()
 	c.UnAuthenticate()
 	err := c.Authenticate(context.Background())
-	if err != AuthorizationFailed {
+	if !errors.Is(err, AuthorizationFailed) {
 		t.Fatal("Expecting AuthorizationFailed", err)
 	}
 	// FIXME

swift_test.go

@@ -21,6 +21,7 @@ import (
 	"crypto/tls"
 	"encoding/json"
 	"encoding/xml"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -1207,7 +1208,7 @@ func TestObjectCreate(t *testing.T) {
 	// FIXME: work around bug which produces 503 not 422 for empty corrupted files
 	_, _ = fmt.Fprintf(out, "Sausage")
 	err = out.Close()
-	if err != swift.ObjectCorrupted {
+	if !errors.Is(err, swift.ObjectCorrupted) {
 		t.Error("Expecting object corrupted not", err)
 	}
 }
@@ -1238,7 +1239,7 @@ func TestObjectCreateAbort(t *testing.T) {
 	}
 
 	_, err = c.ObjectGetString(ctx, CONTAINER, OBJECT2)
-	if err != swift.ObjectNotFound {
+	if !errors.Is(err, swift.ObjectNotFound) {
 		t.Errorf("Unexpected error: %#v", err)
 	}
 }
@@ -1989,7 +1990,7 @@ func TestVersionDeleteContent(t *testing.T) {
 	if err := c.ObjectDelete(ctx, CURRENT_CONTAINER, OBJECT); err != nil {
 		t.Fatal(err)
 	}
-	if err := c.ObjectDelete(ctx, CURRENT_CONTAINER, OBJECT); err != swift.ObjectNotFound {
+	if err := c.ObjectDelete(ctx, CURRENT_CONTAINER, OBJECT); !errors.Is(err, swift.ObjectNotFound) {
 		t.Fatalf("Expecting Object not found error, got: %v", err)
 	}
 }
@@ -2000,7 +2001,7 @@ func testExistenceAfterDelete(t *testing.T, c *swift.Connection, container, obje
 	ctx := context.Background()
 	for i := 10; i <= 0; i-- {
 		_, _, err := c.Object(ctx, container, object)
-		if err == swift.ObjectNotFound {
+		if errors.Is(err, swift.ObjectNotFound) {
 			break
 		}
 		if i == 0 {
@@ -2020,7 +2021,7 @@ func TestObjectDelete(t *testing.T) {
 	}
 	testExistenceAfterDelete(t, c, CONTAINER, OBJECT)
 	err = c.ObjectDelete(ctx, CONTAINER, OBJECT)
-	if err != swift.ObjectNotFound {
+	if !errors.Is(err, swift.ObjectNotFound) {
 		t.Fatal("Expecting Object not found", err)
 	}
 }
@@ -2030,7 +2031,7 @@ func TestBulkDelete(t *testing.T) {
 	c, rollback := makeConnectionWithContainer(t)
 	defer rollback()
 	result, err := c.BulkDelete(ctx, CONTAINER, []string{OBJECT})
-	if err == swift.Forbidden {
+	if errors.Is(err, swift.Forbidden) {
 		t.Log("Server doesn't support BulkDelete - skipping test")
 		return
 	}
@@ -3237,11 +3238,11 @@ func TestContainerDelete(t *testing.T) {
 		t.Fatal(err)
 	}
 	err = c.ContainerDelete(ctx, CONTAINER)
-	if err != swift.ContainerNotFound {
+	if !errors.Is(err, swift.ContainerNotFound) {
 		t.Fatal("Expecting container not found", err)
 	}
 	_, _, err = c.Container(ctx, CONTAINER)
-	if err != swift.ContainerNotFound {
+	if !errors.Is(err, swift.ContainerNotFound) {
 		t.Fatal("Expecting container not found", err)
 	}
 }