Prevent panic in ResponseError.Error() (Azure#21839)

* Prevent panic in ResponseError.Error()

If RawResponse is nil (unit test scenarios), print "nil RawResponse"
instead of panicing.

* handle nil Request in *http.Response

tweak string for missing RawResponse

Author: jhendrixMSFT

sdk/azcore/CHANGELOG.md

@@ -15,6 +15,7 @@
 * Fixed an issue that could cause an HTTP/2 request to hang when the TCP connection becomes unresponsive.
 * Block key and SAS authentication for non TLS protected endpoints.
 * Passing a `nil` credential value will no longer cause a panic. Instead, the authentication is skipped.
+* Calling `Error` on a zero-value `azcore.ResponseError` will no longer panic.
 
 ### Other Changes
 

sdk/azcore/internal/exported/response_error.go

@@ -113,33 +113,45 @@ type ResponseError struct {
 // Error implements the error interface for type ResponseError.
 // Note that the message contents are not contractual and can change over time.
 func (e *ResponseError) Error() string {
+	const separator = "--------------------------------------------------------------------------------"
 	// write the request method and URL with response status code
 	msg := &bytes.Buffer{}
-	fmt.Fprintf(msg, "%s %s://%s%s\n", e.RawResponse.Request.Method, e.RawResponse.Request.URL.Scheme, e.RawResponse.Request.URL.Host, e.RawResponse.Request.URL.Path)
-	fmt.Fprintln(msg, "--------------------------------------------------------------------------------")
-	fmt.Fprintf(msg, "RESPONSE %d: %s\n", e.RawResponse.StatusCode, e.RawResponse.Status)
+	if e.RawResponse != nil {
+		if e.RawResponse.Request != nil {
+			fmt.Fprintf(msg, "%s %s://%s%s\n", e.RawResponse.Request.Method, e.RawResponse.Request.URL.Scheme, e.RawResponse.Request.URL.Host, e.RawResponse.Request.URL.Path)
+		} else {
+			fmt.Fprintln(msg, "Request information not available")
+		}
+		fmt.Fprintln(msg, separator)
+		fmt.Fprintf(msg, "RESPONSE %d: %s\n", e.RawResponse.StatusCode, e.RawResponse.Status)
+	} else {
+		fmt.Fprintln(msg, "Missing RawResponse")
+		fmt.Fprintln(msg, separator)
+	}
 	if e.ErrorCode != "" {
 		fmt.Fprintf(msg, "ERROR CODE: %s\n", e.ErrorCode)
 	} else {
 		fmt.Fprintln(msg, "ERROR CODE UNAVAILABLE")
 	}
-	fmt.Fprintln(msg, "--------------------------------------------------------------------------------")
-	body, err := exported.Payload(e.RawResponse, nil)
-	if err != nil {
-		// this really shouldn't fail at this point as the response
-		// body is already cached (it was read in NewResponseError)
-		fmt.Fprintf(msg, "Error reading response body: %v", err)
-	} else if len(body) > 0 {
-		if err := json.Indent(msg, body, "", "  "); err != nil {
-			// failed to pretty-print so just dump it verbatim
-			fmt.Fprint(msg, string(body))
+	if e.RawResponse != nil {
+		fmt.Fprintln(msg, separator)
+		body, err := exported.Payload(e.RawResponse, nil)
+		if err != nil {
+			// this really shouldn't fail at this point as the response
+			// body is already cached (it was read in NewResponseError)
+			fmt.Fprintf(msg, "Error reading response body: %v", err)
+		} else if len(body) > 0 {
+			if err := json.Indent(msg, body, "", "  "); err != nil {
+				// failed to pretty-print so just dump it verbatim
+				fmt.Fprint(msg, string(body))
+			}
+			// the standard library doesn't have a pretty-printer for XML
+			fmt.Fprintln(msg)
+		} else {
+			fmt.Fprintln(msg, "Response contained no body")
 		}
-		// the standard library doesn't have a pretty-printer for XML
-		fmt.Fprintln(msg)
-	} else {
-		fmt.Fprintln(msg, "Response contained no body")
 	}
-	fmt.Fprintln(msg, "--------------------------------------------------------------------------------")
+	fmt.Fprintln(msg, separator)
 
 	return msg.String()
 }

sdk/azcore/internal/exported/response_error_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared"
+	"github.com/stretchr/testify/require"
 )
 
 func TestNewResponseErrorNoBodyNoErrorCode(t *testing.T) {
@@ -450,3 +451,20 @@ func TestExtractErrorCodeFromJSON(t *testing.T) {
 		t.Fatalf("expected %s got %s", "ResourceNotFound", code)
 	}
 }
+
+func TestNilRawResponse(t *testing.T) {
+	const expected = "Missing RawResponse\n--------------------------------------------------------------------------------\nERROR CODE UNAVAILABLE\n--------------------------------------------------------------------------------\n"
+	require.EqualValues(t, expected, (&ResponseError{}).Error())
+}
+
+func TestNilRequestInRawResponse(t *testing.T) {
+	const expected = "Request information not available\n--------------------------------------------------------------------------------\nRESPONSE 400: status\nERROR CODE UNAVAILABLE\n--------------------------------------------------------------------------------\nResponse contained no body\n--------------------------------------------------------------------------------\n"
+	respErr := &ResponseError{
+		RawResponse: &http.Response{
+			Body:       http.NoBody,
+			Status:     "status",
+			StatusCode: http.StatusBadRequest,
+		},
+	}
+	require.EqualValues(t, expected, respErr.Error())
+}