request/unpacker: Fix empty nginx error not parsed correctly (#72)

qingstor · Jul 11, 2019 · c9f6655 · c9f6655
1 parent 54a6849
commit c9f6655
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 18 deletions.
diff --git a/request/unpacker/qingstor.go b/request/unpacker/qingstor.go
@@ -19,9 +19,11 @@ package unpacker
 import (
 	"bytes"
 	"encoding/json"
+	"io"
 	"net/http"
 	"reflect"
 
+	"github.com/yunify/qingstor-sdk-go/v3/logger"
 	"github.com/yunify/qingstor-sdk-go/v3/request/data"
 	"github.com/yunify/qingstor-sdk-go/v3/request/errors"
 )
@@ -58,27 +60,43 @@ func (qu *QingStorUnpacker) UnpackHTTPRequest(o *data.Operation, r *http.Respons
 }
 
 func (qu *QingStorUnpacker) parseError() error {
-	if !qu.baseUnpacker.isResponseRight() {
-		if qu.baseUnpacker.httpResponse.Header.Get("Content-Type") == "application/json" {
-			buffer := &bytes.Buffer{}
-			buffer.ReadFrom(qu.baseUnpacker.httpResponse.Body)
-			qu.baseUnpacker.httpResponse.Body.Close()
+	if qu.baseUnpacker.isResponseRight() {
+		return nil
+	}
+
+	// QingStor nginx could refuse user's request directly and only return status code.
+	// We should handle this and build a qingstor error with message.
+	if qu.baseUnpacker.httpResponse.Header.Get("Content-Type") != "application/json" {
+		qsError := &errors.QingStorError{
+			StatusCode: qu.baseUnpacker.httpResponse.StatusCode,
+			Message:    http.StatusText(qu.baseUnpacker.httpResponse.StatusCode),
+		}
+		return qsError
+	}
 
-			qsError := &errors.QingStorError{}
-			if buffer.Len() > 0 {
-				err := json.Unmarshal(buffer.Bytes(), qsError)
-				if err != nil {
-					return err
-				}
-			}
-			qsError.StatusCode = qu.baseUnpacker.httpResponse.StatusCode
-			if qsError.RequestID == "" {
-				qsError.RequestID = qu.baseUnpacker.httpResponse.Header.Get(http.CanonicalHeaderKey("X-QS-Request-ID"))
-			}
+	buffer := &bytes.Buffer{}
+	_, err := io.Copy(buffer, qu.baseUnpacker.httpResponse.Body)
+	if err != nil {
+		logger.Errorf(nil, "Copy from error response body failed for %v", err)
+		return err
+	}
+	err = qu.baseUnpacker.httpResponse.Body.Close()
+	if err != nil {
+		logger.Errorf(nil, "Close error response body failed for %v", err)
+		return err
+	}
 
-			return qsError
+	qsError := &errors.QingStorError{}
+	if buffer.Len() > 0 {
+		err := json.Unmarshal(buffer.Bytes(), qsError)
+		if err != nil {
+			return err
 		}
 	}
+	qsError.StatusCode = qu.baseUnpacker.httpResponse.StatusCode
+	if qsError.RequestID == "" {
+		qsError.RequestID = qu.baseUnpacker.httpResponse.Header.Get(http.CanonicalHeaderKey("X-QS-Request-ID"))
+	}
 
-	return nil
+	return qsError
 }
diff --git a/request/unpacker/qingstor_test.go b/request/unpacker/qingstor_test.go
@@ -148,3 +148,25 @@ func TestQingStorUnpacker_UnpackHeadHTTPRequestWithError(t *testing.T) {
 		assert.Equal(t, "aa08cf7a43f611e5886952542e6ce14b", e.RequestID)
 	}
 }
+
+func TestQingStorUnpacker_UnpackHTTPRequestWithEmptyError(t *testing.T) {
+	type ListBucketsOutput struct {
+		StatusCode *int `location:"statusCode"`
+		Error      *errors.QingStorError
+		RequestID  *string `location:"requestID"`
+	}
+
+	httpResponse := &http.Response{Header: http.Header{}}
+	httpResponse.StatusCode = 400
+	httpResponse.Body = ioutil.NopCloser(strings.NewReader(""))
+
+	output := &ListBucketsOutput{}
+	outputValue := reflect.ValueOf(output)
+	unpacker := QingStorUnpacker{}
+	err := unpacker.UnpackHTTPRequest(&data.Operation{}, httpResponse, &outputValue)
+	assert.NotNil(t, err)
+	switch e := err.(type) {
+	case *errors.QingStorError:
+		assert.Equal(t, 400, e.StatusCode)
+	}
+}