From 068718e07d956d3e08bd19a75c12bae9c510963c Mon Sep 17 00:00:00 2001
From: Alexander Noskov <anoskov@mirantis.com>
Date: Tue, 25 Aug 2020 22:58:59 -0500
Subject: [PATCH] Proper error handling with Sushy emulator

This change adds a more informative error message in case of using airshipctl
with a Sushy emulator.

In the current implementation, by executing `airshipctl baremetal powerstatus`
command with invalid BMH configuration for example, returns:
redfish client encountered an error: BMC responded '500 INTERNAL SERVER ERROR'.

After this change the output looks like this:
redfish client encountered an error: BMC responded '500 INTERNAL SERVER ERROR'.
BMC responded: 'Error finding domain by name/UUID "air-ephemeral1" at libvirt
URI qemu:///system": Domain not found: no domain with matching name 'air-ephemeral1''

In case of using airshipctl with baremetal BMC, extendedInfo contains a valid
error message, so there are no issues.

Relates-To: #320

Change-Id: I437f50d5df4b0561f352804f269b0319badcc755
---
 pkg/remote/redfish/utils.go | 50 +++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go
index 0ebf66e67..bff28afa6 100644
--- a/pkg/remote/redfish/utils.go
+++ b/pkg/remote/redfish/utils.go
@@ -33,28 +33,28 @@ const (
 	redfishURLSchemeSeparator = "+"
 )
 
+func processExtendedInfo(extendedInfo map[string]interface{}) (string, error) {
+	message, ok := extendedInfo["Message"]
+	if !ok {
+		return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo.Message"}
+	}
+
+	messageContent, ok := message.(string)
+	if !ok {
+		return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo.Message"}
+	}
+
+	// Resolution may be omitted in some responses
+	if resolution, ok := extendedInfo["Resolution"]; ok {
+		return fmt.Sprintf("%s %s", messageContent, resolution), nil
+	}
+
+	return messageContent, nil
+}
+
 // DecodeRawError decodes a raw Redfish HTTP response and retrieves the extended information and available resolutions
 // returned by the BMC.
 func DecodeRawError(rawResponse []byte) (string, error) {
-	processExtendedInfo := func(extendedInfo map[string]interface{}) (string, error) {
-		message, ok := extendedInfo["Message"]
-		if !ok {
-			return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo.Message"}
-		}
-
-		messageContent, ok := message.(string)
-		if !ok {
-			return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo.Message"}
-		}
-
-		// Resolution may be omitted in some responses
-		if resolution, ok := extendedInfo["Resolution"]; ok {
-			return fmt.Sprintf("%s %s", messageContent, resolution), nil
-		}
-
-		return messageContent, nil
-	}
-
 	// Unmarshal raw Redfish response as arbitrary JSON map
 	var arbitraryJSON map[string]interface{}
 	if err := json.Unmarshal(rawResponse, &arbitraryJSON); err != nil {
@@ -81,7 +81,7 @@ func DecodeRawError(rawResponse []byte) (string, error) {
 	switch extendedInfo := extendedInfoContent.(type) {
 	case []interface{}:
 		if len(extendedInfo) == 0 {
-			return "", ErrUnrecognizedRedfishResponse{Key: "error.@MessageExtendedInfo"}
+			return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo"}
 		}
 
 		var errorMessage string
@@ -91,6 +91,10 @@ func DecodeRawError(rawResponse []byte) (string, error) {
 				return "", ErrUnrecognizedRedfishResponse{Key: "error.@Message.ExtendedInfo"}
 			}
 
+			if _, ok := infoContent["Message"]; !ok {
+				return errContent["message"].(string), nil
+			}
+
 			message, err := processExtendedInfo(infoContent)
 			if err != nil {
 				return "", err
@@ -208,9 +212,7 @@ func ScreenRedfishError(httpResp *http.Response, clientErr error) error {
 				httpResp.Status),
 		}
 	default:
-		finalError = ErrRedfishClient{Message: fmt.Sprintf("BMC responded '%s'.", httpResp.Status)}
-		log.Debugf("BMC responded '%s'. Attempting to unmarshal the raw BMC error response.",
-			httpResp.Status)
+		finalError = ErrRedfishClient{Message: httpResp.Status}
 	}
 
 	// Retrieve the raw HTTP response body
@@ -221,7 +223,7 @@ func ScreenRedfishError(httpResp *http.Response, clientErr error) error {
 
 	// Attempt to decode the BMC response from the raw HTTP response
 	if bmcResponse, err := DecodeRawError(oAPIErr.Body()); err == nil {
-		finalError.Message = fmt.Sprintf("%s BMC responded: '%s'", finalError.Message, bmcResponse)
+		finalError.Message = fmt.Sprintf("%s\nBMC responded: '%s'", finalError.Message, bmcResponse)
 	} else {
 		log.Debugf("Unable to decode BMC response. %q", err)
 	}